Re: [PATCH] ipmi: Cleanup oops on initialization failure

[Corey Minyard]

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@xxxxxxxxxx>
> Tested-by: Meelis Roos <mroos@xxxxxxxx>
>
>
> 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);
> >