From 1f3f76812d5dfc791193b39c2140a8bd09962c0e Mon Sep 17 00:00:00 2001 From: Niklas Schnelle Date: Fri, 16 Jul 2021 11:53:37 +0200 Subject: [PATCH] s390/pci: improve DMA translation init and exit Currently zpci_dma_init_device()/zpci_dma_exit_device() is called as part of zpci_enable_device()/zpci_disable_device() and errors for zpci_dma_exit_device() are always ignored even if we could abort. Improve upon this by moving zpci_dma_exit_device() out of zpci_disable_device() and check for errors whenever we have a way to abort the current operation. Note that for example in zpci_event_hard_deconfigured() the device is expected to be gone so we really can't abort and proceed even in case of error. Similarly move the cc == 3 special case out of zpci_unregister_ioat() and into the callers allowing to abort when finding an already disabled devices precludes proceeding with the operation. While we are at it log IOAT register/unregister errors in the s390 debugfs log, Reviewed-by: Matthew Rosato Signed-off-by: Niklas Schnelle Signed-off-by: Heiko Carstens --- arch/s390/include/asm/pci.h | 2 ++ arch/s390/include/asm/pci_dma.h | 2 -- arch/s390/pci/pci.c | 43 ++++++++++++++++++----------------------- arch/s390/pci/pci_bus.c | 5 +++++ arch/s390/pci/pci_dma.c | 25 ++++++++++++++++-------- arch/s390/pci/pci_event.c | 5 ++++- arch/s390/pci/pci_sysfs.c | 19 +++++++++++++++--- drivers/iommu/s390-iommu.c | 18 ++++++++++++----- 8 files changed, 76 insertions(+), 43 deletions(-) diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h index c51e64d..e4803ec 100644 --- a/arch/s390/include/asm/pci.h +++ b/arch/s390/include/asm/pci.h @@ -272,6 +272,8 @@ struct zpci_dev *get_zdev_by_fid(u32); /* DMA */ int zpci_dma_init(void); void zpci_dma_exit(void); +int zpci_dma_init_device(struct zpci_dev *zdev); +int zpci_dma_exit_device(struct zpci_dev *zdev); /* IRQ */ int __init zpci_irq_init(void); diff --git a/arch/s390/include/asm/pci_dma.h b/arch/s390/include/asm/pci_dma.h index f62cd3e..3b8e89d 100644 --- a/arch/s390/include/asm/pci_dma.h +++ b/arch/s390/include/asm/pci_dma.h @@ -182,8 +182,6 @@ static inline unsigned long *get_st_pto(unsigned long entry) } /* Prototypes */ -int zpci_dma_init_device(struct zpci_dev *); -void zpci_dma_exit_device(struct zpci_dev *); void dma_free_seg_table(unsigned long); unsigned long *dma_alloc_cpu_table(void); void dma_cleanup_tables(unsigned long *); diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index b05b86e..728508d 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -113,13 +113,16 @@ int zpci_register_ioat(struct zpci_dev *zdev, u8 dmaas, { u64 req = ZPCI_CREATE_REQ(zdev->fh, dmaas, ZPCI_MOD_FC_REG_IOAT); struct zpci_fib fib = {0}; - u8 status; + u8 cc, status; WARN_ON_ONCE(iota & 0x3fff); fib.pba = base; fib.pal = limit; fib.iota = iota | ZPCI_IOTA_RTTO_FLAG; - return zpci_mod_fc(req, &fib, &status) ? -EIO : 0; + cc = zpci_mod_fc(req, &fib, &status); + if (cc) + zpci_dbg(3, "reg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); + return cc; } /* Modify PCI: Unregister I/O address translation parameters */ @@ -130,9 +133,9 @@ int zpci_unregister_ioat(struct zpci_dev *zdev, u8 dmaas) u8 cc, status; cc = zpci_mod_fc(req, &fib, &status); - if (cc == 3) /* Function already gone. */ - cc = 0; - return cc ? -EIO : 0; + if (cc) + zpci_dbg(3, "unreg ioat fid:%x, cc:%d, status:%d\n", zdev->fid, cc, status); + return cc; } /* Modify PCI: Set PCI function measurement parameters */ @@ -654,24 +657,12 @@ void zpci_free_domain(int domain) int zpci_enable_device(struct zpci_dev *zdev) { u32 fh = zdev->fh; - int rc; + int rc = 0; - if (clp_enable_fh(zdev, &fh, ZPCI_NR_DMA_SPACES)) { + if (clp_enable_fh(zdev, &fh, ZPCI_NR_DMA_SPACES)) rc = -EIO; - goto out; - } - zdev->fh = fh; - - rc = zpci_dma_init_device(zdev); - if (rc) - goto out_dma; - - return 0; - -out_dma: - clp_disable_fh(zdev, &fh); -out: - zdev->fh = fh; + else + zdev->fh = fh; return rc; } @@ -680,9 +671,6 @@ int zpci_disable_device(struct zpci_dev *zdev) u32 fh = zdev->fh; int cc, rc = 0; - zpci_dma_exit_device(zdev); - if (!zdev_enabled(zdev)) - return 0; cc = clp_disable_fh(zdev, &fh); if (!cc) { zdev->fh = fh; @@ -808,6 +796,11 @@ int zpci_deconfigure_device(struct zpci_dev *zdev) if (zdev->zbus->bus) zpci_bus_remove_device(zdev, false); + if (zdev->dma_table) { + rc = zpci_dma_exit_device(zdev); + if (rc) + return rc; + } if (zdev_enabled(zdev)) { rc = zpci_disable_device(zdev); if (rc) @@ -831,6 +824,8 @@ void zpci_release_device(struct kref *kref) if (zdev->zbus->bus) zpci_bus_remove_device(zdev, false); + if (zdev->dma_table) + zpci_dma_exit_device(zdev); if (zdev_enabled(zdev)) zpci_disable_device(zdev); diff --git a/arch/s390/pci/pci_bus.c b/arch/s390/pci/pci_bus.c index 3c9ad518..5d77acb 100644 --- a/arch/s390/pci/pci_bus.c +++ b/arch/s390/pci/pci_bus.c @@ -49,6 +49,11 @@ static int zpci_bus_prepare_device(struct zpci_dev *zdev) rc = zpci_enable_device(zdev); if (rc) return rc; + rc = zpci_dma_init_device(zdev); + if (rc) { + zpci_disable_device(zdev); + return rc; + } } if (!zdev->has_resources) { diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c index ebc9a49..58f2f7a 100644 --- a/arch/s390/pci/pci_dma.c +++ b/arch/s390/pci/pci_dma.c @@ -590,10 +590,11 @@ int zpci_dma_init_device(struct zpci_dev *zdev) } } - rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, - (u64) zdev->dma_table); - if (rc) + if (zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + (u64)zdev->dma_table)) { + rc = -EIO; goto free_bitmap; + } return 0; free_bitmap: @@ -608,17 +609,25 @@ out: return rc; } -void zpci_dma_exit_device(struct zpci_dev *zdev) +int zpci_dma_exit_device(struct zpci_dev *zdev) { + int cc = 0; + /* * At this point, if the device is part of an IOMMU domain, this would * be a strong hint towards a bug in the IOMMU API (common) code and/or * simultaneous access via IOMMU and DMA API. So let's issue a warning. */ WARN_ON(zdev->s390_domain); - - if (zpci_unregister_ioat(zdev, 0)) - return; + if (zdev_enabled(zdev)) + cc = zpci_unregister_ioat(zdev, 0); + /* + * cc == 3 indicates the function is gone already. This can happen + * if the function was deconfigured/disabled suddenly and we have not + * received a new handle yet. + */ + if (cc && cc != 3) + return -EIO; dma_cleanup_tables(zdev->dma_table); zdev->dma_table = NULL; @@ -626,8 +635,8 @@ void zpci_dma_exit_device(struct zpci_dev *zdev) zdev->iommu_bitmap = NULL; vfree(zdev->lazy_bitmap); zdev->lazy_bitmap = NULL; - zdev->next_bit = 0; + return 0; } static int __init dma_alloc_cpu_table_caches(void) diff --git a/arch/s390/pci/pci_event.c b/arch/s390/pci/pci_event.c index cd447b9..c856f80 100644 --- a/arch/s390/pci/pci_event.c +++ b/arch/s390/pci/pci_event.c @@ -84,7 +84,10 @@ static void zpci_event_hard_deconfigured(struct zpci_dev *zdev, u32 fh) /* Even though the device is already gone we still * need to free zPCI resources as part of the disable. */ - zpci_disable_device(zdev); + if (zdev->dma_table) + zpci_dma_exit_device(zdev); + if (zdev_enabled(zdev)) + zpci_disable_device(zdev); zdev->state = ZPCI_FN_STATE_STANDBY; } diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c index 6e2450c..335c281 100644 --- a/arch/s390/pci/pci_sysfs.c +++ b/arch/s390/pci/pci_sysfs.c @@ -82,13 +82,26 @@ 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); - ret = zpci_disable_device(zdev); - if (ret) - goto out; + if (zdev->dma_table) { + ret = zpci_dma_exit_device(zdev); + if (ret) + goto out; + } + + if (zdev_enabled(zdev)) { + ret = zpci_disable_device(zdev); + if (ret) + goto out; + } ret = zpci_enable_device(zdev); if (ret) goto out; + ret = zpci_dma_init_device(zdev); + if (ret) { + zpci_disable_device(zdev); + goto out; + } pci_rescan_bus(zdev->zbus->bus); } out: diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index 6019e58..83df387 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -90,7 +90,7 @@ static int s390_iommu_attach_device(struct iommu_domain *domain, struct zpci_dev *zdev = to_zpci_dev(dev); struct s390_domain_device *domain_device; unsigned long flags; - int rc; + int cc, rc; if (!zdev) return -ENODEV; @@ -99,14 +99,21 @@ static int s390_iommu_attach_device(struct iommu_domain *domain, if (!domain_device) return -ENOMEM; - if (zdev->dma_table) - zpci_dma_exit_device(zdev); + if (zdev->dma_table) { + cc = zpci_dma_exit_device(zdev); + if (cc) { + rc = -EIO; + goto out_free; + } + } zdev->dma_table = s390_domain->dma_table; - rc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, + cc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma, (u64) zdev->dma_table); - if (rc) + if (cc) { + rc = -EIO; goto out_restore; + } spin_lock_irqsave(&s390_domain->list_lock, flags); /* First device defines the DMA range limits */ @@ -130,6 +137,7 @@ static int s390_iommu_attach_device(struct iommu_domain *domain, out_restore: zpci_dma_init_device(zdev); +out_free: kfree(domain_device); return rc; -- 2.7.4