From 92e847212676bb3c5f9f7e317907367dbb8c504b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 3 Sep 2015 14:58:55 -0500 Subject: ipmi: Add device tree bindings information Signed-off-by: Corey Minyard diff --git a/Documentation/devicetree/bindings/ipmi.txt b/Documentation/devicetree/bindings/ipmi.txt new file mode 100644 index 0000000..d5f1a87 --- /dev/null +++ b/Documentation/devicetree/bindings/ipmi.txt @@ -0,0 +1,25 @@ +IPMI device + +Required properties: +- compatible: should be one of ipmi-kcs, ipmi-smic, or ipmi-bt +- device_type: should be ipmi +- reg: Address and length of the register set for the device + +Optional properties: +- interrupts: The interrupt for the device. Without this the interface + is polled. +- reg-size - The size of the register. Defaults to 1 +- reg-spacing - The number of bytes between register starts. Defaults to 1 +- reg-shift - The amount to shift the registers to the right to get the data + into bit zero. + +Example: + +smic@fff3a000 { + compatible = "ipmi-smic"; + device_type = "ipmi"; + reg = <0xfff3a000 0x1000>; + interrupts = <0 24 4>; + reg-size = <4>; + reg-spacing = <4>; +}; -- cgit v0.10.2 From 0fbcf4af7c8362d4691f9388efa57d0b14b34225 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 9 Jun 2015 16:51:46 -0500 Subject: ipmi: Convert the IPMI SI ACPI handling to a platform device The IPMI SI driver was using direct PNP, but that was not really ideal because the IPMI device is a platform device. There was some special handling in the acpi_pnp.c code for making this work, but that was breaking ACPI handling for the IPMI SSIF driver. So without this patch there were significant issues getting the SSIF driver to work with ACPI. So use a platform device for ACPI detection and remove the entry from acpi_pnp.c. Signed-off-by: Corey Minyard diff --git a/drivers/acpi/acpi_pnp.c b/drivers/acpi/acpi_pnp.c index fb76552..c58940b 100644 --- a/drivers/acpi/acpi_pnp.c +++ b/drivers/acpi/acpi_pnp.c @@ -19,8 +19,6 @@ static const struct acpi_device_id acpi_pnp_device_ids[] = { {"PNP0600"}, /* Generic ESDI/IDE/ATA compatible hard disk controller */ /* floppy */ {"PNP0700"}, - /* ipmi_si */ - {"IPI0001"}, /* tpm_inf_pnp */ {"IFX0101"}, /* Infineon TPMs */ {"IFX0102"}, /* Infineon TPMs */ diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 8a45e92..6443e76 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -64,7 +64,6 @@ #include #include #include -#include #include #include #include @@ -309,9 +308,6 @@ static int num_force_kipmid; #ifdef CONFIG_PCI static bool pci_registered; #endif -#ifdef CONFIG_ACPI -static bool pnp_registered; -#endif #ifdef CONFIG_PARISC static bool parisc_registered; #endif @@ -2233,134 +2229,6 @@ static void spmi_find_bmc(void) try_init_spmi(spmi); } } - -static int ipmi_pnp_probe(struct pnp_dev *dev, - const struct pnp_device_id *dev_id) -{ - struct acpi_device *acpi_dev; - struct smi_info *info; - struct resource *res, *res_second; - acpi_handle handle; - acpi_status status; - unsigned long long tmp; - int rv = -EINVAL; - - acpi_dev = pnp_acpi_device(dev); - if (!acpi_dev) - return -ENODEV; - - info = smi_info_alloc(); - if (!info) - return -ENOMEM; - - info->addr_source = SI_ACPI; - printk(KERN_INFO PFX "probing via ACPI\n"); - - handle = acpi_dev->handle; - info->addr_info.acpi_info.acpi_handle = handle; - - /* _IFT tells us the interface type: KCS, BT, etc */ - status = acpi_evaluate_integer(handle, "_IFT", NULL, &tmp); - if (ACPI_FAILURE(status)) { - dev_err(&dev->dev, "Could not find ACPI IPMI interface type\n"); - goto err_free; - } - - switch (tmp) { - case 1: - info->si_type = SI_KCS; - break; - case 2: - info->si_type = SI_SMIC; - break; - case 3: - info->si_type = SI_BT; - break; - case 4: /* SSIF, just ignore */ - rv = -ENODEV; - goto err_free; - default: - dev_info(&dev->dev, "unknown IPMI type %lld\n", tmp); - goto err_free; - } - - res = pnp_get_resource(dev, IORESOURCE_IO, 0); - if (res) { - info->io_setup = port_setup; - info->io.addr_type = IPMI_IO_ADDR_SPACE; - } else { - res = pnp_get_resource(dev, IORESOURCE_MEM, 0); - if (res) { - info->io_setup = mem_setup; - info->io.addr_type = IPMI_MEM_ADDR_SPACE; - } - } - if (!res) { - dev_err(&dev->dev, "no I/O or memory address\n"); - goto err_free; - } - info->io.addr_data = res->start; - - info->io.regspacing = DEFAULT_REGSPACING; - res_second = pnp_get_resource(dev, - (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? - IORESOURCE_IO : IORESOURCE_MEM, - 1); - if (res_second) { - if (res_second->start > info->io.addr_data) - info->io.regspacing = res_second->start - info->io.addr_data; - } - info->io.regsize = DEFAULT_REGSPACING; - info->io.regshift = 0; - - /* If _GPE exists, use it; otherwise use standard interrupts */ - status = acpi_evaluate_integer(handle, "_GPE", NULL, &tmp); - if (ACPI_SUCCESS(status)) { - info->irq = tmp; - info->irq_setup = acpi_gpe_irq_setup; - } else if (pnp_irq_valid(dev, 0)) { - info->irq = pnp_irq(dev, 0); - info->irq_setup = std_irq_setup; - } - - info->dev = &dev->dev; - pnp_set_drvdata(dev, info); - - dev_info(info->dev, "%pR regsize %d spacing %d irq %d\n", - res, info->io.regsize, info->io.regspacing, - info->irq); - - rv = add_smi(info); - if (rv) - kfree(info); - - return rv; - -err_free: - kfree(info); - return rv; -} - -static void ipmi_pnp_remove(struct pnp_dev *dev) -{ - struct smi_info *info = pnp_get_drvdata(dev); - - cleanup_one_si(info); -} - -static const struct pnp_device_id pnp_dev_table[] = { - {"IPI0001", 0}, - {"", 0}, -}; - -static struct pnp_driver ipmi_pnp_driver = { - .name = DEVICE_NAME, - .probe = ipmi_pnp_probe, - .remove = ipmi_pnp_remove, - .id_table = pnp_dev_table, -}; - -MODULE_DEVICE_TABLE(pnp, pnp_dev_table); #endif #ifdef CONFIG_DMI @@ -2669,10 +2537,19 @@ static struct pci_driver ipmi_pci_driver = { }; #endif /* CONFIG_PCI */ -static const struct of_device_id ipmi_match[]; -static int ipmi_probe(struct platform_device *dev) -{ #ifdef CONFIG_OF +static const struct of_device_id of_ipmi_match[] = { + { .type = "ipmi", .compatible = "ipmi-kcs", + .data = (void *)(unsigned long) SI_KCS }, + { .type = "ipmi", .compatible = "ipmi-smic", + .data = (void *)(unsigned long) SI_SMIC }, + { .type = "ipmi", .compatible = "ipmi-bt", + .data = (void *)(unsigned long) SI_BT }, + {}, +}; + +static int of_ipmi_probe(struct platform_device *dev) +{ const struct of_device_id *match; struct smi_info *info; struct resource resource; @@ -2683,9 +2560,9 @@ static int ipmi_probe(struct platform_device *dev) dev_info(&dev->dev, "probing via device tree\n"); - match = of_match_device(ipmi_match, &dev->dev); + match = of_match_device(of_ipmi_match, &dev->dev); if (!match) - return -EINVAL; + return -ENODEV; if (!of_device_is_available(np)) return -EINVAL; @@ -2754,33 +2631,161 @@ static int ipmi_probe(struct platform_device *dev) kfree(info); return ret; } -#endif return 0; } +#else +#define of_ipmi_match NULL +static int of_ipmi_probe(struct platform_device *dev) +{ + return -ENODEV; +} +#endif -static int ipmi_remove(struct platform_device *dev) +#ifdef CONFIG_ACPI +static int acpi_ipmi_probe(struct platform_device *dev) { -#ifdef CONFIG_OF - cleanup_one_si(dev_get_drvdata(&dev->dev)); + struct smi_info *info; + struct resource *res, *res_second; + acpi_handle handle; + acpi_status status; + unsigned long long tmp; + int rv = -EINVAL; + + handle = ACPI_HANDLE(&dev->dev); + if (!handle) + return -ENODEV; + + info = smi_info_alloc(); + if (!info) + return -ENOMEM; + + info->addr_source = SI_ACPI; + dev_info(&dev->dev, PFX "probing via ACPI\n"); + + info->addr_info.acpi_info.acpi_handle = handle; + + /* _IFT tells us the interface type: KCS, BT, etc */ + status = acpi_evaluate_integer(handle, "_IFT", NULL, &tmp); + if (ACPI_FAILURE(status)) { + dev_err(&dev->dev, "Could not find ACPI IPMI interface type\n"); + goto err_free; + } + + switch (tmp) { + case 1: + info->si_type = SI_KCS; + break; + case 2: + info->si_type = SI_SMIC; + break; + case 3: + info->si_type = SI_BT; + break; + case 4: /* SSIF, just ignore */ + rv = -ENODEV; + goto err_free; + default: + dev_info(&dev->dev, "unknown IPMI type %lld\n", tmp); + goto err_free; + } + + res = platform_get_resource(dev, IORESOURCE_IO, 0); + if (res) { + info->io_setup = port_setup; + info->io.addr_type = IPMI_IO_ADDR_SPACE; + } else { + res = platform_get_resource(dev, IORESOURCE_MEM, 0); + if (res) { + info->io_setup = mem_setup; + info->io.addr_type = IPMI_MEM_ADDR_SPACE; + } + } + if (!res) { + dev_err(&dev->dev, "no I/O or memory address\n"); + goto err_free; + } + info->io.addr_data = res->start; + + info->io.regspacing = DEFAULT_REGSPACING; + res_second = platform_get_resource(dev, + (info->io.addr_type == IPMI_IO_ADDR_SPACE) ? + IORESOURCE_IO : IORESOURCE_MEM, + 1); + if (res_second) { + if (res_second->start > info->io.addr_data) + info->io.regspacing = + res_second->start - info->io.addr_data; + } + info->io.regsize = DEFAULT_REGSPACING; + info->io.regshift = 0; + + /* If _GPE exists, use it; otherwise use standard interrupts */ + status = acpi_evaluate_integer(handle, "_GPE", NULL, &tmp); + if (ACPI_SUCCESS(status)) { + info->irq = tmp; + info->irq_setup = acpi_gpe_irq_setup; + } else { + int irq = platform_get_irq(dev, 0); + + if (irq > 0) { + info->irq = irq; + info->irq_setup = std_irq_setup; + } + } + + info->dev = &dev->dev; + platform_set_drvdata(dev, info); + + dev_info(info->dev, "%pR regsize %d spacing %d irq %d\n", + res, info->io.regsize, info->io.regspacing, + info->irq); + + rv = add_smi(info); + if (rv) + kfree(info); + + return rv; + +err_free: + kfree(info); + return rv; +} + +static struct acpi_device_id acpi_ipmi_match[] = { + { "IPI0001", 0 }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, acpi_ipmi_match); +#else +static int acpi_ipmi_probe(struct platform_device *dev) +{ + return -ENODEV; +} #endif - return 0; + +static int ipmi_probe(struct platform_device *dev) +{ + if (of_ipmi_probe(dev) == 0) + return 0; + + return acpi_ipmi_probe(dev); } -static const struct of_device_id ipmi_match[] = +static int ipmi_remove(struct platform_device *dev) { - { .type = "ipmi", .compatible = "ipmi-kcs", - .data = (void *)(unsigned long) SI_KCS }, - { .type = "ipmi", .compatible = "ipmi-smic", - .data = (void *)(unsigned long) SI_SMIC }, - { .type = "ipmi", .compatible = "ipmi-bt", - .data = (void *)(unsigned long) SI_BT }, - {}, -}; + struct smi_info *info = dev_get_drvdata(&dev->dev); + + if (info) + cleanup_one_si(info); + + return 0; +} static struct platform_driver ipmi_driver = { .driver = { .name = DEVICE_NAME, - .of_match_table = ipmi_match, + .of_match_table = of_ipmi_match, + .acpi_match_table = ACPI_PTR(acpi_ipmi_match), }, .probe = ipmi_probe, .remove = ipmi_remove, @@ -3692,13 +3697,6 @@ static int init_ipmi_si(void) } #endif -#ifdef CONFIG_ACPI - if (si_tryacpi) { - pnp_register_driver(&ipmi_pnp_driver); - pnp_registered = true; - } -#endif - #ifdef CONFIG_DMI if (si_trydmi) dmi_find_bmc(); @@ -3850,10 +3848,6 @@ static void cleanup_ipmi_si(void) if (pci_registered) pci_unregister_driver(&ipmi_pci_driver); #endif -#ifdef CONFIG_ACPI - if (pnp_registered) - pnp_unregister_driver(&ipmi_pnp_driver); -#endif #ifdef CONFIG_PARISC if (parisc_registered) unregister_parisc_driver(&ipmi_parisc_driver); -- cgit v0.10.2 From cca85f19c260df495a487495479c67803b25fa8a Mon Sep 17 00:00:00 2001 From: Neelesh Gupta Date: Thu, 16 Jul 2015 16:46:54 +0530 Subject: ipmi/powernv: Fix potential invalid pointer dereference If the OPAL call to receive the ipmi message fails, then we free up the smi message and return. But, the driver still holds the reference to old smi message in the 'cur_msg' which can potentially be accessed later and freed again leading to kernel oops. To fix it up, The kernel driver should reset the 'cur_msg' and send reply to the user in addition to freeing the message. Signed-off-by: Neelesh Gupta Fixed a checkpatch warning dealing with an else after a return. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c index 9b409c0..62c0c63 100644 --- a/drivers/char/ipmi/ipmi_powernv.c +++ b/drivers/char/ipmi/ipmi_powernv.c @@ -143,8 +143,15 @@ static int ipmi_powernv_recv(struct ipmi_smi_powernv *smi) pr_devel("%s: -> %d (size %lld)\n", __func__, rc, rc == 0 ? size : 0); if (rc) { + /* If came via the poll, and response was not yet ready */ + if (rc == OPAL_EMPTY) { + spin_unlock_irqrestore(&smi->msg_lock, flags); + return 0; + } + + smi->cur_msg = NULL; spin_unlock_irqrestore(&smi->msg_lock, flags); - ipmi_free_smi_msg(msg); + send_error_reply(smi, msg, IPMI_ERR_UNSPECIFIED); return 0; } -- cgit v0.10.2 From b2234ee9fc059c17e811a365383e3412a2f50bed Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 19 Feb 2015 11:29:24 -0600 Subject: ipmi: Add a comment in how messages are delivered from the lower layer To avoid confusion in the future. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index bf75f63..ef4a418 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -3959,6 +3959,10 @@ free_msg: if (!run_to_completion) spin_lock_irqsave(&intf->xmit_msgs_lock, flags); + /* + * We can get an asynchronous event or receive message in addition + * to commands we send. + */ if (msg == intf->curr_msg) intf->curr_msg = NULL; if (!run_to_completion) -- cgit v0.10.2 From 881c585ef79addb2440a7f8d59bda5640d0ff623 Mon Sep 17 00:00:00 2001 From: Nicholas Krause Date: Sat, 9 May 2015 15:32:28 -0400 Subject: impi:Remove unneeded setting of module owner to THIS_MODULE in the platform structure, powernv_ipmi_driver This removes the no longer required setting of the module owner for the plaform structure,powernv_ipmi_driver to THIS_MODULE as the driver core for ipmi drivers will directly find and set the module owner for this driver. Signed-off-by: Nicholas Krause Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c index 62c0c63..6e658aa 100644 --- a/drivers/char/ipmi/ipmi_powernv.c +++ b/drivers/char/ipmi/ipmi_powernv.c @@ -307,7 +307,6 @@ static const struct of_device_id ipmi_powernv_match[] = { static struct platform_driver powernv_ipmi_driver = { .driver = { .name = "ipmi-powernv", - .owner = THIS_MODULE, .of_match_table = ipmi_powernv_match, }, .probe = ipmi_powernv_probe, -- cgit v0.10.2 From fedb25ea903d3520b6cff00c41740a892cf6bc0e Mon Sep 17 00:00:00 2001 From: Shailendra Verma Date: Tue, 26 May 2015 00:54:57 +0530 Subject: char:ipmi - Change 1 to true for bool type variables during initialization. Signed-off-by: Shailendra Verma Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 6443e76..b1fdbf7 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -1279,14 +1279,14 @@ static int smi_num; /* Used to sequence the SMIs */ #define DEFAULT_REGSIZE 1 #ifdef CONFIG_ACPI -static bool si_tryacpi = 1; +static bool si_tryacpi = true; #endif #ifdef CONFIG_DMI -static bool si_trydmi = 1; +static bool si_trydmi = true; #endif -static bool si_tryplatform = 1; +static bool si_tryplatform = true; #ifdef CONFIG_PCI -static bool si_trypci = 1; +static bool si_trypci = true; #endif static bool si_trydefaults = IS_ENABLED(CONFIG_IPMI_SI_PROBE_DEFAULTS); static char *si_type[SI_MAX_PARMS]; diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 207689c..428a5fa 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1154,11 +1154,11 @@ static int use_thread; module_param(use_thread, int, 0); MODULE_PARM_DESC(use_thread, "Use the thread interface."); -static bool ssif_tryacpi = 1; +static bool ssif_tryacpi = true; module_param_named(tryacpi, ssif_tryacpi, bool, 0); MODULE_PARM_DESC(tryacpi, "Setting this to zero will disable the default scan of the interfaces identified via ACPI"); -static bool ssif_trydmi = 1; +static bool ssif_trydmi = true; module_param_named(trydmi, ssif_trydmi, bool, 0); MODULE_PARM_DESC(trydmi, "Setting this to zero will disable the default scan of the interfaces identified via DMI (SMBIOS)"); -- cgit v0.10.2 From a7930899ca0082a33350b253c6ed34f67255f98e Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 27 Jun 2015 18:12:14 +0200 Subject: ipmi: Delete an unnecessary check before the function call "cleanup_one_si" The cleanup_one_si() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index b1fdbf7..4387bd6 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2775,9 +2775,7 @@ static int ipmi_remove(struct platform_device *dev) { struct smi_info *info = dev_get_drvdata(&dev->dev); - if (info) - cleanup_one_si(info); - + cleanup_one_si(info); return 0; } -- cgit v0.10.2 From 5186cf9c74034a4a7856de9c8048493be34c457d Mon Sep 17 00:00:00 2001 From: Mathias Krause Date: Sat, 13 Jun 2015 14:19:33 +0200 Subject: ipmi: constify SSIF ACPI device ids Constify the ACPI device ID array, it doesn't need to be writable at runtime. Signed-off-by: Mathias Krause Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index 428a5fa..b043d8d 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1787,7 +1787,7 @@ skip_addr: } #ifdef CONFIG_ACPI -static struct acpi_device_id ssif_acpi_match[] = { +static const struct acpi_device_id ssif_acpi_match[] = { { "IPI0001", 0 }, { }, }; -- cgit v0.10.2 From 81d02b7f8c507f06299476a0e5b2aa677c5eaecb Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Sat, 13 Jun 2015 10:34:25 -0500 Subject: ipmi: Make some data const that was only read Several data structures were only used for reading, so make them const. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c index 61e7161..feafdab 100644 --- a/drivers/char/ipmi/ipmi_bt_sm.c +++ b/drivers/char/ipmi/ipmi_bt_sm.c @@ -694,7 +694,7 @@ static int bt_size(void) return sizeof(struct si_sm_data); } -struct si_sm_handlers bt_smi_handlers = { +const struct si_sm_handlers bt_smi_handlers = { .init_data = bt_init_data, .start_transaction = bt_start_transaction, .get_result = bt_get_result, diff --git a/drivers/char/ipmi/ipmi_kcs_sm.c b/drivers/char/ipmi/ipmi_kcs_sm.c index 8c25f59..1da61af 100644 --- a/drivers/char/ipmi/ipmi_kcs_sm.c +++ b/drivers/char/ipmi/ipmi_kcs_sm.c @@ -540,7 +540,7 @@ static void kcs_cleanup(struct si_sm_data *kcs) { } -struct si_sm_handlers kcs_smi_handlers = { +const struct si_sm_handlers kcs_smi_handlers = { .init_data = init_kcs_data, .start_transaction = start_kcs_transaction, .get_result = get_kcs_result, diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index ef4a418..e9ea29c 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -342,7 +342,7 @@ struct ipmi_smi { * an umpreemptible region to use this. You must fetch the * value into a local variable and make sure it is not NULL. */ - struct ipmi_smi_handlers *handlers; + const struct ipmi_smi_handlers *handlers; void *send_info; #ifdef CONFIG_PROC_FS @@ -1015,7 +1015,7 @@ int ipmi_get_smi_info(int if_num, struct ipmi_smi_info *data) { int rv = 0; ipmi_smi_t intf; - struct ipmi_smi_handlers *handlers; + const struct ipmi_smi_handlers *handlers; mutex_lock(&ipmi_interfaces_mutex); list_for_each_entry_rcu(intf, &ipmi_interfaces, link) { @@ -1501,7 +1501,7 @@ static struct ipmi_smi_msg *smi_add_send_msg(ipmi_smi_t intf, } -static void smi_send(ipmi_smi_t intf, struct ipmi_smi_handlers *handlers, +static void smi_send(ipmi_smi_t intf, const struct ipmi_smi_handlers *handlers, struct ipmi_smi_msg *smi_msg, int priority) { int run_to_completion = intf->run_to_completion; @@ -2747,7 +2747,7 @@ void ipmi_poll_interface(ipmi_user_t user) } EXPORT_SYMBOL(ipmi_poll_interface); -int ipmi_register_smi(struct ipmi_smi_handlers *handlers, +int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, void *send_info, struct ipmi_device_id *device_id, struct device *si_dev, @@ -4019,7 +4019,7 @@ static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, unsigned int *waiting_msgs) { struct ipmi_recv_msg *msg; - struct ipmi_smi_handlers *handlers; + const struct ipmi_smi_handlers *handlers; if (intf->in_shutdown) return; @@ -4086,7 +4086,7 @@ static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent, ipmi_inc_stat(intf, retransmitted_ipmb_commands); - smi_send(intf, intf->handlers, smi_msg, 0); + smi_send(intf, handlers, smi_msg, 0); } else ipmi_free_smi_msg(smi_msg); diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 4387bd6..4a4a13d 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -163,7 +163,7 @@ struct smi_info { int intf_num; ipmi_smi_t intf; struct si_sm_data *si_sm; - struct si_sm_handlers *handlers; + const struct si_sm_handlers *handlers; enum si_type si_type; spinlock_t si_lock; struct ipmi_smi_msg *waiting_msg; @@ -1254,7 +1254,7 @@ static void set_maintenance_mode(void *send_info, bool enable) atomic_set(&smi_info->req_events, 0); } -static struct ipmi_smi_handlers handlers = { +static const struct ipmi_smi_handlers handlers = { .owner = THIS_MODULE, .start_processing = smi_start_processing, .get_smi_info = get_smi_info, @@ -1442,14 +1442,14 @@ static int std_irq_setup(struct smi_info *info) return rv; } -static unsigned char port_inb(struct si_sm_io *io, unsigned int offset) +static unsigned char port_inb(const struct si_sm_io *io, unsigned int offset) { unsigned int addr = io->addr_data; return inb(addr + (offset * io->regspacing)); } -static void port_outb(struct si_sm_io *io, unsigned int offset, +static void port_outb(const struct si_sm_io *io, unsigned int offset, unsigned char b) { unsigned int addr = io->addr_data; @@ -1457,14 +1457,14 @@ static void port_outb(struct si_sm_io *io, unsigned int offset, outb(b, addr + (offset * io->regspacing)); } -static unsigned char port_inw(struct si_sm_io *io, unsigned int offset) +static unsigned char port_inw(const struct si_sm_io *io, unsigned int offset) { unsigned int addr = io->addr_data; return (inw(addr + (offset * io->regspacing)) >> io->regshift) & 0xff; } -static void port_outw(struct si_sm_io *io, unsigned int offset, +static void port_outw(const struct si_sm_io *io, unsigned int offset, unsigned char b) { unsigned int addr = io->addr_data; @@ -1472,14 +1472,14 @@ static void port_outw(struct si_sm_io *io, unsigned int offset, outw(b << io->regshift, addr + (offset * io->regspacing)); } -static unsigned char port_inl(struct si_sm_io *io, unsigned int offset) +static unsigned char port_inl(const struct si_sm_io *io, unsigned int offset) { unsigned int addr = io->addr_data; return (inl(addr + (offset * io->regspacing)) >> io->regshift) & 0xff; } -static void port_outl(struct si_sm_io *io, unsigned int offset, +static void port_outl(const struct si_sm_io *io, unsigned int offset, unsigned char b) { unsigned int addr = io->addr_data; @@ -1552,49 +1552,52 @@ static int port_setup(struct smi_info *info) return 0; } -static unsigned char intf_mem_inb(struct si_sm_io *io, unsigned int offset) +static unsigned char intf_mem_inb(const struct si_sm_io *io, + unsigned int offset) { return readb((io->addr)+(offset * io->regspacing)); } -static void intf_mem_outb(struct si_sm_io *io, unsigned int offset, - unsigned char b) +static void intf_mem_outb(const struct si_sm_io *io, unsigned int offset, + unsigned char b) { writeb(b, (io->addr)+(offset * io->regspacing)); } -static unsigned char intf_mem_inw(struct si_sm_io *io, unsigned int offset) +static unsigned char intf_mem_inw(const struct si_sm_io *io, + unsigned int offset) { return (readw((io->addr)+(offset * io->regspacing)) >> io->regshift) & 0xff; } -static void intf_mem_outw(struct si_sm_io *io, unsigned int offset, - unsigned char b) +static void intf_mem_outw(const struct si_sm_io *io, unsigned int offset, + unsigned char b) { writeb(b << io->regshift, (io->addr)+(offset * io->regspacing)); } -static unsigned char intf_mem_inl(struct si_sm_io *io, unsigned int offset) +static unsigned char intf_mem_inl(const struct si_sm_io *io, + unsigned int offset) { return (readl((io->addr)+(offset * io->regspacing)) >> io->regshift) & 0xff; } -static void intf_mem_outl(struct si_sm_io *io, unsigned int offset, - unsigned char b) +static void intf_mem_outl(const struct si_sm_io *io, unsigned int offset, + unsigned char b) { writel(b << io->regshift, (io->addr)+(offset * io->regspacing)); } #ifdef readq -static unsigned char mem_inq(struct si_sm_io *io, unsigned int offset) +static unsigned char mem_inq(const struct si_sm_io *io, unsigned int offset) { return (readq((io->addr)+(offset * io->regspacing)) >> io->regshift) & 0xff; } -static void mem_outq(struct si_sm_io *io, unsigned int offset, +static void mem_outq(const struct si_sm_io *io, unsigned int offset, unsigned char b) { writeq(b << io->regshift, (io->addr)+(offset * io->regspacing)); @@ -2522,7 +2525,7 @@ static void ipmi_pci_remove(struct pci_dev *pdev) pci_disable_device(pdev); } -static struct pci_device_id ipmi_pci_devices[] = { +static const struct pci_device_id ipmi_pci_devices[] = { { PCI_DEVICE(PCI_HP_VENDOR_ID, PCI_MMC_DEVICE_ID) }, { PCI_DEVICE_CLASS(PCI_ERMC_CLASSCODE, PCI_ERMC_CLASSCODE_MASK) }, { 0, } @@ -2751,7 +2754,7 @@ err_free: return rv; } -static struct acpi_device_id acpi_ipmi_match[] = { +static const struct acpi_device_id acpi_ipmi_match[] = { { "IPI0001", 0 }, { }, }; @@ -3324,7 +3327,7 @@ static inline void wait_for_timer_and_thread(struct smi_info *smi_info) del_timer_sync(&smi_info->si_timer); } -static struct ipmi_default_vals +static const struct ipmi_default_vals { int type; int port; diff --git a/drivers/char/ipmi/ipmi_si_sm.h b/drivers/char/ipmi/ipmi_si_sm.h index df89f73..a705027 100644 --- a/drivers/char/ipmi/ipmi_si_sm.h +++ b/drivers/char/ipmi/ipmi_si_sm.h @@ -46,8 +46,8 @@ struct si_sm_data; * this interface. */ struct si_sm_io { - unsigned char (*inputb)(struct si_sm_io *io, unsigned int offset); - void (*outputb)(struct si_sm_io *io, + unsigned char (*inputb)(const struct si_sm_io *io, unsigned int offset); + void (*outputb)(const struct si_sm_io *io, unsigned int offset, unsigned char b); @@ -135,7 +135,7 @@ struct si_sm_handlers { }; /* Current state machines that we can use. */ -extern struct si_sm_handlers kcs_smi_handlers; -extern struct si_sm_handlers smic_smi_handlers; -extern struct si_sm_handlers bt_smi_handlers; +extern const struct si_sm_handlers kcs_smi_handlers; +extern const struct si_sm_handlers smic_smi_handlers; +extern const struct si_sm_handlers bt_smi_handlers; diff --git a/drivers/char/ipmi/ipmi_smic_sm.c b/drivers/char/ipmi/ipmi_smic_sm.c index c8e77af..8f7c73f 100644 --- a/drivers/char/ipmi/ipmi_smic_sm.c +++ b/drivers/char/ipmi/ipmi_smic_sm.c @@ -589,7 +589,7 @@ static int smic_size(void) return sizeof(struct si_sm_data); } -struct si_sm_handlers smic_smi_handlers = { +const struct si_sm_handlers smic_smi_handlers = { .init_data = init_smic_data, .start_transaction = start_smic_transaction, .get_result = smic_get_result, diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 0b1e569..41de0cf 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -207,7 +207,7 @@ static inline int ipmi_demangle_device_id(const unsigned char *data, upper layer until the start_processing() function in the handlers is called, and the lower layer must get the interface from that call. */ -int ipmi_register_smi(struct ipmi_smi_handlers *handlers, +int ipmi_register_smi(const struct ipmi_smi_handlers *handlers, void *send_info, struct ipmi_device_id *device_id, struct device *dev, -- cgit v0.10.2 From b0868dd5c17c0d9cc8919e786db2e428aa225621 Mon Sep 17 00:00:00 2001 From: Hidehiro Kawai Date: Mon, 27 Jul 2015 14:55:16 +0900 Subject: ipmi: Remove unneeded set_run_to_completion call send_panic_events() calls intf->handlers->set_run_to_completion(), but it has already been done in the caller function panic_event(). Remove it from send_panic_events(). Signed-off-by: Hidehiro Kawai Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index e9ea29c..5e31c33 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -4368,9 +4368,7 @@ static void send_panic_events(char *str) /* Interface is not ready. */ continue; - intf->run_to_completion = 1; /* Send the event announcing the panic. */ - intf->handlers->set_run_to_completion(intf->send_info, 1); ipmi_panic_request_and_wait(intf, &addr, &msg); } -- cgit v0.10.2 From e45361d733d0a1432b0f6307375045e66ac02489 Mon Sep 17 00:00:00 2001 From: Hidehiro Kawai Date: Mon, 27 Jul 2015 14:55:16 +0900 Subject: ipmi: Factor out message flushing procedure Factor out message flushing procedure which is used in run-to-completion mode. This patch doesn't change the logic. Signed-off-by: Hidehiro Kawai Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 4a4a13d..5bd6d5b 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -924,11 +924,25 @@ static void check_start_timer_thread(struct smi_info *smi_info) } } +static void flush_messages(struct smi_info *smi_info) +{ + enum si_sm_result result; + + /* + * Currently, this function is called only in run-to-completion + * mode. This means we are single-threaded, no need for locks. + */ + result = smi_event_handler(smi_info, 0); + while (result != SI_SM_IDLE) { + udelay(SI_SHORT_TIMEOUT_USEC); + result = smi_event_handler(smi_info, SI_SHORT_TIMEOUT_USEC); + } +} + static void sender(void *send_info, struct ipmi_smi_msg *msg) { struct smi_info *smi_info = send_info; - enum si_sm_result result; unsigned long flags; debug_timestamp("Enqueue"); @@ -940,17 +954,7 @@ static void sender(void *send_info, */ smi_info->waiting_msg = msg; - /* - * Run to completion means we are single-threaded, no - * need for locks. - */ - - result = smi_event_handler(smi_info, 0); - while (result != SI_SM_IDLE) { - udelay(SI_SHORT_TIMEOUT_USEC); - result = smi_event_handler(smi_info, - SI_SHORT_TIMEOUT_USEC); - } + flush_messages(smi_info); return; } @@ -971,17 +975,10 @@ static void sender(void *send_info, static void set_run_to_completion(void *send_info, bool i_run_to_completion) { struct smi_info *smi_info = send_info; - enum si_sm_result result; smi_info->run_to_completion = i_run_to_completion; - if (i_run_to_completion) { - result = smi_event_handler(smi_info, 0); - while (result != SI_SM_IDLE) { - udelay(SI_SHORT_TIMEOUT_USEC); - result = smi_event_handler(smi_info, - SI_SHORT_TIMEOUT_USEC); - } - } + if (i_run_to_completion) + flush_messages(smi_info); } /* -- cgit v0.10.2 From 82802f968bd3118af04eaeb3814c21d9813be527 Mon Sep 17 00:00:00 2001 From: Hidehiro Kawai Date: Mon, 27 Jul 2015 14:55:16 +0900 Subject: ipmi: Don't flush messages in sender() in run-to-completion mode When flushing queued messages in run-to-completion mode, smi_event_handler() is recursively called. flush_messages() smi_event_handler() handle_transaction_done() deliver_recv_msg() ipmi_smi_msg_received() smi_recv_tasklet() sender() flush_messages() smi_event_handler() ... The depth of the recursive call depends on the number of queued messages, so it can cause a stack overflow if many messages have been queued. To solve this problem, this patch removes flush_messages() from sender()@ipmi_si_intf.c. Instead, add flush_messages() to caller side of sender() if needed. Additionally, to implement this, add new handler flush_messages to struct ipmi_smi_handlers. Signed-off-by: Hidehiro Kawai Fixed up a comment and some spacing issues. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 5e31c33..6e191ff 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -4295,6 +4295,9 @@ static void ipmi_panic_request_and_wait(ipmi_smi_t intf, 0, 1); /* Don't retry, and don't wait. */ if (rv) atomic_sub(2, &panic_done_count); + else if (intf->handlers->flush_messages) + intf->handlers->flush_messages(intf->send_info); + while (atomic_read(&panic_done_count) != 0) ipmi_poll(intf); } diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 5bd6d5b..2f4cf6e 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -924,8 +924,9 @@ static void check_start_timer_thread(struct smi_info *smi_info) } } -static void flush_messages(struct smi_info *smi_info) +static void flush_messages(void *send_info) { + struct smi_info *smi_info = send_info; enum si_sm_result result; /* @@ -949,12 +950,10 @@ static void sender(void *send_info, if (smi_info->run_to_completion) { /* - * If we are running to completion, start it and run - * transactions until everything is clear. + * If we are running to completion, start it. Upper + * layer will call flush_messages to clear it out. */ smi_info->waiting_msg = msg; - - flush_messages(smi_info); return; } @@ -1260,6 +1259,7 @@ static const struct ipmi_smi_handlers handlers = { .set_need_watch = set_need_watch, .set_maintenance_mode = set_maintenance_mode, .set_run_to_completion = set_run_to_completion, + .flush_messages = flush_messages, .poll = poll, }; diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h index 41de0cf..f8cea14 100644 --- a/include/linux/ipmi_smi.h +++ b/include/linux/ipmi_smi.h @@ -115,6 +115,11 @@ struct ipmi_smi_handlers { implement it. */ void (*set_need_watch)(void *send_info, bool enable); + /* + * Called when flushing all pending messages. + */ + void (*flush_messages)(void *send_info); + /* Called when the interface should go into "run to completion" mode. If this call sets the value to true, the interface should make sure that all messages are flushed -- cgit v0.10.2 From 06e5e345fea8df24b1d935f98741343df4cab664 Mon Sep 17 00:00:00 2001 From: Hidehiro Kawai Date: Mon, 27 Jul 2015 14:55:16 +0900 Subject: ipmi: Avoid touching possible corrupted lists in the panic context When processing queued messages in the panic context, IPMI driver tries to do it without any locking to avoid deadlocks. However, this means we can touch a corrupted list if the kernel panicked while manipulating the list. Fortunately, current `add-tail and del-from-head' style implementation won't touch the corrupted part, but it is inherently risky. To get rid of the risk, this patch re-initializes the message lists on panic if the related spinlock has already been acquired. As the result, we may lose queued messages, but it's not so painful. Dropping messages on the received message list is also less problematic because no one can respond the received messages. Signed-off-by: Hidehiro Kawai Fixed a comment typo. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index 6e191ff..cdac5f7 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -4511,6 +4511,23 @@ static int panic_event(struct notifier_block *this, /* Interface is not ready. */ continue; + /* + * If we were interrupted while locking xmit_msgs_lock or + * waiting_rcv_msgs_lock, the corresponding list may be + * corrupted. In this case, drop items on the list for + * the safety. + */ + if (!spin_trylock(&intf->xmit_msgs_lock)) { + INIT_LIST_HEAD(&intf->xmit_msgs); + INIT_LIST_HEAD(&intf->hp_xmit_msgs); + } else + spin_unlock(&intf->xmit_msgs_lock); + + if (!spin_trylock(&intf->waiting_rcv_msgs_lock)) + INIT_LIST_HEAD(&intf->waiting_rcv_msgs); + else + spin_unlock(&intf->waiting_rcv_msgs_lock); + intf->run_to_completion = 1; intf->handlers->set_run_to_completion(intf->send_info, 1); } -- cgit v0.10.2 From c49c097610fe1aabf86111297280a718abb5dcc2 Mon Sep 17 00:00:00 2001 From: Hidehiro Kawai Date: Mon, 27 Jul 2015 14:55:16 +0900 Subject: ipmi: Don't call receive handler in the panic context Received handlers defined as ipmi_recv_hndl member of struct ipmi_user_hndl can take a spinlock. This means that if the kernel panics while holding the lock, a deadlock may happen on the lock while flushing queued messages in the panic context. Calling the receive handler doesn't make much meanings in the panic context, simply skip it to avoid possible deadlocks. Signed-off-by: Hidehiro Kawai Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c index cdac5f7..e3536da 100644 --- a/drivers/char/ipmi/ipmi_msghandler.c +++ b/drivers/char/ipmi/ipmi_msghandler.c @@ -744,7 +744,13 @@ static void deliver_response(struct ipmi_recv_msg *msg) ipmi_inc_stat(intf, unhandled_local_responses); } ipmi_free_recv_msg(msg); - } else { + } else if (!oops_in_progress) { + /* + * If we are running in the panic context, calling the + * receive handler doesn't much meaning and has a deadlock + * risk. At this moment, simply skip it in that case. + */ + ipmi_user_t user = msg->user; user->handler->ipmi_recv_hndl(msg, user->handler_data); } -- cgit v0.10.2 From d08828973d96eb26e48fb7ca8fb8a8d49adbe53a Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 18 Aug 2015 14:29:10 -0500 Subject: ipmi: Compensate for BMCs that wont set the irq enable bit It appears that some BMCs support interrupts but don't support setting the irq enable bits. The interrupts are just always on. Sigh. Add code to compensate. The new code was very similar to another functions, so this also factors out the common code into other functions. Signed-off-by: Corey Minyard Tested-by: Henrik Korkuc diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 2f4cf6e..21bddc1 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -262,9 +262,21 @@ struct smi_info { bool supports_event_msg_buff; /* - * Can we clear the global enables receive irq bit? + * Can we disable interrupts the global enables receive irq + * bit? There are currently two forms of brokenness, some + * systems cannot disable the bit (which is technically within + * the spec but a bad idea) and some systems have the bit + * forced to zero even though interrupts work (which is + * clearly outside the spec). The next bool tells which form + * of brokenness is present. */ - bool cannot_clear_recv_irq_bit; + bool cannot_disable_irq; + + /* + * Some systems are broken and cannot set the irq enable + * bit, even if they support interrupts. + */ + bool irq_enable_broken; /* * Did we get an attention that we did not handle? @@ -554,13 +566,14 @@ static u8 current_global_enables(struct smi_info *smi_info, u8 base, if (smi_info->supports_event_msg_buff) enables |= IPMI_BMC_EVT_MSG_BUFF; - if ((smi_info->irq && !smi_info->interrupt_disabled) || - smi_info->cannot_clear_recv_irq_bit) + if (((smi_info->irq && !smi_info->interrupt_disabled) || + smi_info->cannot_disable_irq) && + !smi_info->irq_enable_broken) enables |= IPMI_BMC_RCV_MSG_INTR; if (smi_info->supports_event_msg_buff && - smi_info->irq && !smi_info->interrupt_disabled) - + smi_info->irq && !smi_info->interrupt_disabled && + !smi_info->irq_enable_broken) enables |= IPMI_BMC_EVT_MSG_INTR; *irq_on = enables & (IPMI_BMC_EVT_MSG_INTR | IPMI_BMC_RCV_MSG_INTR); @@ -2908,12 +2921,7 @@ static int try_get_dev_id(struct smi_info *smi_info) return rv; } -/* - * Some BMCs do not support clearing the receive irq bit in the global - * enables (even if they don't support interrupts on the BMC). Check - * for this and handle it properly. - */ -static void check_clr_rcv_irq(struct smi_info *smi_info) +static int get_global_enables(struct smi_info *smi_info, u8 *enables) { unsigned char msg[3]; unsigned char *resp; @@ -2921,12 +2929,8 @@ static void check_clr_rcv_irq(struct smi_info *smi_info) int rv; resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL); - if (!resp) { - printk(KERN_WARNING PFX "Out of memory allocating response for" - " global enables command, cannot check recv irq bit" - " handling.\n"); - return; - } + if (!resp) + return -ENOMEM; msg[0] = IPMI_NETFN_APP_REQUEST << 2; msg[1] = IPMI_GET_BMC_GLOBAL_ENABLES_CMD; @@ -2934,9 +2938,9 @@ static void check_clr_rcv_irq(struct smi_info *smi_info) rv = wait_for_msg_done(smi_info); if (rv) { - printk(KERN_WARNING PFX "Error getting response from get" - " global enables command, cannot check recv irq bit" - " handling.\n"); + dev_warn(smi_info->dev, + "Error getting response from get global enables command: %d\n", + rv); goto out; } @@ -2947,27 +2951,44 @@ static void check_clr_rcv_irq(struct smi_info *smi_info) resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || resp[1] != IPMI_GET_BMC_GLOBAL_ENABLES_CMD || resp[2] != 0) { - printk(KERN_WARNING PFX "Invalid return from get global" - " enables command, cannot check recv irq bit" - " handling.\n"); + dev_warn(smi_info->dev, + "Invalid return from get global enables command: %ld %x %x %x\n", + resp_len, resp[0], resp[1], resp[2]); rv = -EINVAL; goto out; + } else { + *enables = resp[3]; } - if ((resp[3] & IPMI_BMC_RCV_MSG_INTR) == 0) - /* Already clear, should work ok. */ - goto out; +out: + kfree(resp); + return rv; +} + +/* + * Returns 1 if it gets an error from the command. + */ +static int set_global_enables(struct smi_info *smi_info, u8 enables) +{ + unsigned char msg[3]; + unsigned char *resp; + unsigned long resp_len; + int rv; + + resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL); + if (!resp) + return -ENOMEM; msg[0] = IPMI_NETFN_APP_REQUEST << 2; msg[1] = IPMI_SET_BMC_GLOBAL_ENABLES_CMD; - msg[2] = resp[3] & ~IPMI_BMC_RCV_MSG_INTR; + msg[2] = enables; smi_info->handlers->start_transaction(smi_info->si_sm, msg, 3); rv = wait_for_msg_done(smi_info); if (rv) { - printk(KERN_WARNING PFX "Error getting response from set" - " global enables command, cannot check recv irq bit" - " handling.\n"); + dev_warn(smi_info->dev, + "Error getting response from set global enables command: %d\n", + rv); goto out; } @@ -2977,25 +2998,93 @@ static void check_clr_rcv_irq(struct smi_info *smi_info) if (resp_len < 3 || resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 || resp[1] != IPMI_SET_BMC_GLOBAL_ENABLES_CMD) { - printk(KERN_WARNING PFX "Invalid return from get global" - " enables command, cannot check recv irq bit" - " handling.\n"); + dev_warn(smi_info->dev, + "Invalid return from set global enables command: %ld %x %x\n", + resp_len, resp[0], resp[1]); rv = -EINVAL; goto out; } - if (resp[2] != 0) { + if (resp[2] != 0) + rv = 1; + +out: + kfree(resp); + return rv; +} + +/* + * Some BMCs do not support clearing the receive irq bit in the global + * enables (even if they don't support interrupts on the BMC). Check + * for this and handle it properly. + */ +static void check_clr_rcv_irq(struct smi_info *smi_info) +{ + u8 enables = 0; + int rv; + + rv = get_global_enables(smi_info, &enables); + if (!rv) { + if ((enables & IPMI_BMC_RCV_MSG_INTR) == 0) + /* Already clear, should work ok. */ + return; + + enables &= ~IPMI_BMC_RCV_MSG_INTR; + rv = set_global_enables(smi_info, enables); + } + + if (rv < 0) { + dev_err(smi_info->dev, + "Cannot check clearing the rcv irq: %d\n", rv); + return; + } + + if (rv) { /* * An error when setting the event buffer bit means * clearing the bit is not supported. */ - printk(KERN_WARNING PFX "The BMC does not support clearing" - " the recv irq bit, compensating, but the BMC needs to" - " be fixed.\n"); - smi_info->cannot_clear_recv_irq_bit = true; + dev_warn(smi_info->dev, + "The BMC does not support clearing the recv irq bit, compensating, but the BMC needs to be fixed.\n"); + smi_info->cannot_disable_irq = true; + } +} + +/* + * Some BMCs do not support setting the interrupt bits in the global + * enables even if they support interrupts. Clearly bad, but we can + * compensate. + */ +static void check_set_rcv_irq(struct smi_info *smi_info) +{ + u8 enables = 0; + int rv; + + if (!smi_info->irq) + return; + + rv = get_global_enables(smi_info, &enables); + if (!rv) { + enables |= IPMI_BMC_RCV_MSG_INTR; + rv = set_global_enables(smi_info, enables); + } + + if (rv < 0) { + dev_err(smi_info->dev, + "Cannot check setting the rcv irq: %d\n", rv); + return; + } + + if (rv) { + /* + * An error when setting the event buffer bit means + * setting the bit is not supported. + */ + dev_warn(smi_info->dev, + "The BMC does not support setting the recv irq bit, compensating, but the BMC needs to be fixed.\n"); + smi_info->cannot_disable_irq = true; + smi_info->irq_enable_broken = true; } - out: - kfree(resp); } static int try_enable_event_buffer(struct smi_info *smi_info) @@ -3316,6 +3405,12 @@ static void setup_xaction_handlers(struct smi_info *smi_info) setup_dell_poweredge_bt_xaction_handler(smi_info); } +static void check_for_broken_irqs(struct smi_info *smi_info) +{ + check_clr_rcv_irq(smi_info); + check_set_rcv_irq(smi_info); +} + static inline void wait_for_timer_and_thread(struct smi_info *smi_info) { if (smi_info->thread != NULL) @@ -3493,10 +3588,9 @@ static int try_smi_init(struct smi_info *new_smi) goto out_err; } - check_clr_rcv_irq(new_smi); - setup_oem_data_handler(new_smi); setup_xaction_handlers(new_smi); + check_for_broken_irqs(new_smi); new_smi->waiting_msg = NULL; new_smi->curr_msg = NULL; -- cgit v0.10.2 From acbd9ae70a94bdc626508f444879e19ebe1c421f Mon Sep 17 00:00:00 2001 From: Brijesh Singh Date: Mon, 24 Aug 2015 09:15:25 -0500 Subject: ipmi: add of_device_id in MODULE_DEVICE_TABLE Fix autoloading ipmi modules when using device tree. Signed-off-by: Brijesh Singh Moved this change up into the CONFIG_OF section to account for changes to the probing code. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 21bddc1..654f6f3 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c @@ -2646,6 +2646,7 @@ static int of_ipmi_probe(struct platform_device *dev) } return 0; } +MODULE_DEVICE_TABLE(of, of_ipmi_match); #else #define of_ipmi_match NULL static int of_ipmi_probe(struct platform_device *dev) -- cgit v0.10.2 From bf2d087749d91e1fa2826edde1e2fd650d3053ca Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 27 Aug 2015 15:49:18 -0500 Subject: ipmi:ssif: Add a module parm to specify that SMBus alerts don't work They are broken on some platforms, this gives people a chance to work around it until the firmware is fixed. Signed-off-by: Corey Minyard diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c index b043d8d..877205d 100644 --- a/drivers/char/ipmi/ipmi_ssif.c +++ b/drivers/char/ipmi/ipmi_ssif.c @@ -1136,6 +1136,10 @@ module_param_array(slave_addrs, int, &num_slave_addrs, 0); MODULE_PARM_DESC(slave_addrs, "The default IPMB slave address for the controller."); +static bool alerts_broken; +module_param(alerts_broken, bool, 0); +MODULE_PARM_DESC(alerts_broken, "Don't enable alerts for the controller."); + /* * Bit 0 enables message debugging, bit 1 enables state debugging, and * bit 2 enables timing debugging. This is an array indexed by @@ -1582,6 +1586,10 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) ssif_info->global_enables |= IPMI_BMC_EVT_MSG_BUFF; } + /* Some systems don't behave well if you enable alerts. */ + if (alerts_broken) + goto found; + msg[0] = IPMI_NETFN_APP_REQUEST << 2; msg[1] = IPMI_SET_BMC_GLOBAL_ENABLES_CMD; msg[2] = ssif_info->global_enables | IPMI_BMC_RCV_MSG_INTR; -- cgit v0.10.2