Re: [Openipmi-developer] [PATCH] ipmi_si: Fix crash when using hard-coded device
On Fri, Feb 22, 2019 at 05:11:49PM +0800, Yang Yingliang wrote: > Tested-by: Yang Yingliang Thanks for testing. This is queued for 5.1. I'd like to let the soak a bit in linux-next. It is tagged for a backport to stable releases. -corey > > [ 128.069528] ipmi_si: IPMI System Interface driver > [ 128.069556] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS > [ 128.069559] ipmi_platform: ipmi_si: SMBIOS: io 0xe4 regsize 1 spacing 1 > irq 0 > [ 128.069561] ipmi_si: Adding SMBIOS-specified bt state machine > [ 128.069657] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: probing via ACPI > [ 128.069694] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: [io > 0xffc0e3-0xffc0e7] regsize 1 spacing 1 irq 0 > [ 128.069696] ipmi_si hisi-lpc-ipmi.1.auto: Hard-coded device at this > address already exists > [ 128.069815] ipmi_si hardcode-ipmi-si.0: ipmi_platform: probing via > hardcoded > [ 128.069817] ipmi_platform: ipmi_si: hardcoded: io 0xffc0e3 regsize 1 > spacing 1 irq 0 > [ 128.069819] ipmi_si: Adding hardcoded-specified bt state machine > [ 128.069892] ipmi_si: Trying SMBIOS-specified bt state machine at i/o > address 0xe4, slave address 0x0, irq 0 > [ 128.069896] ipmi_si dmi-ipmi-si.0: Could not set up I/O space > [ 128.069900] ipmi_si: Trying hardcoded-specified bt state machine at i/o > address 0xffc0e3, slave address 0x0, irq 0 > [ 128.081782] ipmi_si hardcode-ipmi-si.0: bt cap response too short: 3 > [ 128.081784] ipmi_si hardcode-ipmi-si.0: using default values > [ 128.081786] ipmi_si hardcode-ipmi-si.0: req2rsp=5 secs retries=2 > [ 128.157855] ipmi_si hardcode-ipmi-si.0: IPMI message handler: Found new > BMC (man_id: 0x0007db, prod_id: 0x0001, dev_id: 0x01) > [ 128.441860] ipmi_si hardcode-ipmi-si.0: IPMI bt interface initialized > > On 2019/2/22 2:33, miny...@acm.org wrote: > > From: Corey Minyard > > > > When excuting a command like: > >modprobe ipmi_si ports=0xffc0e3 type=bt > > The system would get an oops. > > > > The trouble here is that ipmi_si_hardcode_find_bmc() is called before > > ipmi_si_platform_init(), but initialization of the hard-coded device > > creates an IPMI platform device, which won't be initialized yet. > > > > The real trouble is that hard-coded devices aren't created with > > any device, and the fixup is done later. So do it right, create the > > hard-coded devices as normal platform devices. > > > > This required adding some new resource types to the IPMI platform > > code for passing information required by the hard-coded device > > and adding some code to remove the hard-coded platform devices > > on module removal. > > > > To enforce the "hard-coded devices passed by the user take priority > > over firmware devices" rule, some special code was added to check > > and see if a hard-coded device already exists. > > > > Reported-by: Yang Yingliang > > Signed-off-by: Corey Minyard > > --- > > > > I believe this is the right fix. Can you test/review and send the > > results? > > > > Thanks, > > > > -corey > > > > drivers/char/ipmi/ipmi_si.h | 4 +- > > drivers/char/ipmi/ipmi_si_hardcode.c | 236 --- > > drivers/char/ipmi/ipmi_si_intf.c | 23 ++- > > drivers/char/ipmi/ipmi_si_platform.c | 25 ++- > > 4 files changed, 214 insertions(+), 74 deletions(-) > > > > diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h > > index 52f6152d1fcb..7ae52c17618e 100644 > > --- a/drivers/char/ipmi/ipmi_si.h > > +++ b/drivers/char/ipmi/ipmi_si.h > > @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); > > int ipmi_si_remove_by_dev(struct device *dev); > > void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, > > unsigned long addr); > > -int ipmi_si_hardcode_find_bmc(void); > > +void ipmi_hardcode_init(void); > > +void ipmi_si_hardcode_exit(void); > > +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); > > void ipmi_si_platform_init(void); > > void ipmi_si_platform_shutdown(void); > > diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c > > b/drivers/char/ipmi/ipmi_si_hardcode.c > > index 487642809c58..1e5783961b0d 100644 > > --- a/drivers/char/ipmi/ipmi_si_hardcode.c > > +++ b/drivers/char/ipmi/ipmi_si_hardcode.c > > @@ -3,6 +3,7 @@ > > #define pr_fmt(fmt) "ipmi_hardcode: " fmt > > #include > > +#include > > #include "ipmi_si.h" > > /* > > @@ -12,23 +13,22 @@ > > #define SI_MAX_PARMS 4 > > -static char *si_type[SI_MAX_PARMS]; > > #define MAX_SI_TYPE_STR 30 > > -static char si_type_str[MAX_SI_TYPE_STR]; > > +static char si_type_str[MAX_SI_TYPE_STR] __initdata; > > static unsigned long addrs[SI_MAX_PARMS]; > > static unsigned int num_addrs; > > static unsigned int ports[SI_MAX_PARMS]; > > static unsigned int num_ports; > > -static int irqs[SI_MAX_PARMS]; > > -static unsigned int num_irqs; > > -static int regspacings[SI_MAX_PARMS]; > > -static unsigned int
Re: [Openipmi-developer] [PATCH] ipmi_si: Fix crash when using hard-coded device
Tested-by: Yang Yingliang [ 128.069528] ipmi_si: IPMI System Interface driver [ 128.069556] ipmi_si dmi-ipmi-si.0: ipmi_platform: probing via SMBIOS [ 128.069559] ipmi_platform: ipmi_si: SMBIOS: io 0xe4 regsize 1 spacing 1 irq 0 [ 128.069561] ipmi_si: Adding SMBIOS-specified bt state machine [ 128.069657] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: probing via ACPI [ 128.069694] ipmi_si hisi-lpc-ipmi.1.auto: ipmi_platform: [io 0xffc0e3-0xffc0e7] regsize 1 spacing 1 irq 0 [ 128.069696] ipmi_si hisi-lpc-ipmi.1.auto: Hard-coded device at this address already exists [ 128.069815] ipmi_si hardcode-ipmi-si.0: ipmi_platform: probing via hardcoded [ 128.069817] ipmi_platform: ipmi_si: hardcoded: io 0xffc0e3 regsize 1 spacing 1 irq 0 [ 128.069819] ipmi_si: Adding hardcoded-specified bt state machine [ 128.069892] ipmi_si: Trying SMBIOS-specified bt state machine at i/o address 0xe4, slave address 0x0, irq 0 [ 128.069896] ipmi_si dmi-ipmi-si.0: Could not set up I/O space [ 128.069900] ipmi_si: Trying hardcoded-specified bt state machine at i/o address 0xffc0e3, slave address 0x0, irq 0 [ 128.081782] ipmi_si hardcode-ipmi-si.0: bt cap response too short: 3 [ 128.081784] ipmi_si hardcode-ipmi-si.0: using default values [ 128.081786] ipmi_si hardcode-ipmi-si.0: req2rsp=5 secs retries=2 [ 128.157855] ipmi_si hardcode-ipmi-si.0: IPMI message handler: Found new BMC (man_id: 0x0007db, prod_id: 0x0001, dev_id: 0x01) [ 128.441860] ipmi_si hardcode-ipmi-si.0: IPMI bt interface initialized On 2019/2/22 2:33, miny...@acm.org wrote: From: Corey Minyard When excuting a command like: modprobe ipmi_si ports=0xffc0e3 type=bt The system would get an oops. The trouble here is that ipmi_si_hardcode_find_bmc() is called before ipmi_si_platform_init(), but initialization of the hard-coded device creates an IPMI platform device, which won't be initialized yet. The real trouble is that hard-coded devices aren't created with any device, and the fixup is done later. So do it right, create the hard-coded devices as normal platform devices. This required adding some new resource types to the IPMI platform code for passing information required by the hard-coded device and adding some code to remove the hard-coded platform devices on module removal. To enforce the "hard-coded devices passed by the user take priority over firmware devices" rule, some special code was added to check and see if a hard-coded device already exists. Reported-by: Yang Yingliang Signed-off-by: Corey Minyard --- I believe this is the right fix. Can you test/review and send the results? Thanks, -corey drivers/char/ipmi/ipmi_si.h | 4 +- drivers/char/ipmi/ipmi_si_hardcode.c | 236 --- drivers/char/ipmi/ipmi_si_intf.c | 23 ++- drivers/char/ipmi/ipmi_si_platform.c | 25 ++- 4 files changed, 214 insertions(+), 74 deletions(-) diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h index 52f6152d1fcb..7ae52c17618e 100644 --- a/drivers/char/ipmi/ipmi_si.h +++ b/drivers/char/ipmi/ipmi_si.h @@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io); int ipmi_si_remove_by_dev(struct device *dev); void ipmi_si_remove_by_data(int addr_space, enum si_type si_type, unsigned long addr); -int ipmi_si_hardcode_find_bmc(void); +void ipmi_hardcode_init(void); +void ipmi_si_hardcode_exit(void); +int ipmi_si_hardcode_match(int addr_type, unsigned long addr); void ipmi_si_platform_init(void); void ipmi_si_platform_shutdown(void); diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c b/drivers/char/ipmi/ipmi_si_hardcode.c index 487642809c58..1e5783961b0d 100644 --- a/drivers/char/ipmi/ipmi_si_hardcode.c +++ b/drivers/char/ipmi/ipmi_si_hardcode.c @@ -3,6 +3,7 @@ #define pr_fmt(fmt) "ipmi_hardcode: " fmt #include +#include #include "ipmi_si.h" /* @@ -12,23 +13,22 @@ #define SI_MAX_PARMS 4 -static char *si_type[SI_MAX_PARMS]; #define MAX_SI_TYPE_STR 30 -static char si_type_str[MAX_SI_TYPE_STR]; +static char si_type_str[MAX_SI_TYPE_STR] __initdata; static unsigned long addrs[SI_MAX_PARMS]; static unsigned int num_addrs; static unsigned int ports[SI_MAX_PARMS]; static unsigned int num_ports; -static int irqs[SI_MAX_PARMS]; -static unsigned int num_irqs; -static int regspacings[SI_MAX_PARMS]; -static unsigned int num_regspacings; -static int regsizes[SI_MAX_PARMS]; -static unsigned int num_regsizes; -static int regshifts[SI_MAX_PARMS]; -static unsigned int num_regshifts; -static int slave_addrs[SI_MAX_PARMS]; /* Leaving 0 chooses the default value */ -static unsigned int num_slave_addrs; +static int irqs[SI_MAX_PARMS] __initdata; +static unsigned int num_irqs __initdata; +static int regspacings[SI_MAX_PARMS] __initdata; +static unsigned int num_regspacings __initdata; +static int regsizes[SI_MAX_PARMS]
[Openipmi-developer] [PATCH] ipmi_si: Fix crash when using hard-coded device
From: Corey Minyard
When excuting a command like:
modprobe ipmi_si ports=0xffc0e3 type=bt
The system would get an oops.
The trouble here is that ipmi_si_hardcode_find_bmc() is called before
ipmi_si_platform_init(), but initialization of the hard-coded device
creates an IPMI platform device, which won't be initialized yet.
The real trouble is that hard-coded devices aren't created with
any device, and the fixup is done later. So do it right, create the
hard-coded devices as normal platform devices.
This required adding some new resource types to the IPMI platform
code for passing information required by the hard-coded device
and adding some code to remove the hard-coded platform devices
on module removal.
To enforce the "hard-coded devices passed by the user take priority
over firmware devices" rule, some special code was added to check
and see if a hard-coded device already exists.
Reported-by: Yang Yingliang
Signed-off-by: Corey Minyard
---
I believe this is the right fix. Can you test/review and send the
results?
Thanks,
-corey
drivers/char/ipmi/ipmi_si.h | 4 +-
drivers/char/ipmi/ipmi_si_hardcode.c | 236 ---
drivers/char/ipmi/ipmi_si_intf.c | 23 ++-
drivers/char/ipmi/ipmi_si_platform.c | 25 ++-
4 files changed, 214 insertions(+), 74 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_si.h b/drivers/char/ipmi/ipmi_si.h
index 52f6152d1fcb..7ae52c17618e 100644
--- a/drivers/char/ipmi/ipmi_si.h
+++ b/drivers/char/ipmi/ipmi_si.h
@@ -25,7 +25,9 @@ void ipmi_irq_finish_setup(struct si_sm_io *io);
int ipmi_si_remove_by_dev(struct device *dev);
void ipmi_si_remove_by_data(int addr_space, enum si_type si_type,
unsigned long addr);
-int ipmi_si_hardcode_find_bmc(void);
+void ipmi_hardcode_init(void);
+void ipmi_si_hardcode_exit(void);
+int ipmi_si_hardcode_match(int addr_type, unsigned long addr);
void ipmi_si_platform_init(void);
void ipmi_si_platform_shutdown(void);
diff --git a/drivers/char/ipmi/ipmi_si_hardcode.c
b/drivers/char/ipmi/ipmi_si_hardcode.c
index 487642809c58..1e5783961b0d 100644
--- a/drivers/char/ipmi/ipmi_si_hardcode.c
+++ b/drivers/char/ipmi/ipmi_si_hardcode.c
@@ -3,6 +3,7 @@
#define pr_fmt(fmt) "ipmi_hardcode: " fmt
#include
+#include
#include "ipmi_si.h"
/*
@@ -12,23 +13,22 @@
#define SI_MAX_PARMS 4
-static char *si_type[SI_MAX_PARMS];
#define MAX_SI_TYPE_STR 30
-static char si_type_str[MAX_SI_TYPE_STR];
+static char si_type_str[MAX_SI_TYPE_STR] __initdata;
static unsigned long addrs[SI_MAX_PARMS];
static unsigned int num_addrs;
static unsigned int ports[SI_MAX_PARMS];
static unsigned int num_ports;
-static int irqs[SI_MAX_PARMS];
-static unsigned int num_irqs;
-static int regspacings[SI_MAX_PARMS];
-static unsigned int num_regspacings;
-static int regsizes[SI_MAX_PARMS];
-static unsigned int num_regsizes;
-static int regshifts[SI_MAX_PARMS];
-static unsigned int num_regshifts;
-static int slave_addrs[SI_MAX_PARMS]; /* Leaving 0 chooses the default value */
-static unsigned int num_slave_addrs;
+static int irqs[SI_MAX_PARMS] __initdata;
+static unsigned int num_irqs __initdata;
+static int regspacings[SI_MAX_PARMS] __initdata;
+static unsigned int num_regspacings __initdata;
+static int regsizes[SI_MAX_PARMS] __initdata;
+static unsigned int num_regsizes __initdata;
+static int regshifts[SI_MAX_PARMS] __initdata;
+static unsigned int num_regshifts __initdata;
+static int slave_addrs[SI_MAX_PARMS] __initdata;
+static unsigned int num_slave_addrs __initdata;
module_param_string(type, si_type_str, MAX_SI_TYPE_STR, 0);
MODULE_PARM_DESC(type, "Defines the type of each interface, each"
@@ -73,12 +73,133 @@ MODULE_PARM_DESC(slave_addrs, "Set the default IPMB slave
address for"
" overridden by this parm. This is an array indexed"
" by interface number.");
-int ipmi_si_hardcode_find_bmc(void)
+static struct platform_device *ipmi_hc_pdevs[SI_MAX_PARMS];
+
+static void __init ipmi_hardcode_init_one(const char *si_type_str,
+ unsigned int i,
+ unsigned long addr,
+ unsigned int flags)
{
- int ret = -ENODEV;
- int i;
- struct si_sm_io io;
+ struct platform_device *pdev;
+ unsigned int num_r = 1, size;
+ struct resource r[4];
+ struct property_entry p[6];
+ enum si_type si_type;
+ unsigned int regspacing, regsize;
+ int rv;
+
+ memset(p, 0, sizeof(p));
+ memset(r, 0, sizeof(r));
+
+ if (!si_type_str || !*si_type_str || strcmp(si_type_str, "kcs") == 0) {
+ size = 2;
+ si_type = SI_KCS;
+ } else if (strcmp(si_type_str, "smic") == 0) {
+ size = 2;
+ si_type = SI_SMIC;
+ }
