From 3b0a6d1a1b3335fde9cbbc659568ed619b07d24a Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Thu, 4 Feb 2016 14:02:45 -0600 Subject: PCI/AER: Rename pci_ops_aer to aer_inj_pci_ops Rename pci_ops_aer to aer_inj_pci_ops pci_read_aer() to aer_inj_read_config() pci_write_aer() to aer_inj_write_config() This is more conventional and more informative. No functional change. Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aer_inject.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c index 20db790465dd..148a301a6b3c 100644 --- a/drivers/pci/pcie/aer/aer_inject.c +++ b/drivers/pci/pcie/aer/aer_inject.c @@ -181,8 +181,8 @@ static u32 *find_pci_config_dword(struct aer_error *err, int where, return target; } -static int pci_read_aer(struct pci_bus *bus, unsigned int devfn, int where, - int size, u32 *val) +static int aer_inj_read_config(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val) { u32 *sim; struct aer_error *err; @@ -212,8 +212,8 @@ out: return ops->read(bus, devfn, where, size, val); } -static int pci_write_aer(struct pci_bus *bus, unsigned int devfn, int where, - int size, u32 val) +static int aer_inj_write_config(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 val) { u32 *sim; struct aer_error *err; @@ -247,9 +247,9 @@ out: return ops->write(bus, devfn, where, size, val); } -static struct pci_ops pci_ops_aer = { - .read = pci_read_aer, - .write = pci_write_aer, +static struct pci_ops aer_inj_pci_ops = { + .read = aer_inj_read_config, + .write = aer_inj_write_config, }; static void pci_bus_ops_init(struct pci_bus_ops *bus_ops, @@ -270,9 +270,9 @@ static int pci_bus_set_aer_ops(struct pci_bus *bus) bus_ops = kmalloc(sizeof(*bus_ops), GFP_KERNEL); if (!bus_ops) return -ENOMEM; - ops = pci_bus_set_ops(bus, &pci_ops_aer); + ops = pci_bus_set_ops(bus, &aer_inj_pci_ops); spin_lock_irqsave(&inject_lock, flags); - if (ops == &pci_ops_aer) + if (ops == &aer_inj_pci_ops) goto out; pci_bus_ops_init(bus_ops, bus, ops); list_add(&bus_ops->list, &pci_bus_ops_list); -- cgit v1.2.3 From 7e8fbdc628760857369af05636ed4ddc4fc8569b Mon Sep 17 00:00:00 2001 From: David Daney Date: Tue, 22 Dec 2015 13:44:51 -0800 Subject: PCI/AER: Restore pci_ops pointer while calling original pci_ops The aer_inject module intercepts config space accesses by replacing the bus->ops pointer. If it forwards accesses to the original pci_ops, and those original ops use bus->ops, they see the aer_pci_ops instead of their own pci_ops, which can cause a crash. For example, pci_generic_config_read() uses the bus->ops->map_bus pointer. If bus->ops is set to aer_pci_ops, which doesn't supply .map_bus, pci_generic_config_read() will dereference an invalid pointer and cause a crash. Temporarily restore the original bus->ops pointer while calling ops->read() or ops->write(). Callers of these functions already hold pci_lock, which prevents other users of bus->ops until we're finished. [bhelgaas: changelog] Signed-off-by: David Daney Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aer_inject.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c index 148a301a6b3c..79a5e112711a 100644 --- a/drivers/pci/pcie/aer/aer_inject.c +++ b/drivers/pci/pcie/aer/aer_inject.c @@ -188,7 +188,9 @@ static int aer_inj_read_config(struct pci_bus *bus, unsigned int devfn, struct aer_error *err; unsigned long flags; struct pci_ops *ops; + struct pci_ops *my_ops; int domain; + int rv; spin_lock_irqsave(&inject_lock, flags); if (size != sizeof(u32)) @@ -208,8 +210,19 @@ static int aer_inj_read_config(struct pci_bus *bus, unsigned int devfn, } out: ops = __find_pci_bus_ops(bus); + /* + * pci_lock must already be held, so we can directly + * manipulate bus->ops. Many config access functions, + * including pci_generic_config_read() require the original + * bus->ops be installed to function, so temporarily put them + * back. + */ + my_ops = bus->ops; + bus->ops = ops; + rv = ops->read(bus, devfn, where, size, val); + bus->ops = my_ops; spin_unlock_irqrestore(&inject_lock, flags); - return ops->read(bus, devfn, where, size, val); + return rv; } static int aer_inj_write_config(struct pci_bus *bus, unsigned int devfn, @@ -220,7 +233,9 @@ static int aer_inj_write_config(struct pci_bus *bus, unsigned int devfn, unsigned long flags; int rw1cs; struct pci_ops *ops; + struct pci_ops *my_ops; int domain; + int rv; spin_lock_irqsave(&inject_lock, flags); if (size != sizeof(u32)) @@ -243,8 +258,19 @@ static int aer_inj_write_config(struct pci_bus *bus, unsigned int devfn, } out: ops = __find_pci_bus_ops(bus); + /* + * pci_lock must already be held, so we can directly + * manipulate bus->ops. Many config access functions, + * including pci_generic_config_write() require the original + * bus->ops be installed to function, so temporarily put them + * back. + */ + my_ops = bus->ops; + bus->ops = ops; + rv = ops->write(bus, devfn, where, size, val); + bus->ops = my_ops; spin_unlock_irqrestore(&inject_lock, flags); - return ops->write(bus, devfn, where, size, val); + return rv; } static struct pci_ops aer_inj_pci_ops = { -- cgit v1.2.3 From 0e6053dc6e7a42c8ba9ce6e81adb3350c7df4bc8 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Fri, 22 Jan 2016 22:50:19 +0800 Subject: PCI/AER: Use list_first_entry_or_null() to simplify code Use list_first_entry_or_null() instead of list_empty() + list_entry() to simplify the code. Signed-off-by: Geliang Tang Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aer/aer_inject.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'drivers/pci/pcie') diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c index 79a5e112711a..e2760a39a98a 100644 --- a/drivers/pci/pcie/aer/aer_inject.c +++ b/drivers/pci/pcie/aer/aer_inject.c @@ -124,16 +124,13 @@ static struct pci_ops *__find_pci_bus_ops(struct pci_bus *bus) static struct pci_bus_ops *pci_bus_ops_pop(void) { unsigned long flags; - struct pci_bus_ops *bus_ops = NULL; + struct pci_bus_ops *bus_ops; spin_lock_irqsave(&inject_lock, flags); - if (list_empty(&pci_bus_ops_list)) - bus_ops = NULL; - else { - struct list_head *lh = pci_bus_ops_list.next; - list_del(lh); - bus_ops = list_entry(lh, struct pci_bus_ops, list); - } + bus_ops = list_first_entry_or_null(&pci_bus_ops_list, + struct pci_bus_ops, list); + if (bus_ops) + list_del(&bus_ops->list); spin_unlock_irqrestore(&inject_lock, flags); return bus_ops; } -- cgit v1.2.3 From 4e48fe4148698ffd3935800f4967362e80a7ae92 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 5 Feb 2016 14:57:12 -0600 Subject: PCI/PME: Remove redundant port lookup We've already looked up srv->port a few lines earlier, and there's no need to do it again. Remove the redundant lookup. Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki suspend_level = PME_SUSPEND_WAKEUP; } if (!wakeup || ret) { - struct pci_dev *port = srv->port; - pcie_pme_interrupt_enable(port, false); pcie_clear_root_pme_status(port); data->suspend_level = PME_SUSPEND_NOIRQ; -- cgit v1.2.3 From 41ccebaecef50e56f822791a52b7bd9e9608e5e6 Mon Sep 17 00:00:00 2001 From: Bjorn Helgaas Date: Fri, 5 Feb 2016 14:57:19 -0600 Subject: PCI/PME: Restructure pcie_pme_suspend() to prevent compiler warning Previously we had this: if (wakeup) ret = enable_irq_wake(...); if (!wakeup || ret) ... "ret" is only evaluated when "wakeup" is true, and it is always initialized in that case, but gcc isn't smart enough to figure that out and warns: drivers/pci/pcie/pme.c:414:14: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized] Restructure the code slightly to make it easier for gcc (and maybe for humans as well). Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki port; - bool wakeup; + bool wakeup, wake_irq_enabled = false; int ret; if (device_may_wakeup(&port->dev)) { @@ -409,9 +409,12 @@ static int pcie_pme_suspend(struct pcie_device *srv) spin_lock_irq(&data->lock); if (wakeup) { ret = enable_irq_wake(srv->irq); - data->suspend_level = PME_SUSPEND_WAKEUP; + if (ret == 0) { + data->suspend_level = PME_SUSPEND_WAKEUP; + wake_irq_enabled = true; + } } - if (!wakeup || ret) { + if (!wake_irq_enabled) { pcie_pme_interrupt_enable(port, false); pcie_clear_root_pme_status(port); data->suspend_level = PME_SUSPEND_NOIRQ; -- cgit v1.2.3