[Openipmi-developer] [PATCH ] ipmi: kcs_bmc: fix IRQ exception if the channel is not open
The original core KCS IRQ handling function 'kcs_bmc_handle_event' had a wrong logical desgin, it should force abort the KCS transaction only if the IBF flag is set, because multiple KCS channels share the same handle function; and it should return 0, which indicates that event of current channel is handled. An negative value -ENODATA will cause the IRQ handler of BMC KCS controller return IRQ_NONE, then IRQ moudle will treat this IRQ 'nobody cared', it will trigger IRQ exception. irq 30: nobody cared (try booting with the "irqpoll" option) CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.10.15-npcm750 #1 Hardware name: NPCMX50 Chip family [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x8c/0xa0) [] (dump_stack) from [] (__report_bad_irq+0x3c/0xdc) [] (__report_bad_irq) from [] (note_interrupt+0x29c/0x2ec) [] (note_interrupt) from [] (handle_irq_event_percpu+0x5c/0x68) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x48/0x6c) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xc8/0x198) [] (handle_fasteoi_irq) from [] (__handle_domain_irq+0x90/0xe8) [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x9c) [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) Exception stack(0xc0a01de8 to 0xc0a01e30) 1de0: 2080 c0a6fbc0 c096d294 1e00: 0001 dc406400 f03ff100 0082 c0a01e94 c0a6fbc0 c0a01e38 1e20: 00200102 c01015bc 6113 [] (__irq_svc) from [] (__do_softirq+0xbc/0x358) [] (__do_softirq) from [] (irq_exit+0xb8/0xec) [] (irq_exit) from [] (__handle_domain_irq+0x94/0xe8) [] (__handle_domain_irq) from [] (gic_handle_irq+0x58/0x9c) [] (gic_handle_irq) from [] (__irq_svc+0x6c/0x90) Exception stack(0xc0a01ef8 to 0xc0a01f40) 1ee0: 03ae 1f00: dcc0f338 c0111060 c0a0 c0a0cc44 c0a0cbe4 c0a1c22b c07bc218 0001 1f20: dcffca40 c0a01f54 c0a01f58 c0a01f48 c0103524 c0103528 6013 [] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c) [] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c) [] (default_idle_call) from [] (do_idle+0xc8/0x134) [] (do_idle) from [] (cpu_startup_entry+0x28/0x2c) [] (cpu_startup_entry) from [] (rest_init+0x84/0x88) [] (rest_init) from [] (start_kernel+0x388/0x394) [] (start_kernel) from [<807c>] (0x807c) handlers: [] npcm7xx_kcs_irq Disabling IRQ #30 Signed-off-by: Haiyue Wang --- Hi Corey, This patch looks introducing BIG modification, it should be easily understandable, and makes code clean & fix an error design, which is introduced by misunderstanding the IRQ return value. BR, Haiyue --- drivers/char/ipmi/kcs_bmc.c | 31 ++- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c index fbfc05e..bb882ab1 100644 --- a/drivers/char/ipmi/kcs_bmc.c +++ b/drivers/char/ipmi/kcs_bmc.c @@ -210,34 +210,23 @@ static void kcs_bmc_handle_cmd(struct kcs_bmc *kcs_bmc) int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc) { unsigned long flags; - int ret = 0; + int ret = -ENODATA; u8 status; spin_lock_irqsave(_bmc->lock, flags); - if (!kcs_bmc->running) { - kcs_force_abort(kcs_bmc); - ret = -ENODEV; - goto out_unlock; - } - - status = read_status(kcs_bmc) & (KCS_STATUS_IBF | KCS_STATUS_CMD_DAT); - - switch (status) { - case KCS_STATUS_IBF | KCS_STATUS_CMD_DAT: - kcs_bmc_handle_cmd(kcs_bmc); - break; - - case KCS_STATUS_IBF: - kcs_bmc_handle_data(kcs_bmc); - break; + status = read_status(kcs_bmc); + if (status & KCS_STATUS_IBF) { + if (!kcs_bmc->running) + kcs_force_abort(kcs_bmc); + else if (status & KCS_STATUS_CMD_DAT) + kcs_bmc_handle_cmd(kcs_bmc); + else + kcs_bmc_handle_data(kcs_bmc); - default: - ret = -ENODATA; - break; + ret = 0; } -out_unlock: spin_unlock_irqrestore(_bmc->lock, flags); return ret; -- 2.7.4 -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] ipmi: Cleanup oops on initialization failure
On 06/21/2018 01:47 AM, Meelis Roos wrote: The corresponding dmesg: [7.372830] IPMI System Interface driver. [7.373034] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS [7.373109] ipmi_si: SMBIOS: mem 0x0 regsize 1 spacing 1 irq 0 [7.373182] ipmi_si: Adding SMBIOS-specified kcs state machine [7.373352] ipmi_si: Trying SMBIOS-specified kcs state machine at mem address 0x0, slave address 0x20, irq 0 [7.373479] ipmi_si dmi-ipmi-si.0: Could not set up I/O space BTW, can you send me at least the IPMI portion of the output of dmidecode for your machine? I have seen a lot of these where the address in the SMBIOS tables is incorrect, and I'm wondering if it's something in the driver, or if it's really the tables that are bad. Handle 0x001B, DMI type 38, 18 bytes IPMI Device Information Interface Type: KCS (Keyboard Control Style) Specification Version: 2.0 I2C Slave Address: 0x10 NV Storage Device: Not Present Base Address: 0x (Memory-mapped) Register Spacing: Successive Byte Boundaries Thanks a bunch. It looks like the SMBIOS tables are wrong. I wonder if this is what some vendor do if there is no IPMI device installed. I guess I need to add a check for this. Another machine (Sun X2100) with similar crash is also cured by the patch, but this is slightly different (not NULL): [8.891217] IPMI System Interface driver. [8.898404] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS [8.905635] ipmi_si: SMBIOS: io 0xca2 regsize 1 spacing 1 irq 0 [8.912895] ipmi_si: Adding SMBIOS-specified kcs state machine [8.920246] ipmi_si: Trying SMBIOS-specified kcs state machine at i/o address 0xca2, slave address 0x20, irq 0 [8.934379] ipmi_si dmi-ipmi-si.0: Interface detection failed IPMI Device Information Interface Type: KCS (Keyboard Control Style) Specification Version: 1.5 I2C Slave Address: 0x10 NV Storage Device: Not Present Base Address: 0x0CA2 (I/O) Register Spacing: Successive Byte Boundaries That's even worse. The SMBIOS table says the interface is there, but it's not there. Not much I can do about that :(. Thanks again, -corey -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [Openipmi-developer] [PATCH] ipmi: Cleanup oops on initialization failure
> > The corresponding dmesg: > > > > [7.372830] IPMI System Interface driver. > > [7.373034] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS > > [7.373109] ipmi_si: SMBIOS: mem 0x0 regsize 1 spacing 1 irq 0 > > [7.373182] ipmi_si: Adding SMBIOS-specified kcs state machine > > [7.373352] ipmi_si: Trying SMBIOS-specified kcs state machine at mem > > address 0x0, slave address 0x20, irq 0 > > [7.373479] ipmi_si dmi-ipmi-si.0: Could not set up I/O space > > > > > BTW, can you send me at least the IPMI portion of the output of > > > dmidecode for your machine? I have seen a lot of these where the > > > address in the SMBIOS tables is incorrect, and I'm wondering if > > > it's something in the driver, or if it's really the tables that > > > are bad. > > Handle 0x001B, DMI type 38, 18 bytes > > IPMI Device Information > > Interface Type: KCS (Keyboard Control Style) > > Specification Version: 2.0 > > I2C Slave Address: 0x10 > > NV Storage Device: Not Present > > Base Address: 0x (Memory-mapped) > > Register Spacing: Successive Byte Boundaries > > Thanks a bunch. It looks like the SMBIOS tables are wrong. I > wonder if this is what some vendor do if there is no IPMI device > installed. I guess I need to add a check for this. Another machine (Sun X2100) with similar crash is also cured by the patch, but this is slightly different (not NULL): [8.891217] IPMI System Interface driver. [8.898404] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS [8.905635] ipmi_si: SMBIOS: io 0xca2 regsize 1 spacing 1 irq 0 [8.912895] ipmi_si: Adding SMBIOS-specified kcs state machine [8.920246] ipmi_si: Trying SMBIOS-specified kcs state machine at i/o address 0xca2, slave address 0x20, irq 0 [8.934379] ipmi_si dmi-ipmi-si.0: Interface detection failed IPMI Device Information Interface Type: KCS (Keyboard Control Style) Specification Version: 1.5 I2C Slave Address: 0x10 NV Storage Device: Not Present Base Address: 0x0CA2 (I/O) Register Spacing: Successive Byte Boundaries -- Meelis Roos (mr...@linux.ee) -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer