Re: [PATCH kernel v8 12/31] powerpc/spapr: vfio: Switch from iommu_table to new iommu_table_group

[David Gibson]

On Fri, Apr 17, 2015 at 07:46:23PM +1000, Alexey Kardashevskiy wrote:
> On 04/16/2015 03:55 PM, David Gibson wrote:
> >On Fri, Apr 10, 2015 at 04:30:54PM +1000, Alexey Kardashevskiy wrote:
> >>Modern IBM POWERPC systems support multiple (currently two) TCE tables
> >>per IOMMU group (a.k.a. PE). This adds a iommu_table_group container
> >>for TCE tables. Right now just one table is supported.
> >>
> >>Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> >>---
> >>  arch/powerpc/include/asm/iommu.h            |  18 +++--
> >>  arch/powerpc/kernel/iommu.c                 |  34 ++++----
> >>  arch/powerpc/platforms/powernv/pci-ioda.c   |  38 +++++----
> >>  arch/powerpc/platforms/powernv/pci-p5ioc2.c |  17 ++--
> >>  arch/powerpc/platforms/powernv/pci.c        |   2 +-
> >>  arch/powerpc/platforms/powernv/pci.h        |   4 +-
> >>  arch/powerpc/platforms/pseries/iommu.c      |   9 ++-
> >>  drivers/vfio/vfio_iommu_spapr_tce.c         | 120 ++++++++++++++++++++--------
> >>  8 files changed, 160 insertions(+), 82 deletions(-)
> >>
> >>diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
> >>index eb75726..667aa1a 100644
> >>--- a/arch/powerpc/include/asm/iommu.h
> >>+++ b/arch/powerpc/include/asm/iommu.h
> >>@@ -90,9 +90,7 @@ struct iommu_table {
> >>  	struct iommu_pool pools[IOMMU_NR_POOLS];
> >>  	unsigned long *it_map;       /* A simple allocation bitmap for now */
> >>  	unsigned long  it_page_shift;/* table iommu page size */
> >>-#ifdef CONFIG_IOMMU_API
> >>-	struct iommu_group *it_group;
> >>-#endif
> >>+	struct iommu_table_group *it_group;
> >>  	struct iommu_table_ops *it_ops;
> >>  	void (*set_bypass)(struct iommu_table *tbl, bool enable);
> >>  };
> >>@@ -126,14 +124,24 @@ extern void iommu_free_table(struct iommu_table *tbl, const char *node_name);
> >>   */
> >>  extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
> >>  					    int nid);
> >>+
> >>+#define IOMMU_TABLE_GROUP_MAX_TABLES	1
> >>+
> >>+struct iommu_table_group {
> >>  #ifdef CONFIG_IOMMU_API
> >>-extern void iommu_register_group(struct iommu_table *tbl,
> >>+	struct iommu_group *group;
> >>+#endif
> >>+	struct iommu_table tables[IOMMU_TABLE_GROUP_MAX_TABLES];
> >
> >There's nothing to indicate which of the tables are in use at the
> >current time.  I mean, it doesn't matter now because there's only one,
> >but the patch doesn't make a whole lot of sense without that.
> 
> 
> Later in the patchset, the code will look at @it_size to know if the table
> is in use.

Ok, that makes sense.  Might be worth a comment here.

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

Attachment: pgpLDow9wRNg_.pgp
Description: PGP signature