lkml.org 
[lkml]   [2015]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH kernel v8 12/31] powerpc/spapr: vfio: Switch from iommu_table to new iommu_table_group
On Fri, Apr 17, 2015 at 01:48:13AM +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@ozlabs.ru>
> >>---
> >> 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.
> >
> >>+};
> >>+
> >>+#ifdef CONFIG_IOMMU_API
> >>+extern void iommu_register_group(struct iommu_table_group *table_group,
> >> int pci_domain_number, unsigned long pe_num);
> >> extern int iommu_add_device(struct device *dev);
> >> extern void iommu_del_device(struct device *dev);
> >> extern int __init tce_iommu_bus_notifier_init(void);
> >> #else
> >>-static inline void iommu_register_group(struct iommu_table *tbl,
> >>+static inline void iommu_register_group(struct iommu_table_group *table_group,
> >> int pci_domain_number,
> >> unsigned long pe_num)
> >> {
> >>diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> >>index b39d00a..fd49c8e 100644
> >>--- a/arch/powerpc/kernel/iommu.c
> >>+++ b/arch/powerpc/kernel/iommu.c
> >>@@ -712,17 +712,20 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid)
> >>
> >> struct iommu_table *iommu_table_alloc(int node)
> >> {
> >>- struct iommu_table *tbl;
> >>+ struct iommu_table_group *table_group;
> >>
> >>- tbl = kzalloc_node(sizeof(struct iommu_table), GFP_KERNEL, node);
> >>+ table_group = kzalloc_node(sizeof(struct iommu_table_group), GFP_KERNEL,
> >>+ node);
> >>+ table_group->tables[0].it_group = table_group;
> >>
> >>- return tbl;
> >>+ return &table_group->tables[0];
> >> }
> >>
> >> void iommu_free_table(struct iommu_table *tbl, const char *node_name)
> >
> >Surely the free function should take a table group rather than a table
> >as argument.
>
>
> No, it should not. Tables lifetime is not the same even within the
> same group.

If that's so, then this function shouldn't free the group...

--
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
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-04-20 08:41    [W:0.122 / U:4.148 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site