diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h index a54c2cc88c47..30820a649e6e 100644 --- a/arch/s390/include/asm/pci.h +++ b/arch/s390/include/asm/pci.h @@ -122,6 +122,7 @@ struct zpci_dev { struct rcu_head rcu; struct hotplug_slot hotplug_slot; + struct mutex state_lock; /* protect state changes */ enum zpci_state state; u32 fid; /* function ID, used by sclp */ u32 fh; /* function handle, used by insn's */ diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index dff609e8a2a0..17267f659d22 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -730,12 +731,12 @@ EXPORT_SYMBOL_GPL(zpci_disable_device); * equivalent to its state during boot when first probing a driver. * Consequently after reset the PCI function requires re-initialization via the * common PCI code including re-enabling IRQs via pci_alloc_irq_vectors() - * and enabling the function via e.g.pci_enablde_device_flags().The caller + * and enabling the function via e.g. pci_enable_device_flags(). The caller * must guard against concurrent reset attempts. * * In most cases this function should not be called directly but through * pci_reset_function() or pci_reset_bus() which handle the save/restore and - * locking. + * locking - asserted by lockdep. * * Return: 0 on success and an error value otherwise */ @@ -744,6 +745,7 @@ int zpci_hot_reset_device(struct zpci_dev *zdev) u8 status; int rc; + lockdep_assert_held(&zdev->state_lock); zpci_dbg(3, "rst fid:%x, fh:%x\n", zdev->fid, zdev->fh); if (zdev_enabled(zdev)) { /* Disables device access, DMAs and IRQs (reset state) */ @@ -806,6 +808,7 @@ struct zpci_dev *zpci_create_device(u32 fid, u32 fh, enum zpci_state state) zdev->state = state; kref_init(&zdev->kref); + mutex_init(&zdev->state_lock); mutex_init(&zdev->fmb_lock); mutex_init(&zdev->kzdev_lock); @@ -870,6 +873,10 @@ int zpci_deconfigure_device(struct zpci_dev *zdev) { int rc; + lockdep_assert_held(&zdev->state_lock); + if (zdev->state != ZPCI_FN_STATE_CONFIGURED) + return 0; + if (zdev->zbus->bus) zpci_bus_remove_device(zdev, false); diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c index 4d9773ef9e0a..a17804b0dab4 100644 --- a/arch/s390/pci/pci_event.c +++ b/arch/s390/pci/pci_event.c @@ -267,6 +267,7 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf) zpci_err_hex(ccdf, sizeof(*ccdf)); if (zdev) { + mutex_lock(&zdev->state_lock); zpci_update_fh(zdev, ccdf->fh); if (zdev->zbus->bus) pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn); @@ -294,6 +295,8 @@ static void __zpci_event_error(struct zpci_ccdf_err *ccdf) } pci_dev_put(pdev); no_pdev: + if (zdev) + mutex_unlock(&zdev->state_lock); zpci_zdev_put(zdev); } @@ -326,6 +329,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf) zpci_dbg(3, "avl fid:%x, fh:%x, pec:%x\n", ccdf->fid, ccdf->fh, ccdf->pec); + + if (existing_zdev) + mutex_lock(&zdev->state_lock); + switch (ccdf->pec) { case 0x0301: /* Reserved|Standby -> Configured */ if (!zdev) { @@ -383,8 +390,10 @@ static void __zpci_event_availability(struct zpci_ccdf_avail *ccdf) default: break; } - if (existing_zdev) + if (existing_zdev) { + mutex_unlock(&zdev->state_lock); zpci_zdev_put(zdev); + } } void zpci_event_availability(void *data) diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c index 8a7abac51816..a0b872b74fe3 100644 --- a/arch/s390/pci/pci_sysfs.c +++ b/arch/s390/pci/pci_sysfs.c @@ -49,6 +49,39 @@ static ssize_t mio_enabled_show(struct device *dev, } static DEVICE_ATTR_RO(mio_enabled); +static int _do_recover(struct pci_dev *pdev, struct zpci_dev *zdev) +{ + u8 status; + int ret; + + pci_stop_and_remove_bus_device(pdev); + if (zdev_enabled(zdev)) { + ret = zpci_disable_device(zdev); + /* + * Due to a z/VM vs LPAR inconsistency in the error + * state the FH may indicate an enabled device but + * disable says the device is already disabled don't + * treat it as an error here. + */ + if (ret == -EINVAL) + ret = 0; + if (ret) + return ret; + } + + ret = zpci_enable_device(zdev); + if (ret) + return ret; + + if (zdev->dma_table) { + ret = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + virt_to_phys(zdev->dma_table), &status); + if (ret) + zpci_disable_device(zdev); + } + return ret; +} + static ssize_t recover_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { @@ -56,7 +89,6 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr, struct pci_dev *pdev = to_pci_dev(dev); struct zpci_dev *zdev = to_zpci(pdev); int ret = 0; - u8 status; /* Can't use device_remove_self() here as that would lead us to lock * the pci_rescan_remove_lock while holding the device' kernfs lock. @@ -70,6 +102,12 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr, */ kn = sysfs_break_active_protection(&dev->kobj, &attr->attr); WARN_ON_ONCE(!kn); + + /* Device needs to be configured and state must not change */ + mutex_lock(&zdev->state_lock); + if (zdev->state != ZPCI_FN_STATE_CONFIGURED) + goto out; + /* device_remove_file() serializes concurrent calls ignoring all but * the first */ @@ -82,35 +120,13 @@ static ssize_t recover_store(struct device *dev, struct device_attribute *attr, */ pci_lock_rescan_remove(); if (pci_dev_is_added(pdev)) { - pci_stop_and_remove_bus_device(pdev); - if (zdev_enabled(zdev)) { - ret = zpci_disable_device(zdev); - /* - * Due to a z/VM vs LPAR inconsistency in the error - * state the FH may indicate an enabled device but - * disable says the device is already disabled don't - * treat it as an error here. - */ - if (ret == -EINVAL) - ret = 0; - if (ret) - goto out; - } - - ret = zpci_enable_device(zdev); - if (ret) - goto out; - - if (zdev->dma_table) { - ret = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, - virt_to_phys(zdev->dma_table), &status); - if (ret) - zpci_disable_device(zdev); - } + ret = _do_recover(pdev, zdev); } -out: pci_rescan_bus(zdev->zbus->bus); pci_unlock_rescan_remove(); + +out: + mutex_unlock(&zdev->state_lock); if (kn) sysfs_unbreak_active_protection(kn); return ret ? ret : count; diff --git a/drivers/pci/hotplug/s390_pci_hpc.c b/drivers/pci/hotplug/s390_pci_hpc.c index a89b7de72dcf..7333b305f2a5 100644 --- a/drivers/pci/hotplug/s390_pci_hpc.c +++ b/drivers/pci/hotplug/s390_pci_hpc.c @@ -26,58 +26,79 @@ static int enable_slot(struct hotplug_slot *hotplug_slot) hotplug_slot); int rc; - if (zdev->state != ZPCI_FN_STATE_STANDBY) - return -EIO; + mutex_lock(&zdev->state_lock); + if (zdev->state != ZPCI_FN_STATE_STANDBY) { + rc = -EIO; + goto out; + } rc = sclp_pci_configure(zdev->fid); zpci_dbg(3, "conf fid:%x, rc:%d\n", zdev->fid, rc); if (rc) - return rc; + goto out; zdev->state = ZPCI_FN_STATE_CONFIGURED; - return zpci_scan_configured_device(zdev, zdev->fh); + rc = zpci_scan_configured_device(zdev, zdev->fh); +out: + mutex_unlock(&zdev->state_lock); + return rc; } static int disable_slot(struct hotplug_slot *hotplug_slot) { struct zpci_dev *zdev = container_of(hotplug_slot, struct zpci_dev, hotplug_slot); - struct pci_dev *pdev; + struct pci_dev *pdev = NULL; + int rc; - if (zdev->state != ZPCI_FN_STATE_CONFIGURED) - return -EIO; + mutex_lock(&zdev->state_lock); + if (zdev->state != ZPCI_FN_STATE_CONFIGURED) { + rc = -EIO; + goto out; + } pdev = pci_get_slot(zdev->zbus->bus, zdev->devfn); if (pdev && pci_num_vf(pdev)) { pci_dev_put(pdev); - return -EBUSY; + rc = -EBUSY; + goto out; } - pci_dev_put(pdev); - return zpci_deconfigure_device(zdev); + rc = zpci_deconfigure_device(zdev); +out: + mutex_unlock(&zdev->state_lock); + if (pdev) + pci_dev_put(pdev); + return rc; } static int reset_slot(struct hotplug_slot *hotplug_slot, bool probe) { struct zpci_dev *zdev = container_of(hotplug_slot, struct zpci_dev, hotplug_slot); + int rc = -EIO; - if (zdev->state != ZPCI_FN_STATE_CONFIGURED) - return -EIO; /* - * We can't take the zdev->lock as reset_slot may be called during - * probing and/or device removal which already happens under the - * zdev->lock. Instead the user should use the higher level - * pci_reset_function() or pci_bus_reset() which hold the PCI device - * lock preventing concurrent removal. If not using these functions - * holding the PCI device lock is required. + * If we can't get the zdev->state_lock the device state is + * currently undergoing a transition and we bail out - just + * the same as if the device's state is not configured at all. */ + if (!mutex_trylock(&zdev->state_lock)) + return rc; - /* As long as the function is configured we can reset */ - if (probe) - return 0; + /* We can reset only if the function is configured */ + if (zdev->state != ZPCI_FN_STATE_CONFIGURED) + goto out; - return zpci_hot_reset_device(zdev); + if (probe) { + rc = 0; + goto out; + } + + rc = zpci_hot_reset_device(zdev); +out: + mutex_unlock(&zdev->state_lock); + return rc; } static int get_power_status(struct hotplug_slot *hotplug_slot, u8 *value)