From f8910ffa81b085030dc54814c85d338c26a3157e Mon Sep 17 00:00:00 2001 From: Xianting Tian Date: Tue, 15 Sep 2020 15:18:17 +0800 Subject: ipmi:msghandler: retry to get device id on an error We fail to get the BMCS's device id with low probability when loading the ipmi driver and it causes BMC device registration failed. When this issue occurs we got below kernel prints: [Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: IPMI message handler: device id demangle failed: -22 [Wed Sep 9 19:52:03 2020] IPMI BT: using default values [Wed Sep 9 19:52:03 2020] IPMI BT: req2rsp=5 secs retries=2 [Wed Sep 9 19:52:03 2020] ipmi_si IPI0001:00: Unable to get the device id: -5 [Wed Sep 9 19:52:04 2020] ipmi_si IPI0001:00: Unable to register device: error -5 When this issue happens, we want to manually unload the driver and try to load it again, but it can't be unloaded by 'rmmod' as it is already 'in use'. We add a print in handle_one_recv_msg(), when this issue happens, the msg we received is "Recv: 1c 01 d5", which means the data_len is 1, data[0] is 0xd5 (completion code), which means "bmc cannot execute command. Command, or request parameter(s), not supported in present state". Debug code: static int handle_one_recv_msg(struct ipmi_smi *intf, struct ipmi_smi_msg *msg) { printk("Recv: %*ph\n", msg->rsp_size, msg->rsp); ... ... } Then in ipmi_demangle_device_id(), it returned '-EINVAL' as 'data_len < 7' and 'data[0] != 0'. We created this patch to retry the get device id when this error happens. We reproduced this issue again and the retry succeed on the first retry, we finally got the correct msg and then all is ok: Recv: 1c 01 00 01 81 05 84 02 af db 07 00 01 00 b9 00 10 00 So use a retry machanism in this patch to give bmc more opportunity to correctly response kernel when we received specific completion codes. Signed-off-by: Xianting Tian Message-Id: <20200915071817.4484-1-tian.xianting@h3c.com> [Cleaned up the verbage a bit in the header and prints.] Signed-off-by: Corey Minyard --- include/uapi/linux/ipmi_msgdefs.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/uapi/linux/ipmi_msgdefs.h b/include/uapi/linux/ipmi_msgdefs.h index c2b23a9fdf3d..0934af3b8037 100644 --- a/include/uapi/linux/ipmi_msgdefs.h +++ b/include/uapi/linux/ipmi_msgdefs.h @@ -69,6 +69,8 @@ #define IPMI_ERR_MSG_TRUNCATED 0xc6 #define IPMI_REQ_LEN_INVALID_ERR 0xc7 #define IPMI_REQ_LEN_EXCEEDED_ERR 0xc8 +#define IPMI_DEVICE_IN_FW_UPDATE_ERR 0xd1 +#define IPMI_DEVICE_IN_INIT_ERR 0xd2 #define IPMI_NOT_IN_MY_STATE_ERR 0xd5 /* IPMI 2.0 */ #define IPMI_LOST_ARBITRATION_ERR 0x81 #define IPMI_BUS_ERR 0x82 -- cgit v1.3-14-g43fede From 42d8a346c5c06689f4f25aecfa287a5aca501a55 Mon Sep 17 00:00:00 2001 From: Xianting Tian Date: Wed, 16 Sep 2020 14:21:29 +0800 Subject: ipmi: add retry in try_get_dev_id() Use a retry machanism to give the BMC more opportunities to correctly respond when we receive specific completion codes. This is similar to what is done in __get_device_id(). Signed-off-by: Xianting Tian Message-Id: <20200916062129.26129-1-tian.xianting@h3c.com> [Moved GET_DEVICE_ID_MAX_RETRY to include/linux/ipmi.h, reworded some text.] Signed-off-by: Corey Minyard --- drivers/char/ipmi/ipmi_msghandler.c | 2 -- drivers/char/ipmi/ipmi_si_intf.c | 17 +++++++++++++++++ include/linux/ipmi.h | 2 ++ 3 files changed, 19 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index e56409659459..555c3b1e4926 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -62,8 +62,6 @@ enum ipmi_panic_event_op { #define IPMI_PANIC_DEFAULT IPMI_SEND_PANIC_EVENT_NONE #endif -#define GET_DEVICE_ID_MAX_RETRY 5 - static enum ipmi_panic_event_op ipmi_send_panic_event = IPMI_PANIC_DEFAULT; static int panic_op_write_handler(const char *val, diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 77b8d551ae7f..164f85007080 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1316,6 +1316,7 @@ static int try_get_dev_id(struct smi_info *smi_info) unsigned char *resp; unsigned long resp_len; int rv = 0; + unsigned int retry_count = 0; resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL); if (!resp) @@ -1327,6 +1328,8 @@ static int try_get_dev_id(struct smi_info *smi_info) */ msg[0] = IPMI_NETFN_APP_REQUEST << 2; msg[1] = IPMI_GET_DEVICE_ID_CMD; + +retry: smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2); rv = wait_for_msg_done(smi_info); @@ -1339,6 +1342,20 @@ static int try_get_dev_id(struct smi_info *smi_info) /* Check and record info from the get device id, in case we need it. */ rv = ipmi_demangle_device_id(resp[0] >> 2, resp[1], resp + 2, resp_len - 2, &smi_info->device_id); + if (rv) { + /* record completion code */ + char cc = *(resp + 2); + + if ((cc == IPMI_DEVICE_IN_FW_UPDATE_ERR + || cc == IPMI_DEVICE_IN_INIT_ERR + || cc == IPMI_NOT_IN_MY_STATE_ERR) + && ++retry_count <= GET_DEVICE_ID_MAX_RETRY) { + dev_warn(smi_info->io.dev, + "BMC returned 0x%2.2x, retry get bmc device id\n", + cc); + goto retry; + } + } out: kfree(resp); diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h index ef61676cfe05..52850a02a3d0 100644 --- a/include/linux/ipmi.h +++ b/include/linux/ipmi.h @@ -333,4 +333,6 @@ struct ipmi_smi_info { /* This is to get the private info of struct ipmi_smi */ extern int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data); +#define GET_DEVICE_ID_MAX_RETRY 5 + #endif /* __LINUX_IPMI_H */ -- cgit v1.3-14-g43fede