Patrick Rudolph has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33488 )
Change subject: drivers/ipmi: Fix multiple issues ......................................................................
drivers/ipmi: Fix multiple issues
* Set abort command define * Set debug level to SPEW * Support zero length data packet in ipmi_kcs_send_message That's required for commands like GET_DEVICE_ID, which have no additional data to send. * Read reply even if given no receive buffer * Prevent buffer overflow in read reply processing
Tested on Wedge100s.
Change-Id: Iefddd88a744c3b96751d3fe8c2951ca2115548ce Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33488 Reviewed-by: Felix Held felix-coreboot@felixheld.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/ipmi/ipmi_kcs.c 1 file changed, 45 insertions(+), 29 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/drivers/ipmi/ipmi_kcs.c b/src/drivers/ipmi/ipmi_kcs.c index d17a1f9..397a800 100644 --- a/src/drivers/ipmi/ipmi_kcs.c +++ b/src/drivers/ipmi/ipmi_kcs.c @@ -22,7 +22,7 @@
#define IPMI_KCS_STATE(_x) ((_x) >> 6)
-#define IPMI_KCS_GET_STATUS_ABORT +#define IPMI_KCS_GET_STATUS_ABORT 0x60 #define IPMI_KCS_START_WRITE 0x61 #define IPMI_KCS_END_WRITE 0x62 #define IPMI_KCS_READ_BYTE 0x68 @@ -43,7 +43,7 @@ static unsigned char ipmi_kcs_status(int port) { unsigned char status = inb(IPMI_STAT(port)); - printk(BIOS_DEBUG, "%s: 0x%02x\n", __func__, status); + printk(BIOS_SPEW, "%s: 0x%02x\n", __func__, status); return status; }
@@ -77,7 +77,7 @@ { unsigned char status;
- printk(BIOS_DEBUG, "%s: %02x\n", __func__, byte); + printk(BIOS_SPEW, "%s: 0x%02x\n", __func__, byte);
outb(byte, IPMI_DATA(port));
@@ -100,7 +100,7 @@ { unsigned char status;
- printk(BIOS_DEBUG, "%s: %02x\n", __func__, byte); + printk(BIOS_SPEW, "%s: 0x%02x\n", __func__, byte);
if (wait_ibf_timeout(port)) return 1; @@ -121,7 +121,7 @@
static int ipmi_kcs_send_cmd_byte(int port, const unsigned char byte) { - printk(BIOS_DEBUG, "%s: 0x%02x\n", __func__, byte); + printk(BIOS_SPEW, "%s: 0x%02x\n", __func__, byte);
if (wait_ibf_timeout(port)) return 1; @@ -156,31 +156,47 @@ return ret; }
- ret = ipmi_kcs_send_data_byte(port, cmd); - if (ret) { - printk(BIOS_ERR, "IPMI CMD failed\n"); - return ret; - } + if (!len) { + ret = ipmi_kcs_send_cmd_byte(port, IPMI_KCS_END_WRITE); + if (ret) { + printk(BIOS_ERR, "IPMI END WRITE failed\n"); + return ret; + }
- while (len-- > 1) { - ret = ipmi_kcs_send_data_byte(port, *msg++); + ret = ipmi_kcs_send_last_data_byte(port, cmd); + if (ret) { + printk(BIOS_ERR, "IPMI BYTE WRITE failed\n"); + return ret; + } + } else { + ret = ipmi_kcs_send_data_byte(port, cmd); + if (ret) { + printk(BIOS_ERR, "IPMI CMD failed\n"); + return ret; + } + + while (len > 1) { + ret = ipmi_kcs_send_data_byte(port, *msg++); + if (ret) { + printk(BIOS_ERR, "IPMI BYTE WRITE failed\n"); + return ret; + } + len--; + } + + ret = ipmi_kcs_send_cmd_byte(port, IPMI_KCS_END_WRITE); + if (ret) { + printk(BIOS_ERR, "IPMI END WRITE failed\n"); + return ret; + } + + ret = ipmi_kcs_send_last_data_byte(port, *msg); if (ret) { printk(BIOS_ERR, "IPMI BYTE WRITE failed\n"); return ret; } }
- ret = ipmi_kcs_send_cmd_byte(port, IPMI_KCS_END_WRITE); - if (ret) { - printk(BIOS_ERR, "IPMI END WRITE failed\n"); - return ret; - } - - ret = ipmi_kcs_send_last_data_byte(port, *msg++); - if (ret) { - printk(BIOS_ERR, "IPMI BYTE WRITE failed\n"); - return ret; - } return 0; }
@@ -188,9 +204,6 @@ { int status, ret = 0;
- if (!msg) - return 0; - if (wait_ibf_timeout(port)) return 1;
@@ -201,15 +214,18 @@ return ret;
if (IPMI_KCS_STATE(status) != IPMI_KCS_STATE_READ) { - printk(BIOS_ERR, "%s: wrong state: 0x%02x\n", __func__, status); + printk(BIOS_ERR, "%s: wrong state: 0x%02x\n", __func__, + status); return -1; }
if (wait_obf_timeout(port)) return -1;
- *msg++ = inb(IPMI_DATA(port)); - ret++; + if (msg && (ret < len)) { + *msg++ = inb(IPMI_DATA(port)); + ret++; + }
if (wait_ibf_timeout(port)) return -1;