Re: [PATCH kernel v11 02/34] powerpc/iommu/powernv: Get rid of set_iommu_table_base_and_group

2015-05-31 Thread David Gibson
On Fri, May 29, 2015 at 06:44:26PM +1000, Alexey Kardashevskiy wrote:
 The set_iommu_table_base_and_group() name suggests that the function
 sets table base and add a device to an IOMMU group.
 
 The actual purpose for table base setting is to put some reference
 into a device so later iommu_add_device() can get the IOMMU group
 reference and the device to the group.
 
 At the moment a group cannot be explicitly passed to iommu_add_device()
 as we want it to work from the bus notifier, we can fix it later and
 remove confusing calls of set_iommu_table_base().
 
 This replaces set_iommu_table_base_and_group() with a couple of
 set_iommu_table_base() + iommu_add_device() which makes reading the code
 easier.
 
 This adds few comments why set_iommu_table_base() and iommu_add_device()
 are called where they are called.
 
 For IODA1/2, this essentially removes iommu_add_device() call from
 the pnv_pci_ioda_dma_dev_setup() as it will always fail at this particular
 place:
 - for physical PE, the device is already attached by iommu_add_device()
 in pnv_pci_ioda_setup_dma_pe();
 - for virtual PE, the sysfs entries are not ready to create all symlinks
 so actual adding is happening in tce_iommu_bus_notifier.
 
 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 Reviewed-by: Gavin Shan gws...@linux.vnet.ibm.com

Reviewed-by: David Gibson da...@gibson.dropbear.id.au

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


pgp5SRDiXfhXF.pgp
Description: PGP signature


[PATCH kernel v11 02/34] powerpc/iommu/powernv: Get rid of set_iommu_table_base_and_group

2015-05-29 Thread Alexey Kardashevskiy
The set_iommu_table_base_and_group() name suggests that the function
sets table base and add a device to an IOMMU group.

The actual purpose for table base setting is to put some reference
into a device so later iommu_add_device() can get the IOMMU group
reference and the device to the group.

At the moment a group cannot be explicitly passed to iommu_add_device()
as we want it to work from the bus notifier, we can fix it later and
remove confusing calls of set_iommu_table_base().

This replaces set_iommu_table_base_and_group() with a couple of
set_iommu_table_base() + iommu_add_device() which makes reading the code
easier.

This adds few comments why set_iommu_table_base() and iommu_add_device()
are called where they are called.

For IODA1/2, this essentially removes iommu_add_device() call from
the pnv_pci_ioda_dma_dev_setup() as it will always fail at this particular
place:
- for physical PE, the device is already attached by iommu_add_device()
in pnv_pci_ioda_setup_dma_pe();
- for virtual PE, the sysfs entries are not ready to create all symlinks
so actual adding is happening in tce_iommu_bus_notifier.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
Reviewed-by: Gavin Shan gws...@linux.vnet.ibm.com
---
Changes:
v10:
* new to the series
---
 arch/powerpc/include/asm/iommu.h|  7 ---
 arch/powerpc/platforms/powernv/pci-ioda.c   | 27 +++
 arch/powerpc/platforms/powernv/pci-p5ioc2.c |  3 ++-
 arch/powerpc/platforms/pseries/iommu.c  | 15 ---
 4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
index 1e27d63..8353c86 100644
--- a/arch/powerpc/include/asm/iommu.h
+++ b/arch/powerpc/include/asm/iommu.h
@@ -140,13 +140,6 @@ static inline int __init tce_iommu_bus_notifier_init(void)
 }
 #endif /* !CONFIG_IOMMU_API */
 
-static inline void set_iommu_table_base_and_group(struct device *dev,
- void *base)
-{
-   set_iommu_table_base(dev, base);
-   iommu_add_device(dev);
-}
-
 extern int ppc_iommu_map_sg(struct device *dev, struct iommu_table *tbl,
struct scatterlist *sglist, int nelems,
unsigned long mask,
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 2f092bb..9a77f3c 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1598,7 +1598,13 @@ static void pnv_pci_ioda_dma_dev_setup(struct pnv_phb 
*phb, struct pci_dev *pdev
 
pe = phb-ioda.pe_array[pdn-pe_number];
WARN_ON(get_dma_ops(pdev-dev) != dma_iommu_ops);
-   set_iommu_table_base_and_group(pdev-dev, pe-tce32_table);
+   set_iommu_table_base(pdev-dev, pe-tce32_table);
+   /*
+* Note: iommu_add_device() will fail here as
+* for physical PE: the device is already added by now;
+* for virtual PE: sysfs entries are not ready yet and
+* tce_iommu_bus_notifier will add the device to a group later.
+*/
 }
 
 static int pnv_pci_ioda_dma_set_mask(struct pnv_phb *phb,
@@ -1659,7 +1665,8 @@ static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe,
struct pci_dev *dev;
 
list_for_each_entry(dev, bus-devices, bus_list) {
-   set_iommu_table_base_and_group(dev-dev, pe-tce32_table);
+   set_iommu_table_base(dev-dev, pe-tce32_table);
+   iommu_add_device(dev-dev);
 
if (dev-subordinate)
pnv_ioda_setup_bus_dma(pe, dev-subordinate);
@@ -1835,7 +1842,13 @@ static void pnv_pci_ioda_setup_dma_pe(struct pnv_phb 
*phb,
if (pe-flags  PNV_IODA_PE_DEV) {
iommu_register_group(tbl, phb-hose-global_number,
 pe-pe_number);
-   set_iommu_table_base_and_group(pe-pdev-dev, tbl);
+   /*
+* Setting table base here only for carrying iommu_group
+* further down to let iommu_add_device() do the job.
+* pnv_pci_ioda_dma_dev_setup will override it later anyway.
+*/
+   set_iommu_table_base(pe-pdev-dev, tbl);
+   iommu_add_device(pe-pdev-dev);
} else if (pe-flags  (PNV_IODA_PE_BUS | PNV_IODA_PE_BUS_ALL)) {
iommu_register_group(tbl, phb-hose-global_number,
 pe-pe_number);
@@ -1963,7 +1976,13 @@ static void pnv_pci_ioda2_setup_dma_pe(struct pnv_phb 
*phb,
if (pe-flags  PNV_IODA_PE_DEV) {
iommu_register_group(tbl, phb-hose-global_number,
 pe-pe_number);
-   set_iommu_table_base_and_group(pe-pdev-dev, tbl);
+   /*
+* Setting table base here only for carrying iommu_group
+* further down to let iommu_add_device() do the job.
+*