| Message ID | 1529497606-3857-1-git-send-email-minyard@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | ipmi: Cleanup oops on initialization failure | expand |
> Commit 93c303d2045b3 "ipmi_si: Clean up shutdown a bit" didn't > copy the behavior of the cleanup in one spot, it needed to > check for a non-NULL interface before cleaning it up. > > Signed-off-by: Corey Minyard <cminyard@mvista.com> Tested-by: Meelis Roos <mroos@linux.ee> 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: 0x0000000000000000 (Memory-mapped) Register Spacing: Successive Byte Boundaries > > Thanks for reporting this. On your tested-by I'll send this up > to Linus. > > -corey > > drivers/char/ipmi/ipmi_si_intf.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c > index 3d0add6..a5987f8 100644 > --- a/drivers/char/ipmi/ipmi_si_intf.c > +++ b/drivers/char/ipmi/ipmi_si_intf.c > @@ -2088,8 +2088,10 @@ static int try_smi_init(struct smi_info *new_smi) > return 0; > > out_err: > - ipmi_unregister_smi(new_smi->intf); > - new_smi->intf = NULL; > + if (new_smi->intf) { > + ipmi_unregister_smi(new_smi->intf); > + new_smi->intf = NULL; > + } > > kfree(init_name); > > -- Meelis Roos (mroos@linux.ee)
On 06/20/2018 09:26 AM, Meelis Roos wrote: >> Commit 93c303d2045b3 "ipmi_si: Clean up shutdown a bit" didn't >> copy the behavior of the cleanup in one spot, it needed to >> check for a non-NULL interface before cleaning it up. >> >> Signed-off-by: Corey Minyard <cminyard@mvista.com> > Tested-by: Meelis Roos <mroos@linux.ee> > > > 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: 0x0000000000000000 (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. -corey >> Thanks for reporting this. On your tested-by I'll send this up >> to Linus. >> >> -corey >> >> drivers/char/ipmi/ipmi_si_intf.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c >> index 3d0add6..a5987f8 100644 >> --- a/drivers/char/ipmi/ipmi_si_intf.c >> +++ b/drivers/char/ipmi/ipmi_si_intf.c >> @@ -2088,8 +2088,10 @@ static int try_smi_init(struct smi_info *new_smi) >> return 0; >> >> out_err: >> - ipmi_unregister_smi(new_smi->intf); >> - new_smi->intf = NULL; >> + if (new_smi->intf) { >> + ipmi_unregister_smi(new_smi->intf); >> + new_smi->intf = NULL; >> + } >> >> kfree(init_name); >> >>
> > 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: 0x0000000000000000 (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: 0x0000000000000CA2 (I/O) Register Spacing: Successive Byte Boundaries -- Meelis Roos (mroos@linux.ee)
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: 0x0000000000000000 (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: 0x0000000000000CA2 (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
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 3d0add6..a5987f8 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2088,8 +2088,10 @@ static int try_smi_init(struct smi_info *new_smi) return 0; out_err: - ipmi_unregister_smi(new_smi->intf); - new_smi->intf = NULL; + if (new_smi->intf) { + ipmi_unregister_smi(new_smi->intf); + new_smi->intf = NULL; + } kfree(init_name);