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 <mjrosato@linux.ibm.com>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

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);

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 *);

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);
 

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) {

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)

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;
 }
 

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:

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;