2019-02-21 18:34:27

by Corey Minyard

[permalink] [raw]
Subject: [PATCH] ipmi_si: Fix crash when using hard-coded device

From: Corey Minyard <[email protected]>

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 <[email protected]>
Signed-off-by: Corey Minyard <[email protected]>
---

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 <linux/moduleparam.h>
+#include <linux/platform_device.h>
#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;
+ } else if (strcmp(si_type_str, "bt") == 0) {
+ size = 3;
+ si_type = SI_BT;
+ } else if (strcmp(si_type_str, "invalid") == 0) {
+ /*
+ * Allow a firmware-specified interface to be
+ * disabled.
+ */
+ size = 1;
+ si_type = SI_TYPE_INVALID;
+ } else {
+ pr_warn("Interface type specified for interface %d, was invalid: %s\n",
+ i, si_type_str);
+ return;
+ }
+
+ regsize = regsizes[i];
+ if (regsize == 0)
+ regsize = DEFAULT_REGSIZE;
+
+ p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type);
+ p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]);
+ p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED);
+ p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]);
+ p[4] = PROPERTY_ENTRY_U8("reg-size", regsize);
+ /* Last entry must be left NULL to terminate it. */
+
+ /*
+ * Register spacing is derived from the resources in
+ * the IPMI platform code.
+ */
+ regspacing = regspacings[i];
+ if (regspacing == 0)
+ regspacing = regsize;
+
+ r[0].start = addr;
+ r[0].end = r[0].start + regsize - 1;
+ r[0].name = "IPMI Address 1";
+ r[0].flags = flags;
+
+ if (size > 1) {
+ r[1].start = r[0].start + regspacing;
+ r[1].end = r[1].start + regsize - 1;
+ r[1].name = "IPMI Address 2";
+ r[1].flags = flags;
+ num_r++;
+ }
+
+ if (size > 2) {
+ r[2].start = r[1].start + regspacing;
+ r[2].end = r[2].start + regsize - 1;
+ r[2].name = "IPMI Address 3";
+ r[2].flags = flags;
+ num_r++;
+ }
+
+ if (irqs[i]) {
+ r[num_r].start = irqs[i];
+ r[num_r].end = irqs[i];
+ r[num_r].name = "IPMI IRQ";
+ r[num_r].flags = IORESOURCE_IRQ;
+ num_r++;
+ }
+
+ pdev = platform_device_alloc("hardcode-ipmi-si", i);
+ if (!pdev) {
+ pr_err("Error allocating IPMI platform device %d\n", i);
+ return;
+ }
+
+ rv = platform_device_add_resources(pdev, r, num_r);
+ if (rv) {
+ dev_err(&pdev->dev,
+ "Unable to add hard-code resources: %d\n", rv);
+ goto err;
+ }
+
+ rv = platform_device_add_properties(pdev, p);
+ if (rv) {
+ dev_err(&pdev->dev,
+ "Unable to add hard-code properties: %d\n", rv);
+ goto err;
+ }
+
+ rv = platform_device_add(pdev);
+ if (rv) {
+ dev_err(&pdev->dev,
+ "Unable to add hard-code device: %d\n", rv);
+ goto err;
+ }
+
+ ipmi_hc_pdevs[i] = pdev;
+ return;
+
+err:
+ platform_device_put(pdev);
+}
+
+void __init ipmi_hardcode_init(void)
+{
+ unsigned int i;
char *str;
+ char *si_type[SI_MAX_PARMS];

/* Parse out the si_type string into its components. */
str = si_type_str;
@@ -95,54 +216,45 @@ int ipmi_si_hardcode_find_bmc(void)
}
}

- memset(&io, 0, sizeof(io));
for (i = 0; i < SI_MAX_PARMS; i++) {
- if (!ports[i] && !addrs[i])
- continue;
-
- io.addr_source = SI_HARDCODED;
- pr_info("probing via hardcoded address\n");
-
- if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) {
- io.si_type = SI_KCS;
- } else if (strcmp(si_type[i], "smic") == 0) {
- io.si_type = SI_SMIC;
- } else if (strcmp(si_type[i], "bt") == 0) {
- io.si_type = SI_BT;
- } else {
- pr_warn("Interface type specified for interface %d, was invalid: %s\n",
- i, si_type[i]);
- continue;
- }
+ if (i < num_ports && ports[i])
+ ipmi_hardcode_init_one(si_type[i], i, ports[i],
+ IORESOURCE_IO);
+ if (i < num_addrs && addrs[i])
+ ipmi_hardcode_init_one(si_type[i], i, addrs[i],
+ IORESOURCE_MEM);
+ }
+}

- if (ports[i]) {
- /* An I/O port */
- io.addr_data = ports[i];
- io.addr_type = IPMI_IO_ADDR_SPACE;
- } else if (addrs[i]) {
- /* A memory port */
- io.addr_data = addrs[i];
- io.addr_type = IPMI_MEM_ADDR_SPACE;
- } else {
- pr_warn("Interface type specified for interface %d, but port and address were not set or set to zero\n",
- i);
- continue;
- }
+void ipmi_si_hardcode_exit(void)
+{
+ unsigned int i;

- io.addr = NULL;
- io.regspacing = regspacings[i];
- if (!io.regspacing)
- io.regspacing = DEFAULT_REGSPACING;
- io.regsize = regsizes[i];
- if (!io.regsize)
- io.regsize = DEFAULT_REGSIZE;
- io.regshift = regshifts[i];
- io.irq = irqs[i];
- if (io.irq)
- io.irq_setup = ipmi_std_irq_setup;
- io.slave_addr = slave_addrs[i];
-
- ret = ipmi_si_add_smi(&io);
+ for (i = 0; i < SI_MAX_PARMS; i++) {
+ if (ipmi_hc_pdevs[i])
+ platform_device_unregister(ipmi_hc_pdevs[i]);
}
- return ret;
+}
+
+/*
+ * Returns true of the given address exists as a hardcoded address,
+ * false if not.
+ */
+int ipmi_si_hardcode_match(int addr_type, unsigned long addr)
+{
+ unsigned int i;
+
+ if (addr_type == IPMI_IO_ADDR_SPACE) {
+ for (i = 0; i < num_ports; i++) {
+ if (ports[i] == addr)
+ return 1;
+ }
+ } else {
+ for (i = 0; i < num_addrs; i++) {
+ if (addrs[i] == addr)
+ return 1;
+ }
+ }
+
+ return 0;
}
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index ae99d6a14789..abbd526626d5 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -1865,6 +1865,18 @@ int ipmi_si_add_smi(struct si_sm_io *io)
int rv = 0;
struct smi_info *new_smi, *dup;

+ /*
+ * If the user gave us a hard-coded device at the same
+ * address, they presumably want us to use it and not what is
+ * in the firmware.
+ */
+ if (io->addr_source != SI_HARDCODED &&
+ ipmi_si_hardcode_match(io->addr_type, io->addr_data)) {
+ dev_info(io->dev,
+ "Hard-coded device at this address already exists");
+ return -ENODEV;
+ }
+
if (!io->io_setup) {
if (io->addr_type == IPMI_IO_ADDR_SPACE) {
io->io_setup = ipmi_si_port_setup;
@@ -2097,7 +2109,7 @@ static int try_smi_init(struct smi_info *new_smi)
return rv;
}

-static int init_ipmi_si(void)
+static int __init init_ipmi_si(void)
{
struct smi_info *e;
enum ipmi_addr_src type = SI_INVALID;
@@ -2105,11 +2117,9 @@ static int init_ipmi_si(void)
if (initialized)
return 0;

- pr_info("IPMI System Interface driver\n");
+ ipmi_hardcode_init();

- /* If the user gave us a device, they presumably want us to use it */
- if (!ipmi_si_hardcode_find_bmc())
- goto do_scan;
+ pr_info("IPMI System Interface driver\n");

ipmi_si_platform_init();

@@ -2121,7 +2131,6 @@ static int init_ipmi_si(void)
with multiple BMCs we assume that there will be several instances
of a given type so if we succeed in registering a type then also
try to register everything else of the same type */
-do_scan:
mutex_lock(&smi_infos_lock);
list_for_each_entry(e, &smi_infos, link) {
/* Try to register a device if it has an IRQ and we either
@@ -2307,6 +2316,8 @@ static void cleanup_ipmi_si(void)
list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
cleanup_one_si(e);
mutex_unlock(&smi_infos_lock);
+
+ ipmi_si_hardcode_exit();
}
module_exit(cleanup_ipmi_si);

diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
index 15cf819f884f..f8f85416d47a 100644
--- a/drivers/char/ipmi/ipmi_si_platform.c
+++ b/drivers/char/ipmi/ipmi_si_platform.c
@@ -128,8 +128,6 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
if (res_second->start > io->addr_data)
io->regspacing = res_second->start - io->addr_data;
}
- io->regsize = DEFAULT_REGSIZE;
- io->regshift = 0;

return res;
}
@@ -137,7 +135,7 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
static int platform_ipmi_probe(struct platform_device *pdev)
{
struct si_sm_io io;
- u8 type, slave_addr, addr_source;
+ u8 type, slave_addr, addr_source, regsize, regshift;
int rv;

rv = device_property_read_u8(&pdev->dev, "addr-source", &addr_source);
@@ -149,7 +147,7 @@ static int platform_ipmi_probe(struct platform_device *pdev)
if (addr_source == SI_SMBIOS) {
if (!si_trydmi)
return -ENODEV;
- } else {
+ } else if (addr_source != SI_HARDCODED) {
if (!si_tryplatform)
return -ENODEV;
}
@@ -169,11 +167,23 @@ static int platform_ipmi_probe(struct platform_device *pdev)
case SI_BT:
io.si_type = type;
break;
+ case SI_TYPE_INVALID: /* User disabled this in hardcode. */
+ return -ENODEV;
default:
dev_err(&pdev->dev, "ipmi-type property is invalid\n");
return -EINVAL;
}

+ io.regsize = DEFAULT_REGSIZE;
+ rv = device_property_read_u8(&pdev->dev, "reg-size", &regsize);
+ if (!rv)
+ io.regsize = regsize;
+
+ io.regshift = 0;
+ rv = device_property_read_u8(&pdev->dev, "reg-shift", &regshift);
+ if (!rv)
+ io.regshift = regshift;
+
if (!ipmi_get_info_from_resources(pdev, &io))
return -EINVAL;

@@ -193,7 +203,8 @@ static int platform_ipmi_probe(struct platform_device *pdev)

io.dev = &pdev->dev;

- pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
+ pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n",
+ ipmi_addr_src_to_str(addr_source),
(io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
io.addr_data, io.regsize, io.regspacing, io.irq);

@@ -358,6 +369,9 @@ static int acpi_ipmi_probe(struct platform_device *pdev)
goto err_free;
}

+ io.regsize = DEFAULT_REGSIZE;
+ io.regshift = 0;
+
res = ipmi_get_info_from_resources(pdev, &io);
if (!res) {
rv = -EINVAL;
@@ -421,6 +435,7 @@ static int ipmi_remove(struct platform_device *pdev)

static const struct platform_device_id si_plat_ids[] = {
{ "dmi-ipmi-si", 0 },
+ { "hardcode-ipmi-si", 0 },
{ }
};

--
2.17.1



2019-02-22 09:12:42

by Yang Yingliang

[permalink] [raw]
Subject: Re: [PATCH] ipmi_si: Fix crash when using hard-coded device

Tested-by: Yang Yingliang <[email protected]>

[ 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, [email protected] wrote:
> From: Corey Minyard <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Corey Minyard <[email protected]>
> ---
>
> 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 <linux/moduleparam.h>
> +#include <linux/platform_device.h>
> #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;
> + } else if (strcmp(si_type_str, "bt") == 0) {
> + size = 3;
> + si_type = SI_BT;
> + } else if (strcmp(si_type_str, "invalid") == 0) {
> + /*
> + * Allow a firmware-specified interface to be
> + * disabled.
> + */
> + size = 1;
> + si_type = SI_TYPE_INVALID;
> + } else {
> + pr_warn("Interface type specified for interface %d, was invalid: %s\n",
> + i, si_type_str);
> + return;
> + }
> +
> + regsize = regsizes[i];
> + if (regsize == 0)
> + regsize = DEFAULT_REGSIZE;
> +
> + p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type);
> + p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]);
> + p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED);
> + p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]);
> + p[4] = PROPERTY_ENTRY_U8("reg-size", regsize);
> + /* Last entry must be left NULL to terminate it. */
> +
> + /*
> + * Register spacing is derived from the resources in
> + * the IPMI platform code.
> + */
> + regspacing = regspacings[i];
> + if (regspacing == 0)
> + regspacing = regsize;
> +
> + r[0].start = addr;
> + r[0].end = r[0].start + regsize - 1;
> + r[0].name = "IPMI Address 1";
> + r[0].flags = flags;
> +
> + if (size > 1) {
> + r[1].start = r[0].start + regspacing;
> + r[1].end = r[1].start + regsize - 1;
> + r[1].name = "IPMI Address 2";
> + r[1].flags = flags;
> + num_r++;
> + }
> +
> + if (size > 2) {
> + r[2].start = r[1].start + regspacing;
> + r[2].end = r[2].start + regsize - 1;
> + r[2].name = "IPMI Address 3";
> + r[2].flags = flags;
> + num_r++;
> + }
> +
> + if (irqs[i]) {
> + r[num_r].start = irqs[i];
> + r[num_r].end = irqs[i];
> + r[num_r].name = "IPMI IRQ";
> + r[num_r].flags = IORESOURCE_IRQ;
> + num_r++;
> + }
> +
> + pdev = platform_device_alloc("hardcode-ipmi-si", i);
> + if (!pdev) {
> + pr_err("Error allocating IPMI platform device %d\n", i);
> + return;
> + }
> +
> + rv = platform_device_add_resources(pdev, r, num_r);
> + if (rv) {
> + dev_err(&pdev->dev,
> + "Unable to add hard-code resources: %d\n", rv);
> + goto err;
> + }
> +
> + rv = platform_device_add_properties(pdev, p);
> + if (rv) {
> + dev_err(&pdev->dev,
> + "Unable to add hard-code properties: %d\n", rv);
> + goto err;
> + }
> +
> + rv = platform_device_add(pdev);
> + if (rv) {
> + dev_err(&pdev->dev,
> + "Unable to add hard-code device: %d\n", rv);
> + goto err;
> + }
> +
> + ipmi_hc_pdevs[i] = pdev;
> + return;
> +
> +err:
> + platform_device_put(pdev);
> +}
> +
> +void __init ipmi_hardcode_init(void)
> +{
> + unsigned int i;
> char *str;
> + char *si_type[SI_MAX_PARMS];
>
> /* Parse out the si_type string into its components. */
> str = si_type_str;
> @@ -95,54 +216,45 @@ int ipmi_si_hardcode_find_bmc(void)
> }
> }
>
> - memset(&io, 0, sizeof(io));
> for (i = 0; i < SI_MAX_PARMS; i++) {
> - if (!ports[i] && !addrs[i])
> - continue;
> -
> - io.addr_source = SI_HARDCODED;
> - pr_info("probing via hardcoded address\n");
> -
> - if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) {
> - io.si_type = SI_KCS;
> - } else if (strcmp(si_type[i], "smic") == 0) {
> - io.si_type = SI_SMIC;
> - } else if (strcmp(si_type[i], "bt") == 0) {
> - io.si_type = SI_BT;
> - } else {
> - pr_warn("Interface type specified for interface %d, was invalid: %s\n",
> - i, si_type[i]);
> - continue;
> - }
> + if (i < num_ports && ports[i])
> + ipmi_hardcode_init_one(si_type[i], i, ports[i],
> + IORESOURCE_IO);
> + if (i < num_addrs && addrs[i])
> + ipmi_hardcode_init_one(si_type[i], i, addrs[i],
> + IORESOURCE_MEM);
> + }
> +}
>
> - if (ports[i]) {
> - /* An I/O port */
> - io.addr_data = ports[i];
> - io.addr_type = IPMI_IO_ADDR_SPACE;
> - } else if (addrs[i]) {
> - /* A memory port */
> - io.addr_data = addrs[i];
> - io.addr_type = IPMI_MEM_ADDR_SPACE;
> - } else {
> - pr_warn("Interface type specified for interface %d, but port and address were not set or set to zero\n",
> - i);
> - continue;
> - }
> +void ipmi_si_hardcode_exit(void)
> +{
> + unsigned int i;
>
> - io.addr = NULL;
> - io.regspacing = regspacings[i];
> - if (!io.regspacing)
> - io.regspacing = DEFAULT_REGSPACING;
> - io.regsize = regsizes[i];
> - if (!io.regsize)
> - io.regsize = DEFAULT_REGSIZE;
> - io.regshift = regshifts[i];
> - io.irq = irqs[i];
> - if (io.irq)
> - io.irq_setup = ipmi_std_irq_setup;
> - io.slave_addr = slave_addrs[i];
> -
> - ret = ipmi_si_add_smi(&io);
> + for (i = 0; i < SI_MAX_PARMS; i++) {
> + if (ipmi_hc_pdevs[i])
> + platform_device_unregister(ipmi_hc_pdevs[i]);
> }
> - return ret;
> +}
> +
> +/*
> + * Returns true of the given address exists as a hardcoded address,
> + * false if not.
> + */
> +int ipmi_si_hardcode_match(int addr_type, unsigned long addr)
> +{
> + unsigned int i;
> +
> + if (addr_type == IPMI_IO_ADDR_SPACE) {
> + for (i = 0; i < num_ports; i++) {
> + if (ports[i] == addr)
> + return 1;
> + }
> + } else {
> + for (i = 0; i < num_addrs; i++) {
> + if (addrs[i] == addr)
> + return 1;
> + }
> + }
> +
> + return 0;
> }
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index ae99d6a14789..abbd526626d5 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -1865,6 +1865,18 @@ int ipmi_si_add_smi(struct si_sm_io *io)
> int rv = 0;
> struct smi_info *new_smi, *dup;
>
> + /*
> + * If the user gave us a hard-coded device at the same
> + * address, they presumably want us to use it and not what is
> + * in the firmware.
> + */
> + if (io->addr_source != SI_HARDCODED &&
> + ipmi_si_hardcode_match(io->addr_type, io->addr_data)) {
> + dev_info(io->dev,
> + "Hard-coded device at this address already exists");
> + return -ENODEV;
> + }
> +
> if (!io->io_setup) {
> if (io->addr_type == IPMI_IO_ADDR_SPACE) {
> io->io_setup = ipmi_si_port_setup;
> @@ -2097,7 +2109,7 @@ static int try_smi_init(struct smi_info *new_smi)
> return rv;
> }
>
> -static int init_ipmi_si(void)
> +static int __init init_ipmi_si(void)
> {
> struct smi_info *e;
> enum ipmi_addr_src type = SI_INVALID;
> @@ -2105,11 +2117,9 @@ static int init_ipmi_si(void)
> if (initialized)
> return 0;
>
> - pr_info("IPMI System Interface driver\n");
> + ipmi_hardcode_init();
>
> - /* If the user gave us a device, they presumably want us to use it */
> - if (!ipmi_si_hardcode_find_bmc())
> - goto do_scan;
> + pr_info("IPMI System Interface driver\n");
>
> ipmi_si_platform_init();
>
> @@ -2121,7 +2131,6 @@ static int init_ipmi_si(void)
> with multiple BMCs we assume that there will be several instances
> of a given type so if we succeed in registering a type then also
> try to register everything else of the same type */
> -do_scan:
> mutex_lock(&smi_infos_lock);
> list_for_each_entry(e, &smi_infos, link) {
> /* Try to register a device if it has an IRQ and we either
> @@ -2307,6 +2316,8 @@ static void cleanup_ipmi_si(void)
> list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
> cleanup_one_si(e);
> mutex_unlock(&smi_infos_lock);
> +
> + ipmi_si_hardcode_exit();
> }
> module_exit(cleanup_ipmi_si);
>
> diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
> index 15cf819f884f..f8f85416d47a 100644
> --- a/drivers/char/ipmi/ipmi_si_platform.c
> +++ b/drivers/char/ipmi/ipmi_si_platform.c
> @@ -128,8 +128,6 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
> if (res_second->start > io->addr_data)
> io->regspacing = res_second->start - io->addr_data;
> }
> - io->regsize = DEFAULT_REGSIZE;
> - io->regshift = 0;
>
> return res;
> }
> @@ -137,7 +135,7 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
> static int platform_ipmi_probe(struct platform_device *pdev)
> {
> struct si_sm_io io;
> - u8 type, slave_addr, addr_source;
> + u8 type, slave_addr, addr_source, regsize, regshift;
> int rv;
>
> rv = device_property_read_u8(&pdev->dev, "addr-source", &addr_source);
> @@ -149,7 +147,7 @@ static int platform_ipmi_probe(struct platform_device *pdev)
> if (addr_source == SI_SMBIOS) {
> if (!si_trydmi)
> return -ENODEV;
> - } else {
> + } else if (addr_source != SI_HARDCODED) {
> if (!si_tryplatform)
> return -ENODEV;
> }
> @@ -169,11 +167,23 @@ static int platform_ipmi_probe(struct platform_device *pdev)
> case SI_BT:
> io.si_type = type;
> break;
> + case SI_TYPE_INVALID: /* User disabled this in hardcode. */
> + return -ENODEV;
> default:
> dev_err(&pdev->dev, "ipmi-type property is invalid\n");
> return -EINVAL;
> }
>
> + io.regsize = DEFAULT_REGSIZE;
> + rv = device_property_read_u8(&pdev->dev, "reg-size", &regsize);
> + if (!rv)
> + io.regsize = regsize;
> +
> + io.regshift = 0;
> + rv = device_property_read_u8(&pdev->dev, "reg-shift", &regshift);
> + if (!rv)
> + io.regshift = regshift;
> +
> if (!ipmi_get_info_from_resources(pdev, &io))
> return -EINVAL;
>
> @@ -193,7 +203,8 @@ static int platform_ipmi_probe(struct platform_device *pdev)
>
> io.dev = &pdev->dev;
>
> - pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
> + pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n",
> + ipmi_addr_src_to_str(addr_source),
> (io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> io.addr_data, io.regsize, io.regspacing, io.irq);
>
> @@ -358,6 +369,9 @@ static int acpi_ipmi_probe(struct platform_device *pdev)
> goto err_free;
> }
>
> + io.regsize = DEFAULT_REGSIZE;
> + io.regshift = 0;
> +
> res = ipmi_get_info_from_resources(pdev, &io);
> if (!res) {
> rv = -EINVAL;
> @@ -421,6 +435,7 @@ static int ipmi_remove(struct platform_device *pdev)
>
> static const struct platform_device_id si_plat_ids[] = {
> { "dmi-ipmi-si", 0 },
> + { "hardcode-ipmi-si", 0 },
> { }
> };
>



2019-02-22 13:14:50

by Corey Minyard

[permalink] [raw]
Subject: Re: [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 <[email protected]>

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, [email protected] wrote:
> > From: Corey Minyard <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Corey Minyard <[email protected]>
> > ---
> >
> > 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 <linux/moduleparam.h>
> > +#include <linux/platform_device.h>
> > #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;
> > + } else if (strcmp(si_type_str, "bt") == 0) {
> > + size = 3;
> > + si_type = SI_BT;
> > + } else if (strcmp(si_type_str, "invalid") == 0) {
> > + /*
> > + * Allow a firmware-specified interface to be
> > + * disabled.
> > + */
> > + size = 1;
> > + si_type = SI_TYPE_INVALID;
> > + } else {
> > + pr_warn("Interface type specified for interface %d, was invalid: %s\n",
> > + i, si_type_str);
> > + return;
> > + }
> > +
> > + regsize = regsizes[i];
> > + if (regsize == 0)
> > + regsize = DEFAULT_REGSIZE;
> > +
> > + p[0] = PROPERTY_ENTRY_U8("ipmi-type", si_type);
> > + p[1] = PROPERTY_ENTRY_U8("slave-addr", slave_addrs[i]);
> > + p[2] = PROPERTY_ENTRY_U8("addr-source", SI_HARDCODED);
> > + p[3] = PROPERTY_ENTRY_U8("reg-shift", regshifts[i]);
> > + p[4] = PROPERTY_ENTRY_U8("reg-size", regsize);
> > + /* Last entry must be left NULL to terminate it. */
> > +
> > + /*
> > + * Register spacing is derived from the resources in
> > + * the IPMI platform code.
> > + */
> > + regspacing = regspacings[i];
> > + if (regspacing == 0)
> > + regspacing = regsize;
> > +
> > + r[0].start = addr;
> > + r[0].end = r[0].start + regsize - 1;
> > + r[0].name = "IPMI Address 1";
> > + r[0].flags = flags;
> > +
> > + if (size > 1) {
> > + r[1].start = r[0].start + regspacing;
> > + r[1].end = r[1].start + regsize - 1;
> > + r[1].name = "IPMI Address 2";
> > + r[1].flags = flags;
> > + num_r++;
> > + }
> > +
> > + if (size > 2) {
> > + r[2].start = r[1].start + regspacing;
> > + r[2].end = r[2].start + regsize - 1;
> > + r[2].name = "IPMI Address 3";
> > + r[2].flags = flags;
> > + num_r++;
> > + }
> > +
> > + if (irqs[i]) {
> > + r[num_r].start = irqs[i];
> > + r[num_r].end = irqs[i];
> > + r[num_r].name = "IPMI IRQ";
> > + r[num_r].flags = IORESOURCE_IRQ;
> > + num_r++;
> > + }
> > +
> > + pdev = platform_device_alloc("hardcode-ipmi-si", i);
> > + if (!pdev) {
> > + pr_err("Error allocating IPMI platform device %d\n", i);
> > + return;
> > + }
> > +
> > + rv = platform_device_add_resources(pdev, r, num_r);
> > + if (rv) {
> > + dev_err(&pdev->dev,
> > + "Unable to add hard-code resources: %d\n", rv);
> > + goto err;
> > + }
> > +
> > + rv = platform_device_add_properties(pdev, p);
> > + if (rv) {
> > + dev_err(&pdev->dev,
> > + "Unable to add hard-code properties: %d\n", rv);
> > + goto err;
> > + }
> > +
> > + rv = platform_device_add(pdev);
> > + if (rv) {
> > + dev_err(&pdev->dev,
> > + "Unable to add hard-code device: %d\n", rv);
> > + goto err;
> > + }
> > +
> > + ipmi_hc_pdevs[i] = pdev;
> > + return;
> > +
> > +err:
> > + platform_device_put(pdev);
> > +}
> > +
> > +void __init ipmi_hardcode_init(void)
> > +{
> > + unsigned int i;
> > char *str;
> > + char *si_type[SI_MAX_PARMS];
> > /* Parse out the si_type string into its components. */
> > str = si_type_str;
> > @@ -95,54 +216,45 @@ int ipmi_si_hardcode_find_bmc(void)
> > }
> > }
> > - memset(&io, 0, sizeof(io));
> > for (i = 0; i < SI_MAX_PARMS; i++) {
> > - if (!ports[i] && !addrs[i])
> > - continue;
> > -
> > - io.addr_source = SI_HARDCODED;
> > - pr_info("probing via hardcoded address\n");
> > -
> > - if (!si_type[i] || strcmp(si_type[i], "kcs") == 0) {
> > - io.si_type = SI_KCS;
> > - } else if (strcmp(si_type[i], "smic") == 0) {
> > - io.si_type = SI_SMIC;
> > - } else if (strcmp(si_type[i], "bt") == 0) {
> > - io.si_type = SI_BT;
> > - } else {
> > - pr_warn("Interface type specified for interface %d, was invalid: %s\n",
> > - i, si_type[i]);
> > - continue;
> > - }
> > + if (i < num_ports && ports[i])
> > + ipmi_hardcode_init_one(si_type[i], i, ports[i],
> > + IORESOURCE_IO);
> > + if (i < num_addrs && addrs[i])
> > + ipmi_hardcode_init_one(si_type[i], i, addrs[i],
> > + IORESOURCE_MEM);
> > + }
> > +}
> > - if (ports[i]) {
> > - /* An I/O port */
> > - io.addr_data = ports[i];
> > - io.addr_type = IPMI_IO_ADDR_SPACE;
> > - } else if (addrs[i]) {
> > - /* A memory port */
> > - io.addr_data = addrs[i];
> > - io.addr_type = IPMI_MEM_ADDR_SPACE;
> > - } else {
> > - pr_warn("Interface type specified for interface %d, but port and address were not set or set to zero\n",
> > - i);
> > - continue;
> > - }
> > +void ipmi_si_hardcode_exit(void)
> > +{
> > + unsigned int i;
> > - io.addr = NULL;
> > - io.regspacing = regspacings[i];
> > - if (!io.regspacing)
> > - io.regspacing = DEFAULT_REGSPACING;
> > - io.regsize = regsizes[i];
> > - if (!io.regsize)
> > - io.regsize = DEFAULT_REGSIZE;
> > - io.regshift = regshifts[i];
> > - io.irq = irqs[i];
> > - if (io.irq)
> > - io.irq_setup = ipmi_std_irq_setup;
> > - io.slave_addr = slave_addrs[i];
> > -
> > - ret = ipmi_si_add_smi(&io);
> > + for (i = 0; i < SI_MAX_PARMS; i++) {
> > + if (ipmi_hc_pdevs[i])
> > + platform_device_unregister(ipmi_hc_pdevs[i]);
> > }
> > - return ret;
> > +}
> > +
> > +/*
> > + * Returns true of the given address exists as a hardcoded address,
> > + * false if not.
> > + */
> > +int ipmi_si_hardcode_match(int addr_type, unsigned long addr)
> > +{
> > + unsigned int i;
> > +
> > + if (addr_type == IPMI_IO_ADDR_SPACE) {
> > + for (i = 0; i < num_ports; i++) {
> > + if (ports[i] == addr)
> > + return 1;
> > + }
> > + } else {
> > + for (i = 0; i < num_addrs; i++) {
> > + if (addrs[i] == addr)
> > + return 1;
> > + }
> > + }
> > +
> > + return 0;
> > }
> > diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> > index ae99d6a14789..abbd526626d5 100644
> > --- a/drivers/char/ipmi/ipmi_si_intf.c
> > +++ b/drivers/char/ipmi/ipmi_si_intf.c
> > @@ -1865,6 +1865,18 @@ int ipmi_si_add_smi(struct si_sm_io *io)
> > int rv = 0;
> > struct smi_info *new_smi, *dup;
> > + /*
> > + * If the user gave us a hard-coded device at the same
> > + * address, they presumably want us to use it and not what is
> > + * in the firmware.
> > + */
> > + if (io->addr_source != SI_HARDCODED &&
> > + ipmi_si_hardcode_match(io->addr_type, io->addr_data)) {
> > + dev_info(io->dev,
> > + "Hard-coded device at this address already exists");
> > + return -ENODEV;
> > + }
> > +
> > if (!io->io_setup) {
> > if (io->addr_type == IPMI_IO_ADDR_SPACE) {
> > io->io_setup = ipmi_si_port_setup;
> > @@ -2097,7 +2109,7 @@ static int try_smi_init(struct smi_info *new_smi)
> > return rv;
> > }
> > -static int init_ipmi_si(void)
> > +static int __init init_ipmi_si(void)
> > {
> > struct smi_info *e;
> > enum ipmi_addr_src type = SI_INVALID;
> > @@ -2105,11 +2117,9 @@ static int init_ipmi_si(void)
> > if (initialized)
> > return 0;
> > - pr_info("IPMI System Interface driver\n");
> > + ipmi_hardcode_init();
> > - /* If the user gave us a device, they presumably want us to use it */
> > - if (!ipmi_si_hardcode_find_bmc())
> > - goto do_scan;
> > + pr_info("IPMI System Interface driver\n");
> > ipmi_si_platform_init();
> > @@ -2121,7 +2131,6 @@ static int init_ipmi_si(void)
> > with multiple BMCs we assume that there will be several instances
> > of a given type so if we succeed in registering a type then also
> > try to register everything else of the same type */
> > -do_scan:
> > mutex_lock(&smi_infos_lock);
> > list_for_each_entry(e, &smi_infos, link) {
> > /* Try to register a device if it has an IRQ and we either
> > @@ -2307,6 +2316,8 @@ static void cleanup_ipmi_si(void)
> > list_for_each_entry_safe(e, tmp_e, &smi_infos, link)
> > cleanup_one_si(e);
> > mutex_unlock(&smi_infos_lock);
> > +
> > + ipmi_si_hardcode_exit();
> > }
> > module_exit(cleanup_ipmi_si);
> > diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
> > index 15cf819f884f..f8f85416d47a 100644
> > --- a/drivers/char/ipmi/ipmi_si_platform.c
> > +++ b/drivers/char/ipmi/ipmi_si_platform.c
> > @@ -128,8 +128,6 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
> > if (res_second->start > io->addr_data)
> > io->regspacing = res_second->start - io->addr_data;
> > }
> > - io->regsize = DEFAULT_REGSIZE;
> > - io->regshift = 0;
> > return res;
> > }
> > @@ -137,7 +135,7 @@ ipmi_get_info_from_resources(struct platform_device *pdev,
> > static int platform_ipmi_probe(struct platform_device *pdev)
> > {
> > struct si_sm_io io;
> > - u8 type, slave_addr, addr_source;
> > + u8 type, slave_addr, addr_source, regsize, regshift;
> > int rv;
> > rv = device_property_read_u8(&pdev->dev, "addr-source", &addr_source);
> > @@ -149,7 +147,7 @@ static int platform_ipmi_probe(struct platform_device *pdev)
> > if (addr_source == SI_SMBIOS) {
> > if (!si_trydmi)
> > return -ENODEV;
> > - } else {
> > + } else if (addr_source != SI_HARDCODED) {
> > if (!si_tryplatform)
> > return -ENODEV;
> > }
> > @@ -169,11 +167,23 @@ static int platform_ipmi_probe(struct platform_device *pdev)
> > case SI_BT:
> > io.si_type = type;
> > break;
> > + case SI_TYPE_INVALID: /* User disabled this in hardcode. */
> > + return -ENODEV;
> > default:
> > dev_err(&pdev->dev, "ipmi-type property is invalid\n");
> > return -EINVAL;
> > }
> > + io.regsize = DEFAULT_REGSIZE;
> > + rv = device_property_read_u8(&pdev->dev, "reg-size", &regsize);
> > + if (!rv)
> > + io.regsize = regsize;
> > +
> > + io.regshift = 0;
> > + rv = device_property_read_u8(&pdev->dev, "reg-shift", &regshift);
> > + if (!rv)
> > + io.regshift = regshift;
> > +
> > if (!ipmi_get_info_from_resources(pdev, &io))
> > return -EINVAL;
> > @@ -193,7 +203,8 @@ static int platform_ipmi_probe(struct platform_device *pdev)
> > io.dev = &pdev->dev;
> > - pr_info("ipmi_si: SMBIOS: %s %#lx regsize %d spacing %d irq %d\n",
> > + pr_info("ipmi_si: %s: %s %#lx regsize %d spacing %d irq %d\n",
> > + ipmi_addr_src_to_str(addr_source),
> > (io.addr_type == IPMI_IO_ADDR_SPACE) ? "io" : "mem",
> > io.addr_data, io.regsize, io.regspacing, io.irq);
> > @@ -358,6 +369,9 @@ static int acpi_ipmi_probe(struct platform_device *pdev)
> > goto err_free;
> > }
> > + io.regsize = DEFAULT_REGSIZE;
> > + io.regshift = 0;
> > +
> > res = ipmi_get_info_from_resources(pdev, &io);
> > if (!res) {
> > rv = -EINVAL;
> > @@ -421,6 +435,7 @@ static int ipmi_remove(struct platform_device *pdev)
> > static const struct platform_device_id si_plat_ids[] = {
> > { "dmi-ipmi-si", 0 },
> > + { "hardcode-ipmi-si", 0 },
> > { }
> > };
>
>