lkml.org 
[lkml]   [2015]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH kernel v7 26/31] powerpc/iommu: Add userspace view of TCE table
    On 04/03/2015 07:50 AM, Alex Williamson wrote:
    >
    > Should have sent this with the other comments, but found it hiding on my
    > desktop...
    >
    > On Sat, 2015-03-28 at 01:55 +1100, Alexey Kardashevskiy wrote:
    >> In order to support memory pre-registration, we need a way to track
    >> the use of every registered memory region and only allow unregistration
    >> if a region is not in use anymore. So we need a way to tell from what
    >> region the just cleared TCE was from.
    >>
    >> This adds a userspace view of the TCE table into iommu_table struct.
    >> It contains userspace address, one per TCE entry. The table is only
    >> allocated when the ownership over an IOMMU group is taken which means
    >> it is only used from outside of the powernv code (such as VFIO).
    >>
    >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
    >> ---
    >> arch/powerpc/include/asm/iommu.h | 6 ++++++
    >> arch/powerpc/kernel/iommu.c | 7 +++++++
    >> arch/powerpc/platforms/powernv/pci-ioda.c | 23 ++++++++++++++++++++++-
    >> 3 files changed, 35 insertions(+), 1 deletion(-)
    >>
    >> diff --git a/arch/powerpc/include/asm/iommu.h b/arch/powerpc/include/asm/iommu.h
    >> index 2c08c91..a768a4d 100644
    >> --- a/arch/powerpc/include/asm/iommu.h
    >> +++ b/arch/powerpc/include/asm/iommu.h
    >> @@ -106,9 +106,15 @@ struct iommu_table {
    >> unsigned long *it_map; /* A simple allocation bitmap for now */
    >> unsigned long it_page_shift;/* table iommu page size */
    >> struct iommu_table_group *it_group;
    >> + unsigned long *it_userspace; /* userspace view of the table */
    >> struct iommu_table_ops *it_ops;
    >> };
    >>
    >> +#define IOMMU_TABLE_USERSPACE_ENTRY(tbl, entry) \
    >> + ((tbl)->it_userspace ? \
    >> + &((tbl)->it_userspace[(entry) - (tbl)->it_offset]) : \
    >> + NULL)
    >> +
    >> /* Pure 2^n version of get_order */
    >> static inline __attribute_const__
    >> int get_iommu_order(unsigned long size, struct iommu_table *tbl)
    >> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
    >> index 0bcd988..82102d1 100644
    >> --- a/arch/powerpc/kernel/iommu.c
    >> +++ b/arch/powerpc/kernel/iommu.c
    >> @@ -38,6 +38,7 @@
    >> #include <linux/pci.h>
    >> #include <linux/iommu.h>
    >> #include <linux/sched.h>
    >> +#include <linux/vmalloc.h>
    >> #include <asm/io.h>
    >> #include <asm/prom.h>
    >> #include <asm/iommu.h>
    >> @@ -1069,6 +1070,9 @@ static int iommu_table_take_ownership(struct iommu_table *tbl)
    >> spin_unlock(&tbl->pools[i].lock);
    >> spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
    >>
    >> + BUG_ON(tbl->it_userspace);
    >> + tbl->it_userspace = vzalloc(sizeof(*tbl->it_userspace) * tbl->it_size);
    >> +
    >
    > -ENOMEM?
    >
    >> return 0;
    >> }
    >>
    >> @@ -1102,6 +1106,9 @@ static void iommu_table_release_ownership(struct iommu_table *tbl)
    >> {
    >> unsigned long flags, i, sz = (tbl->it_size + 7) >> 3;
    >>
    >> + vfree(tbl->it_userspace);
    >> + tbl->it_userspace = NULL;
    >> +
    >> spin_lock_irqsave(&tbl->large_pool.lock, flags);
    >> for (i = 0; i < tbl->nr_pools; i++)
    >> spin_lock(&tbl->pools[i].lock);
    >> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
    >> index bc36cf1..036f3c1 100644
    >> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
    >> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
    >> @@ -26,6 +26,7 @@
    >> #include <linux/iommu.h>
    >> #include <linux/mmzone.h>
    >> #include <linux/sizes.h>
    >> +#include <linux/vmalloc.h>
    >>
    >> #include <asm/mmzone.h>
    >> #include <asm/sections.h>
    >> @@ -1469,6 +1470,9 @@ static void pnv_pci_free_table(struct iommu_table *tbl)
    >> if (!tbl->it_size)
    >> return;
    >>
    >> + if (tbl->it_userspace)
    >
    > Not necessary

    Out of curiosity - why? Is every single implementation is known for
    checking the argument?


    >
    >> + vfree(tbl->it_userspace);
    >> +
    >
    > Why no NULL setting this time?

    iommu_reset_table() (2 lines below) will do memset(0) on the entire struct.


    >
    >> pnv_free_tce_table(tbl->it_base, size, tbl->it_indirect_levels);
    >> iommu_reset_table(tbl, "ioda2");
    >> }
    >> @@ -1656,9 +1660,26 @@ static void pnv_ioda2_set_ownership(struct iommu_table_group *table_group,
    >> pnv_pci_ioda2_set_bypass(pe, !enable);
    >> }
    >>
    >> +static long pnv_pci_ioda2_create_table_with_uas(
    >> + struct iommu_table_group *table_group,
    >> + int num, __u32 page_shift, __u64 window_size, __u32 levels,
    >> + struct iommu_table *tbl)
    >> +{
    >> + long ret = pnv_pci_ioda2_create_table(table_group, num,
    >> + page_shift, window_size, levels, tbl);
    >> +
    >> + if (ret)
    >> + return ret;
    >> +
    >> + BUG_ON(tbl->it_userspace);
    >> + tbl->it_userspace = vzalloc(sizeof(*tbl->it_userspace) * tbl->it_size);
    >
    > -ENOMEM
    >
    >> +
    >> + return 0;
    >> +}
    >> +
    >> static struct iommu_table_group_ops pnv_pci_ioda2_ops = {
    >> .set_ownership = pnv_ioda2_set_ownership,
    >> - .create_table = pnv_pci_ioda2_create_table,
    >> + .create_table = pnv_pci_ioda2_create_table_with_uas,
    >> .set_window = pnv_pci_ioda2_set_window,
    >> .unset_window = pnv_pci_ioda2_unset_window,
    >> };
    >
    >
    >


    Thanks for the review! What is overall resume? Another respin?



    --
    Alexey


    \
     
     \ /
      Last update: 2015-04-08 06:01    [W:2.837 / U:0.368 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site