From d7b1946c7925a270062b2e0718aa57b42ba619c0 Mon Sep 17 00:00:00 2001 From: Vaibhav Jain Date: Wed, 4 Jan 2017 11:48:52 +0530 Subject: cxl: Force psl data-cache flush during device shutdown This change adds a force psl data cache flush during device shutdown callback. This should reduce a possibility of psl holding a dirty cache line while the CAPP is being reinitialized, which may result in a UE [load/store] machine check error. Signed-off-by: Vaibhav Jain Reviewed-by: Andrew Donnellan Acked-by: Frederic Barrat Signed-off-by: Michael Ellerman --- drivers/misc/cxl/pci.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/misc/cxl/pci.c') diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index 80a87ab25b83..73432e7d925d 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -1610,6 +1610,9 @@ static void cxl_pci_remove_adapter(struct cxl *adapter) cxl_sysfs_adapter_remove(adapter); cxl_debugfs_adapter_remove(adapter); + /* Flush adapter datacache as its about to be removed */ + cxl_data_cache_flush(adapter); + cxl_deconfigure_adapter(adapter); device_unregister(&adapter->dev); -- cgit From 14a3ae34bfd0bcb1cc12d55b06a8584c11fac6fc Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Fri, 9 Dec 2016 17:18:50 +1100 Subject: cxl: Prevent read/write to AFU config space while AFU not configured During EEH recovery, we deconfigure all AFUs whilst leaving the corresponding vPHB and virtual PCI device in place. If something attempts to interact with the AFU's PCI config space (e.g. running lspci) after the AFU has been deconfigured and before it's reconfigured, cxl_pcie_{read,write}_config() will read invalid values from the deconfigured struct cxl_afu and proceed to Oops when they try to dereference pointers that have been set to NULL during deconfiguration. Add a rwsem to struct cxl_afu so we can prevent interaction with config space while the AFU is deconfigured. Reported-by: Pradipta Ghosh Suggested-by: Frederic Barrat Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Andrew Donnellan Signed-off-by: Vaibhav Jain Signed-off-by: Michael Ellerman --- drivers/misc/cxl/cxl.h | 2 ++ drivers/misc/cxl/main.c | 3 ++- drivers/misc/cxl/pci.c | 2 ++ drivers/misc/cxl/vphb.c | 51 ++++++++++++++++++++++++++++--------------------- 4 files changed, 35 insertions(+), 23 deletions(-) (limited to 'drivers/misc/cxl/pci.c') diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h index b24d76723fb0..b4a43fd14b99 100644 --- a/drivers/misc/cxl/cxl.h +++ b/drivers/misc/cxl/cxl.h @@ -418,6 +418,8 @@ struct cxl_afu { struct dentry *debugfs; struct mutex contexts_lock; spinlock_t afu_cntl_lock; + /* Used to block access to AFU config space while deconfigured */ + struct rw_semaphore configured_rwsem; /* AFU error buffer fields and bin attribute for sysfs */ u64 eb_len, eb_offset; diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c index 62e0dfb5f15b..2a6bf1d0a3a4 100644 --- a/drivers/misc/cxl/main.c +++ b/drivers/misc/cxl/main.c @@ -268,7 +268,8 @@ struct cxl_afu *cxl_alloc_afu(struct cxl *adapter, int slice) idr_init(&afu->contexts_idr); mutex_init(&afu->contexts_lock); spin_lock_init(&afu->afu_cntl_lock); - + init_rwsem(&afu->configured_rwsem); + down_write(&afu->configured_rwsem); afu->prefault_mode = CXL_PREFAULT_NONE; afu->irqs_max = afu->adapter->user_irqs; diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index 73432e7d925d..cca938845ffd 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -1129,6 +1129,7 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc if ((rc = cxl_native_register_psl_irq(afu))) goto err2; + up_write(&afu->configured_rwsem); return 0; err2: @@ -1141,6 +1142,7 @@ err1: static void pci_deconfigure_afu(struct cxl_afu *afu) { + down_write(&afu->configured_rwsem); cxl_native_release_psl_irq(afu); if (afu->adapter->native->sl_ops->release_serr_irq) afu->adapter->native->sl_ops->release_serr_irq(afu); diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c index 3519acebfdab..639a343b7836 100644 --- a/drivers/misc/cxl/vphb.c +++ b/drivers/misc/cxl/vphb.c @@ -76,23 +76,22 @@ static int cxl_pcie_cfg_record(u8 bus, u8 devfn) return (bus << 8) + devfn; } -static int cxl_pcie_config_info(struct pci_bus *bus, unsigned int devfn, - struct cxl_afu **_afu, int *_record) +static inline struct cxl_afu *pci_bus_to_afu(struct pci_bus *bus) { - struct pci_controller *phb; - struct cxl_afu *afu; - int record; + struct pci_controller *phb = bus ? pci_bus_to_host(bus) : NULL; - phb = pci_bus_to_host(bus); - if (phb == NULL) - return PCIBIOS_DEVICE_NOT_FOUND; + return phb ? phb->private_data : NULL; +} + +static inline int cxl_pcie_config_info(struct pci_bus *bus, unsigned int devfn, + struct cxl_afu *afu, int *_record) +{ + int record; - afu = (struct cxl_afu *)phb->private_data; record = cxl_pcie_cfg_record(bus->number, devfn); if (record > afu->crs_num) return PCIBIOS_DEVICE_NOT_FOUND; - *_afu = afu; *_record = record; return 0; } @@ -106,9 +105,14 @@ static int cxl_pcie_read_config(struct pci_bus *bus, unsigned int devfn, u16 val16; u32 val32; - rc = cxl_pcie_config_info(bus, devfn, &afu, &record); + afu = pci_bus_to_afu(bus); + /* Grab a reader lock on afu. */ + if (afu == NULL || !down_read_trylock(&afu->configured_rwsem)) + return PCIBIOS_DEVICE_NOT_FOUND; + + rc = cxl_pcie_config_info(bus, devfn, afu, &record); if (rc) - return rc; + goto out; switch (len) { case 1: @@ -127,10 +131,9 @@ static int cxl_pcie_read_config(struct pci_bus *bus, unsigned int devfn, WARN_ON(1); } - if (rc) - return PCIBIOS_DEVICE_NOT_FOUND; - - return PCIBIOS_SUCCESSFUL; +out: + up_read(&afu->configured_rwsem); + return rc ? PCIBIOS_DEVICE_NOT_FOUND : PCIBIOS_SUCCESSFUL; } static int cxl_pcie_write_config(struct pci_bus *bus, unsigned int devfn, @@ -139,9 +142,14 @@ static int cxl_pcie_write_config(struct pci_bus *bus, unsigned int devfn, int rc, record; struct cxl_afu *afu; - rc = cxl_pcie_config_info(bus, devfn, &afu, &record); + afu = pci_bus_to_afu(bus); + /* Grab a reader lock on afu. */ + if (afu == NULL || !down_read_trylock(&afu->configured_rwsem)) + return PCIBIOS_DEVICE_NOT_FOUND; + + rc = cxl_pcie_config_info(bus, devfn, afu, &record); if (rc) - return rc; + goto out; switch (len) { case 1: @@ -157,10 +165,9 @@ static int cxl_pcie_write_config(struct pci_bus *bus, unsigned int devfn, WARN_ON(1); } - if (rc) - return PCIBIOS_SET_FAILED; - - return PCIBIOS_SUCCESSFUL; +out: + up_read(&afu->configured_rwsem); + return rc ? PCIBIOS_SET_FAILED : PCIBIOS_SUCCESSFUL; } static struct pci_ops cxl_pcie_pci_ops = -- cgit From 171ed0fcd8966d82c45376f1434678e7b9d4d9b1 Mon Sep 17 00:00:00 2001 From: Andrew Donnellan Date: Mon, 6 Feb 2017 12:07:17 +1100 Subject: cxl: fix nested locking hang during EEH hotplug Commit 14a3ae34bfd0 ("cxl: Prevent read/write to AFU config space while AFU not configured") introduced a rwsem to fix an invalid memory access that occurred when someone attempts to access the config space of an AFU on a vPHB whilst the AFU is deconfigured, such as during EEH recovery. It turns out that it's possible to run into a nested locking issue when EEH recovery fails and a full device hotplug is required. cxl_pci_error_detected() deconfigures the AFU, taking a writer lock on configured_rwsem. When EEH recovery fails, the EEH code calls pci_hp_remove_devices() to remove the device, which in turn calls cxl_remove() -> cxl_pci_remove_afu() -> pci_deconfigure_afu(), which tries to grab the writer lock that's already held. Standard rwsem semantics don't express what we really want to do here and don't allow for nested locking. Fix this by replacing the rwsem with an atomic_t which we can control more finely. Allow the AFU to be locked multiple times so long as there are no readers. Fixes: 14a3ae34bfd0 ("cxl: Prevent read/write to AFU config space while AFU not configured") Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Andrew Donnellan Acked-by: Frederic Barrat Signed-off-by: Michael Ellerman --- drivers/misc/cxl/cxl.h | 5 +++-- drivers/misc/cxl/main.c | 3 +-- drivers/misc/cxl/pci.c | 11 +++++++++-- drivers/misc/cxl/vphb.c | 18 ++++++++++++++---- 4 files changed, 27 insertions(+), 10 deletions(-) (limited to 'drivers/misc/cxl/pci.c') diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h index 6c722d96b775..79e60ec70bd3 100644 --- a/drivers/misc/cxl/cxl.h +++ b/drivers/misc/cxl/cxl.h @@ -418,8 +418,9 @@ struct cxl_afu { struct dentry *debugfs; struct mutex contexts_lock; spinlock_t afu_cntl_lock; - /* Used to block access to AFU config space while deconfigured */ - struct rw_semaphore configured_rwsem; + + /* -1: AFU deconfigured/locked, >= 0: number of readers */ + atomic_t configured_state; /* AFU error buffer fields and bin attribute for sysfs */ u64 eb_len, eb_offset; diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c index 2a6bf1d0a3a4..cc1706a92ace 100644 --- a/drivers/misc/cxl/main.c +++ b/drivers/misc/cxl/main.c @@ -268,8 +268,7 @@ struct cxl_afu *cxl_alloc_afu(struct cxl *adapter, int slice) idr_init(&afu->contexts_idr); mutex_init(&afu->contexts_lock); spin_lock_init(&afu->afu_cntl_lock); - init_rwsem(&afu->configured_rwsem); - down_write(&afu->configured_rwsem); + atomic_set(&afu->configured_state, -1); afu->prefault_mode = CXL_PREFAULT_NONE; afu->irqs_max = afu->adapter->user_irqs; diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c index cca938845ffd..91f645992c94 100644 --- a/drivers/misc/cxl/pci.c +++ b/drivers/misc/cxl/pci.c @@ -1129,7 +1129,7 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc if ((rc = cxl_native_register_psl_irq(afu))) goto err2; - up_write(&afu->configured_rwsem); + atomic_set(&afu->configured_state, 0); return 0; err2: @@ -1142,7 +1142,14 @@ err1: static void pci_deconfigure_afu(struct cxl_afu *afu) { - down_write(&afu->configured_rwsem); + /* + * It's okay to deconfigure when AFU is already locked, otherwise wait + * until there are no readers + */ + if (atomic_read(&afu->configured_state) != -1) { + while (atomic_cmpxchg(&afu->configured_state, 0, -1) != -1) + schedule(); + } cxl_native_release_psl_irq(afu); if (afu->adapter->native->sl_ops->release_serr_irq) afu->adapter->native->sl_ops->release_serr_irq(afu); diff --git a/drivers/misc/cxl/vphb.c b/drivers/misc/cxl/vphb.c index 639a343b7836..512a4897dbf6 100644 --- a/drivers/misc/cxl/vphb.c +++ b/drivers/misc/cxl/vphb.c @@ -83,6 +83,16 @@ static inline struct cxl_afu *pci_bus_to_afu(struct pci_bus *bus) return phb ? phb->private_data : NULL; } +static void cxl_afu_configured_put(struct cxl_afu *afu) +{ + atomic_dec_if_positive(&afu->configured_state); +} + +static bool cxl_afu_configured_get(struct cxl_afu *afu) +{ + return atomic_inc_unless_negative(&afu->configured_state); +} + static inline int cxl_pcie_config_info(struct pci_bus *bus, unsigned int devfn, struct cxl_afu *afu, int *_record) { @@ -107,7 +117,7 @@ static int cxl_pcie_read_config(struct pci_bus *bus, unsigned int devfn, afu = pci_bus_to_afu(bus); /* Grab a reader lock on afu. */ - if (afu == NULL || !down_read_trylock(&afu->configured_rwsem)) + if (afu == NULL || !cxl_afu_configured_get(afu)) return PCIBIOS_DEVICE_NOT_FOUND; rc = cxl_pcie_config_info(bus, devfn, afu, &record); @@ -132,7 +142,7 @@ static int cxl_pcie_read_config(struct pci_bus *bus, unsigned int devfn, } out: - up_read(&afu->configured_rwsem); + cxl_afu_configured_put(afu); return rc ? PCIBIOS_DEVICE_NOT_FOUND : PCIBIOS_SUCCESSFUL; } @@ -144,7 +154,7 @@ static int cxl_pcie_write_config(struct pci_bus *bus, unsigned int devfn, afu = pci_bus_to_afu(bus); /* Grab a reader lock on afu. */ - if (afu == NULL || !down_read_trylock(&afu->configured_rwsem)) + if (afu == NULL || !cxl_afu_configured_get(afu)) return PCIBIOS_DEVICE_NOT_FOUND; rc = cxl_pcie_config_info(bus, devfn, afu, &record); @@ -166,7 +176,7 @@ static int cxl_pcie_write_config(struct pci_bus *bus, unsigned int devfn, } out: - up_read(&afu->configured_rwsem); + cxl_afu_configured_put(afu); return rc ? PCIBIOS_SET_FAILED : PCIBIOS_SUCCESSFUL; } -- cgit