diff mbox series

ipmi: Cleanup oops on initialization failure

Message ID 1529497606-3857-1-git-send-email-minyard@acm.org
State New
Headers show
Series ipmi: Cleanup oops on initialization failure | expand

Commit Message

Corey Minyard June 20, 2018, 12:26 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>


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>

---

This patch should fix the issue.

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.

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(-)

-- 
2.7.4

Comments

Meelis Roos June 20, 2018, 2:26 p.m. UTC | #1
> 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)
Corey Minyard June 20, 2018, 11 p.m. UTC | #2
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);

>>   

>>
Meelis Roos June 21, 2018, 6:47 a.m. UTC | #3
> > 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)
Corey Minyard June 21, 2018, 8:16 p.m. UTC | #4
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 mbox series

Patch

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);