Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 05/09/2018 19:18, Jacob Pan wrote:
> On Wed, 5 Sep 2018 14:14:12 +0200
> Auger Eric  wrote:
> 
>>> + *
>>> + * On Arm and AMD IOMMUs, entry 0 of the PASID table can be used
>>> to hold
>>> + * non-PASID translations. In this case PASID 0 is reserved and
>>> entry 0 points
>>> + * to the io_pgtable base. On Intel IOMMU, the io_pgtable base
>>> would be held in
>>> + * the device table and PASID 0 would be available to the
>>> allocator.
>>> + */  
>> very nice explanation
> With the new Vt-d 3.0 spec., 2nd level IO page table base is no longer
> held in the device context table. Instead it is held in the PASID table
> entry pointed by the RID_PASID field in the device context entry. If
> RID_PASID = 0, then it is the same as ARM and AMD IOMMUs.
> You can refer to ch3.4.3 of the VT-d spec.

I could simplify that paragraph by removing the specific implementations:

"In some IOMMUs, entry 0 of the PASID table can be used to hold
non-PASID translations. In this case PASID 0 is reserved and entry 0
points to the io_pgtable base. In other IOMMUs the io_pgtable base is
held in the device table and PASID 0 is available to the allocator."

I guess in Linux there isn't any reason to set RID_PASID to a non-zero
value? Otherwise the iommu-sva allocator will need minor changes.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 05/09/2018 13:14, Auger Eric wrote:
>> +static struct io_mm *
>> +io_mm_alloc(struct iommu_domain *domain, struct device *dev,
>> +struct mm_struct *mm, unsigned long flags)
>> +{
>> +int ret;
>> +int pasid;
>> +struct io_mm *io_mm;
>> +struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
> don't you need to check param != NULL and flags are compatible with
> those set at init?

It would be redundant, parameters are already checked by bind().
Following your comment below I think this function should also be called
under iommu_param->lock

>> +idr_preload(GFP_KERNEL);
>> +spin_lock(_sva_lock);
>> +pasid = idr_alloc(_pasid_idr, io_mm, param->min_pasid,
>> +  param->max_pasid + 1, GFP_ATOMIC);
> isn't it param->max_pasid - 1?

max_pasid is the last allocatable PASID, and the 'end' parameter of
idr_alloc is exclusive, so this needs to be max_pasid + 1.

>> +static int io_mm_attach(struct iommu_domain *domain, struct device *dev,
>> +struct io_mm *io_mm, void *drvdata)
>> +{
>> +int ret;
>> +bool attach_domain = true;
>> +int pasid = io_mm->pasid;
>> +struct iommu_bond *bond, *tmp;
>> +struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
>> +if (!domain->ops->mm_attach || !domain->ops->mm_detach)
>> +return -ENODEV;
> don't you need to check param is not NULL?

As mm_alloc, this is called by bind() which already performs argument checks

>> +
>> +if (pasid > param->max_pasid || pasid < param->min_pasid)
> pasid >= param->max_pasid ?

max_pasid is inclusive

>> +ret = domain->ops->mm_attach(domain, dev, io_mm, attach_domain);
> the fact the mm_attach/detach() must not sleep may be documented in the
> API doc.

Ok

>>  int __iommu_sva_bind_device(struct device *dev, struct mm_struct *mm,
>>  int *pasid, unsigned long flags, void *drvdata)
>>  {
>> -return -ENOSYS; /* TODO */
>> +int i, ret = 0;
>> +struct io_mm *io_mm = NULL;
>> +struct iommu_domain *domain;
>> +struct iommu_bond *bond = NULL, *tmp;
>> +struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
>> +domain = iommu_get_domain_for_dev(dev);
>> +if (!domain)
>> +return -EINVAL;
>> +
>> +/*
>> + * The device driver does not call sva_device_init/shutdown and
>> + * bind/unbind concurrently, so no need to take the param lock.
>> + */
> what does guarantee that?

The doc for iommu_sva_bind_device mandates that iommu_sva_device_init()
is called before bind(), but nothing is said about unbind and shutdown.
I think that was just based on the assumption that the device driver
doesn't have any reason to call unbind and shutdown concurrently, but
taking the lock here and in unbind is probably safer.

>> +ret = io_mm_attach(domain, dev, io_mm, drvdata);
>> +if (ret)
>> +io_mm_put(io_mm);
> dont't you want to free the io_mm if just allocated?

We do: if the io_mm has just been allocated, it has a single reference
so io_mm_put frees it.

>> + * @mm_attach: attach io_mm to a device. Install PASID entry if necessary
>> + * @mm_detach: detach io_mm from a device. Remove PASID entry and
>> + * flush associated TLB entries.
> if necessary too?

Right

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jacob Pan]

On Wed, 5 Sep 2018 14:14:12 +0200
Auger Eric  wrote:

> > + *
> > + * On Arm and AMD IOMMUs, entry 0 of the PASID table can be used
> > to hold
> > + * non-PASID translations. In this case PASID 0 is reserved and
> > entry 0 points
> > + * to the io_pgtable base. On Intel IOMMU, the io_pgtable base
> > would be held in
> > + * the device table and PASID 0 would be available to the
> > allocator.
> > + */  
> very nice explanation
With the new Vt-d 3.0 spec., 2nd level IO page table base is no longer
held in the device context table. Instead it is held in the PASID table
entry pointed by the RID_PASID field in the device context entry. If
RID_PASID = 0, then it is the same as ARM and AMD IOMMUs.
You can refer to ch3.4.3 of the VT-d spec.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Auger Eric]

Hi Jean-Philippe,

On 05/11/2018 09:06 PM, Jean-Philippe Brucker wrote:
> Allocate IOMMU mm structures and binding them to devices. Four operations
s/binding/bind
> are added to IOMMU drivers:
> 
> * mm_alloc(): to create an io_mm structure and perform architecture-
>   specific operations required to grab the process (for instance on ARM,
>   pin down the CPU ASID so that the process doesn't get assigned a new
>   ASID on rollover).
> 
>   There is a single valid io_mm structure per Linux mm. Future extensions
>   may also use io_mm for kernel-managed address spaces, populated with
>   map()/unmap() calls instead of bound to process address spaces. This
>   patch focuses on "shared" io_mm.
> 
> * mm_attach(): attach an mm to a device. The IOMMU driver checks that the
>   device is capable of sharing an address space, and writes the PASID
>   table entry to install the pgd.
> 
>   Some IOMMU drivers will have a single PASID table per domain, for
>   convenience. Other can implement it differently but to help these
>   drivers, mm_attach and mm_detach take 'attach_domain' and
>   'detach_domain' parameters, that tell whether they need to set and clear
>   the PASID entry or only send the required TLB invalidations.
> 
> * mm_detach(): detach an mm from a device. The IOMMU driver removes the
>   PASID table entry and invalidates the IOTLBs.
> 
> * mm_free(): free a structure allocated by mm_alloc(), and let arch
>   release the process.
> 
> mm_attach and mm_detach operations are serialized with a spinlock. When
> trying to optimize this code, we should at least prevent concurrent
> attach()/detach() on the same domain (so multi-level PASID table code can
> allocate tables lazily). mm_alloc() can sleep, but mm_free must not
> (because we'll have to call it from call_srcu later on).
> 
> At the moment we use an IDR for allocating PASIDs and retrieving contexts.
> We also use a single spinlock. These can be refined and optimized later (a
> custom allocator will be needed for top-down PASID allocation).
> 
> Keeping track of address spaces requires the use of MMU notifiers.
> Handling process exit with regard to unbind() is tricky, so it is left for
> another patch and we explicitly fail mm_alloc() for the moment.
> 
> Signed-off-by: Jean-Philippe Brucker 
> 
> ---
> v1->v2: sanity-check of flags
> ---
>  drivers/iommu/iommu-sva.c | 380 +-
>  drivers/iommu/iommu.c |   1 +
>  include/linux/iommu.h |  28 +++
>  3 files changed, 406 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 8d98f9c09864..6ac679c48f3c 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -5,8 +5,298 @@
>   * Copyright (C) 2018 ARM Ltd.
>   */
>  
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
> +
> +/**
> + * DOC: io_mm model
> + *
> + * The io_mm keeps track of process address spaces shared between CPU and 
> IOMMU.
> + * The following example illustrates the relation between structures
> + * iommu_domain, io_mm and iommu_bond. An iommu_bond is a link between io_mm 
> and
> + * device. A device can have multiple io_mm and an io_mm may be bound to
> + * multiple devices.
> + *  ___
> + * |  IOMMU domain A   |
> + * |   |
> + * | |  IOMMU group   |+--- io_pgtables
> + * | |||
> + * | |   dev 00:00.0 +--- bond --- io_mm X
> + * | ||   \|
> + * |   '- bond ---.
> + * |___|   \
> + *  ___ \
> + * |  IOMMU domain B   |   io_mm Y
> + * |   |   / /
> + * | |  IOMMU group   ||  / /
> + * | ||| / /
> + * | |   dev 00:01.0  bond -' /
> + * | |   dev 00:01.1  bond --'
> + * | |||
> + * |   +--- io_pgtables
> + * |___|
> + *
> + * In this example, device 00:00.0 is in domain A, devices 00:01.* are in 
> domain
> + * B. All devices within the same domain access the same address spaces. 
> Device
> + * 00:00.0 accesses address spaces X and Y, each corresponding to an 
> mm_struct.
> + * Devices 00:01.* only access address space Y. In addition each
> + * IOMMU_DOMAIN_DMA domain has a private address space, io_pgtable, that is
> + * managed with iommu_map()/iommu_unmap(), and isn't shared with the CPU MMU.
> + *
> + * To obtain the above configuration, users would for instance issue the
> + * following calls:
> + *
> + * iommu_sva_bind_device(dev 00:00.0, mm X, ...) -> PASID 1
> + * 

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Kenneth Lee]

On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron 
> To: Ilias Apalodimas 
> CC: Jean-Philippe Brucker ,
>  "xieyishe...@huawei.com" , "k...@vger.kernel.org"
>  , "linux-...@vger.kernel.org"
>  , "xuza...@huawei.com" ,
>  Will Deacon , "ok...@codeaurora.org"
>  , "linux...@kvack.org" ,
>  "yi.l@intel.com" , "ashok@intel.com"
>  , "t...@semihalf.com" ,
>  "j...@8bytes.org" , "robdcl...@gmail.com"
>  , "bhara...@xilinx.com" ,
>  "linux-a...@vger.kernel.org" ,
>  "liudongdo...@huawei.com" , "rfr...@cavium.com"
>  , "devicet...@vger.kernel.org"
>  , "kevin.t...@intel.com"
>  , Jacob Pan ,
>  "alex.william...@redhat.com" ,
>  "rgum...@xilinx.com" , "thunder.leiz...@huawei.com"
>  , "linux-arm-ker...@lists.infradead.org"
>  , "shunyong.y...@hxt-semitech.com"
>  , "dw...@infradead.org"
>  , "liub...@huawei.com" ,
>  "jcro...@codeaurora.org" ,
>  "iommu@lists.linux-foundation.org" ,
>  Robin Murphy , "christian.koe...@amd.com"
>  , "nwatt...@codeaurora.org"
>  , "baolu...@linux.intel.com"
>  , liguo...@hisilicon.com,
>  kenneth-lee-2...@foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.4...@huawei.com>
> Organization: Huawei
> X-Mailer: Claws Mail 3.15.0 (GTK+ 2.24.31; x86_64-w64-mingw32)
> 
> +CC Kenneth Lee
> 
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas  wrote:
> 
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I 
> > > >> think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?  
> > > > 
> > > > There has been some non upstreamed efforts to have mdev and produce 
> > > > userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > > devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we 
> > > > choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there 
> > > > would be
> > > > good.  
> > > 
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?  
> > The best reason i can come up with is "common code". You already have one 
> > API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand 
> > it's
> > a possible case.

Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035

The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for
them.

> > > 
> > > Thanks,
> > > Jean  
> 
> 

(p.s. I sent this mail on May 26 from my public email count. But it
seems the email server is blocked. I resent it from my company count until my
colleague told me just now. Sorry for inconvenience)

-- 
-Kenneth(Hisilicon)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Kenneth Lee]

On Sat, May 26, 2018 at 10:24:45AM +0800, Kenneth Lee wrote:
> Date: Sat, 26 May 2018 10:24:45 +0800
> From: Kenneth Lee 
> To: Jonathan Cameron 
> Cc: Ilias Apalodimas , Jean-Philippe Brucker
>  , "xieyishe...@huawei.com"
>  , "k...@vger.kernel.org" ,
>  "linux-...@vger.kernel.org" ,
>  "xuza...@huawei.com" , Will Deacon
>  , "ok...@codeaurora.org" ,
>  "linux...@kvack.org" , "yi.l@intel.com"
>  , "ashok@intel.com" ,
>  "t...@semihalf.com" , "j...@8bytes.org" ,
>  "robdcl...@gmail.com" , "bhara...@xilinx.com"
>  , "linux-a...@vger.kernel.org"
>  , "liudongdo...@huawei.com"
>  , "rfr...@cavium.com" ,
>  "devicet...@vger.kernel.org" ,
>  "kevin.t...@intel.com" , Jacob Pan
>  , "alex.william...@redhat.com"
>  , "rgum...@xilinx.com" ,
>  "thunder.leiz...@huawei.com" ,
>  "linux-arm-ker...@lists.infradead.org"
>  , "shunyong.y...@hxt-semitech.com"
>  , "dw...@infradead.org"
>  , "liub...@huawei.com" ,
>  "jcro...@codeaurora.org" ,
>  "iommu@lists.linux-foundation.org" ,
>  Robin Murphy , "christian.koe...@amd.com"
>  , "nwatt...@codeaurora.org"
>  , "baolu...@linux.intel.com"
>  , liguo...@hisilicon.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180526022445.GA6069@kllp05>
> 
> On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> > Date: Fri, 25 May 2018 09:39:59 +0100
> > From: Jonathan Cameron 
> > To: Ilias Apalodimas 
> > CC: Jean-Philippe Brucker ,
> >  "xieyishe...@huawei.com" , "k...@vger.kernel.org"
> >  , "linux-...@vger.kernel.org"
> >  , "xuza...@huawei.com" ,
> >  Will Deacon , "ok...@codeaurora.org"
> >  , "linux...@kvack.org" ,
> >  "yi.l@intel.com" , "ashok@intel.com"
> >  , "t...@semihalf.com" ,
> >  "j...@8bytes.org" , "robdcl...@gmail.com"
> >  , "bhara...@xilinx.com" ,
> >  "linux-a...@vger.kernel.org" ,
> >  "liudongdo...@huawei.com" , "rfr...@cavium.com"
> >  , "devicet...@vger.kernel.org"
> >  , "kevin.t...@intel.com"
> >  , Jacob Pan ,
> >  "alex.william...@redhat.com" ,
> >  "rgum...@xilinx.com" , "thunder.leiz...@huawei.com"
> >  , "linux-arm-ker...@lists.infradead.org"
> >  , "shunyong.y...@hxt-semitech.com"
> >  , "dw...@infradead.org"
> >  , "liub...@huawei.com" ,
> >  "jcro...@codeaurora.org" ,
> >  "iommu@lists.linux-foundation.org" ,
> >  Robin Murphy , "christian.koe...@amd.com"
> >  , "nwatt...@codeaurora.org"
> >  , "baolu...@linux.intel.com"
> >  , liguo...@hisilicon.com,
> >  kenneth-lee-2...@foxmail.com
> > Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> > Message-ID: <20180525093959.4...@huawei.com>
> > 
> > +CC Kenneth Lee
> > 
> > On Fri, 25 May 2018 09:33:11 +0300
> > Ilias Apalodimas  wrote:
> > 
> > > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > > >> thought you were talking about mdev devices assigned to VMs, but I 
> > > > >> think
> > > > >> you're referring to mdevs assigned to userspace drivers instead? Out 
> > > > >> of
> > > > >> curiosity, is it only theoretical or does someone actually need 
> > > > >> this?  
> > > > > 
> > > > > There has been some non upstreamed efforts to have mdev and produce 
> > > > > userspace
> > > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > > > devices and
> > > > > we did a proof of concept for ethernet interfaces. At the time we 
> > > > > choose not to
> > > > > involve the IOMMU for the reason you mentioned, but having it there 
> > > > > would be
> > > > > good.  
> > > > 
> > > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > > indirection, and since the mediating driver has to implement these
> > > > operations already, what is gained?  
> > > The best reason i can come up with is "common code". You already have one 
> > > API
> > > doing that for you so we replicate it in a /dev file?
> > > The mdev approach still needs extentions to support what we tried to do 
> > > (i.e
> > > mdev bus might need yo have access on iommu_ops), but as far as i 
> > > undestand it's
> > > a possible case.
> 
> Hi, Jean, Please allow me to share my understanding here:
> https://zhuanlan.zhihu.com/p/35489035
> 
> The reason we do not use the /dev/foo scheme is that the devices to be
> shared are programmable accelerators. We cannot fix up the kernel driver for 
> them.
> > > > 
> > > > Thanks,
> > > > Jean  
> > 
> > 
> 
> -- 
>   -Kenneth Lee (Hisilicon)

I just found this mail was missed in the mailing list. I tried it once
again.

-- 
-Kenneth Lee (Hisilicon)


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Kenneth Lee]

On Fri, May 25, 2018 at 09:39:59AM +0100, Jonathan Cameron wrote:
> Date: Fri, 25 May 2018 09:39:59 +0100
> From: Jonathan Cameron <jonathan.came...@huawei.com>
> To: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> CC: Jean-Philippe Brucker <jean-philippe.bruc...@arm.com>,
>  "xieyishe...@huawei.com" <xieyishe...@huawei.com>, "k...@vger.kernel.org"
>  <k...@vger.kernel.org>, "linux-...@vger.kernel.org"
>  <linux-...@vger.kernel.org>, "xuza...@huawei.com" <xuza...@huawei.com>,
>  Will Deacon <will.dea...@arm.com>, "ok...@codeaurora.org"
>  <ok...@codeaurora.org>, "linux...@kvack.org" <linux...@kvack.org>,
>  "yi.l@intel.com" <yi.l@intel.com>, "ashok@intel.com"
>  <ashok@intel.com>, "t...@semihalf.com" <t...@semihalf.com>,
>  "j...@8bytes.org" <j...@8bytes.org>, "robdcl...@gmail.com"
>  <robdcl...@gmail.com>, "bhara...@xilinx.com" <bhara...@xilinx.com>,
>  "linux-a...@vger.kernel.org" <linux-a...@vger.kernel.org>,
>  "liudongdo...@huawei.com" <liudongdo...@huawei.com>, "rfr...@cavium.com"
>  <rfr...@cavium.com>, "devicet...@vger.kernel.org"
>  <devicet...@vger.kernel.org>, "kevin.t...@intel.com"
>  <kevin.t...@intel.com>, Jacob Pan <jacob.jun@linux.intel.com>,
>  "alex.william...@redhat.com" <alex.william...@redhat.com>,
>  "rgum...@xilinx.com" <rgum...@xilinx.com>, "thunder.leiz...@huawei.com"
>  <thunder.leiz...@huawei.com>, "linux-arm-ker...@lists.infradead.org"
>  <linux-arm-ker...@lists.infradead.org>, "shunyong.y...@hxt-semitech.com"
>  <shunyong.y...@hxt-semitech.com>, "dw...@infradead.org"
>  <dw...@infradead.org>, "liub...@huawei.com" <liub...@huawei.com>,
>  "jcro...@codeaurora.org" <jcro...@codeaurora.org>,
>  "iommu@lists.linux-foundation.org" <iommu@lists.linux-foundation.org>,
>  Robin Murphy <robin.mur...@arm.com>, "christian.koe...@amd.com"
>  <christian.koe...@amd.com>, "nwatt...@codeaurora.org"
>  <nwatt...@codeaurora.org>, "baolu...@linux.intel.com"
>  <baolu...@linux.intel.com>, liguo...@hisilicon.com,
>  kenneth-lee-2...@foxmail.com
> Subject: Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces
> Message-ID: <20180525093959.4...@huawei.com>
> 
> +CC Kenneth Lee
> 
> On Fri, 25 May 2018 09:33:11 +0300
> Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote:
> 
> > On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > > >> Interesting, I hadn't thought about this use-case before. At first I
> > > >> thought you were talking about mdev devices assigned to VMs, but I 
> > > >> think
> > > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > > >> curiosity, is it only theoretical or does someone actually need this?  
> > > > 
> > > > There has been some non upstreamed efforts to have mdev and produce 
> > > > userspace
> > > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > > devices and
> > > > we did a proof of concept for ethernet interfaces. At the time we 
> > > > choose not to
> > > > involve the IOMMU for the reason you mentioned, but having it there 
> > > > would be
> > > > good.  
> > > 
> > > I'm guessing there were good reasons to do it that way but I wonder, is
> > > it not simpler to just have the kernel driver create a /dev/foo, with a
> > > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > > indirection, and since the mediating driver has to implement these
> > > operations already, what is gained?  
> > The best reason i can come up with is "common code". You already have one 
> > API
> > doing that for you so we replicate it in a /dev file?
> > The mdev approach still needs extentions to support what we tried to do (i.e
> > mdev bus might need yo have access on iommu_ops), but as far as i undestand 
> > it's
> > a possible case.

Hi, Jean, Please allow me to share my understanding here:
https://zhuanlan.zhihu.com/p/35489035

The reason we do not use the /dev/foo scheme is that the devices to be
shared are programmable accelerators. We cannot fix up the kernel driver for 
them.
> > > 
> > > Thanks,
> > > Jean  
> 
> 

-- 
-Kenneth Lee (Hisilicon)


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jonathan Cameron]

+CC Kenneth Lee

On Fri, 25 May 2018 09:33:11 +0300
Ilias Apalodimas  wrote:

> On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> > On 24/05/18 12:50, Ilias Apalodimas wrote:  
> > >> Interesting, I hadn't thought about this use-case before. At first I
> > >> thought you were talking about mdev devices assigned to VMs, but I think
> > >> you're referring to mdevs assigned to userspace drivers instead? Out of
> > >> curiosity, is it only theoretical or does someone actually need this?  
> > > 
> > > There has been some non upstreamed efforts to have mdev and produce 
> > > userspace
> > > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > > devices and
> > > we did a proof of concept for ethernet interfaces. At the time we choose 
> > > not to
> > > involve the IOMMU for the reason you mentioned, but having it there would 
> > > be
> > > good.  
> > 
> > I'm guessing there were good reasons to do it that way but I wonder, is
> > it not simpler to just have the kernel driver create a /dev/foo, with a
> > standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> > indirection, and since the mediating driver has to implement these
> > operations already, what is gained?  
> The best reason i can come up with is "common code". You already have one API
> doing that for you so we replicate it in a /dev file?
> The mdev approach still needs extentions to support what we tried to do (i.e
> mdev bus might need yo have access on iommu_ops), but as far as i undestand 
> it's
> a possible case.
> > 
> > Thanks,
> > Jean  


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Ilias Apalodimas]

On Thu, May 24, 2018 at 04:04:39PM +0100, Jean-Philippe Brucker wrote:
> On 24/05/18 12:50, Ilias Apalodimas wrote:
> >> Interesting, I hadn't thought about this use-case before. At first I
> >> thought you were talking about mdev devices assigned to VMs, but I think
> >> you're referring to mdevs assigned to userspace drivers instead? Out of
> >> curiosity, is it only theoretical or does someone actually need this?
> > 
> > There has been some non upstreamed efforts to have mdev and produce 
> > userspace
> > drivers. Huawei is using it on what they call "wrapdrive" for crypto 
> > devices and
> > we did a proof of concept for ethernet interfaces. At the time we choose 
> > not to
> > involve the IOMMU for the reason you mentioned, but having it there would be
> > good.
> 
> I'm guessing there were good reasons to do it that way but I wonder, is
> it not simpler to just have the kernel driver create a /dev/foo, with a
> standard ioctl/mmap/poll interface? Here VFIO adds a layer of
> indirection, and since the mediating driver has to implement these
> operations already, what is gained?
The best reason i can come up with is "common code". You already have one API
doing that for you so we replicate it in a /dev file?
The mdev approach still needs extentions to support what we tried to do (i.e
mdev bus might need yo have access on iommu_ops), but as far as i undestand it's
a possible case.
> 
> Thanks,
> Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 24/05/18 12:50, Ilias Apalodimas wrote:
>> Interesting, I hadn't thought about this use-case before. At first I
>> thought you were talking about mdev devices assigned to VMs, but I think
>> you're referring to mdevs assigned to userspace drivers instead? Out of
>> curiosity, is it only theoretical or does someone actually need this?
> 
> There has been some non upstreamed efforts to have mdev and produce userspace
> drivers. Huawei is using it on what they call "wrapdrive" for crypto devices 
> and
> we did a proof of concept for ethernet interfaces. At the time we choose not 
> to
> involve the IOMMU for the reason you mentioned, but having it there would be
> good.

I'm guessing there were good reasons to do it that way but I wonder, is
it not simpler to just have the kernel driver create a /dev/foo, with a
standard ioctl/mmap/poll interface? Here VFIO adds a layer of
indirection, and since the mediating driver has to implement these
operations already, what is gained?

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Ilias Apalodimas]

> Interesting, I hadn't thought about this use-case before. At first I
> thought you were talking about mdev devices assigned to VMs, but I think
> you're referring to mdevs assigned to userspace drivers instead? Out of
> curiosity, is it only theoretical or does someone actually need this?

There has been some non upstreamed efforts to have mdev and produce userspace
drivers. Huawei is using it on what they call "wrapdrive" for crypto devices and
we did a proof of concept for ethernet interfaces. At the time we choose not to
involve the IOMMU for the reason you mentioned, but having it there would be
good.

Thanks
Ilias
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 22/05/18 17:43, Jacob Pan wrote:
> On Thu, 17 May 2018 11:02:42 +0100
> Jean-Philippe Brucker  wrote:
> 
>> On 17/05/18 00:31, Jacob Pan wrote:
>>> On Fri, 11 May 2018 20:06:04 +0100
>>> I am a little confused about domain vs. pasid relationship. If
>>> each domain represents a address space, should there be a domain for
>>> each pasid?  
>>
>> I don't think there is a formal definition, but from previous
>> discussion the consensus seems to be: domains are a collection of
>> devices that have the same virtual address spaces (one or many).
>>
>> Keeping that definition makes things easier, in my opinion. Some time
>> ago, I did try to represent PASIDs using "subdomains" (introducing a
>> hierarchy of struct iommu_domain), but it required invasive changes in
>> the IOMMU subsystem and probably all over the tree.
>>
>> You do need some kind of "root domain" for each device, so that
>> "iommu_get_domain_for_dev()" still makes sense. That root domain
>> doesn't have a single address space but a collection of subdomains.
>> If you need this anyway, representing a PASID with an iommu_domain
>> doesn't seem preferable than using a different structure (io_mm),
>> because they don't have anything in common.
>>
> My main concern is the PASID table storage. If PASID table storage
> is tied to domain, it is ok to scale up, i.e. multiple devices in a
> domain share a single PASID table. But to scale down, e.g. further
> partition device with VFIO mdev for assignment, each mdev may get its
> own domain via vfio. But there is no IOMMU storage for PASID table at
> mdev device level. Perhaps we do need root domain or some parent-child
> relationship to locate PASID table.

Interesting, I hadn't thought about this use-case before. At first I
thought you were talking about mdev devices assigned to VMs, but I think
you're referring to mdevs assigned to userspace drivers instead? Out of
curiosity, is it only theoretical or does someone actually need this?

I don't think mdev for VM assignment are compatible with PASID, at least
not when the IOMMU is involved. I usually ignore mdev in my reasoning
because, as far as I know, currently they only affect devices that have
their own MMU, and IOMMU domains don't come into play. However, if a
device was backed by the IOMMU, and the device driver wanted to
partition it into mdevs, then users would be tempted to assign mdev1 to
VM1 and mdev2 to VM2.

It doesn't work with PASID, because the PASID spaces of VM1 and VM2 will
conflict. If both VM1 and VM2 allocate PASID #1, then the host has to
somehow arbitrate device accesses, for example scheduling first mdev1
then mdev2. That's possible if the device driver is in charge of the
MMU, but not really compatible with the IOMMU.

So in the IOMMU subsystem, for assigning devices to VMs the only
model that makes sense is SR-IOV, where each VF/mdev has its own RID and
its own PASID table. In that case you'd get one IOMMU domain per VF.


But considering userspace drivers in the host alone, it might make sense
to partition a device into mdevs and assign them to multiple processes.
Interestingly this scheme still doesn't work with the classic MAP/UNMAP
ioctl: since there is a single device context entry for all mdevs, the
mediating driver would have to catch all MAP/UNMAP ioctls and reject
those with IOVAs that overlap those of another mdev. It's doesn't seem
viable. But when using PASID then each mdev has its own address space,
and since PASIDs are allocated by the kernel there is no such conflict.

Anyway, I think this use-case can work with the current structures, if
mediating driver does the bind() instead of VFIO. That's necessary
because you can't let userspace program the PASID into the device, or
they would be able to access address spaces owned by other mdevs. Then
the mediating driver does the bind(), and keeps internal structures to
associate the process to the given mdev.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jacob Pan]

On Thu, 17 May 2018 11:02:42 +0100
Jean-Philippe Brucker  wrote:

> On 17/05/18 00:31, Jacob Pan wrote:
> > On Fri, 11 May 2018 20:06:04 +0100
> > I am a little confused about domain vs. pasid relationship. If
> > each domain represents a address space, should there be a domain for
> > each pasid?  
> 
> I don't think there is a formal definition, but from previous
> discussion the consensus seems to be: domains are a collection of
> devices that have the same virtual address spaces (one or many).
> 
> Keeping that definition makes things easier, in my opinion. Some time
> ago, I did try to represent PASIDs using "subdomains" (introducing a
> hierarchy of struct iommu_domain), but it required invasive changes in
> the IOMMU subsystem and probably all over the tree.
> 
> You do need some kind of "root domain" for each device, so that
> "iommu_get_domain_for_dev()" still makes sense. That root domain
> doesn't have a single address space but a collection of subdomains.
> If you need this anyway, representing a PASID with an iommu_domain
> doesn't seem preferable than using a different structure (io_mm),
> because they don't have anything in common.
> 
My main concern is the PASID table storage. If PASID table storage
is tied to domain, it is ok to scale up, i.e. multiple devices in a
domain share a single PASID table. But to scale down, e.g. further
partition device with VFIO mdev for assignment, each mdev may get its
own domain via vfio. But there is no IOMMU storage for PASID table at
mdev device level. Perhaps we do need root domain or some parent-child
relationship to locate PASID table.

> Thanks,
> Jean

[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 17/05/18 15:25, Jonathan Cameron wrote:
>> +static struct io_mm *
>> +io_mm_alloc(struct iommu_domain *domain, struct device *dev,
>> +struct mm_struct *mm, unsigned long flags)
>> +{
>> +int ret;
>> +int pasid;
>> +struct io_mm *io_mm;
>> +struct iommu_sva_param *param = dev->iommu_param->sva_param;
>> +
>> +if (!domain->ops->mm_alloc || !domain->ops->mm_free)
>> +return ERR_PTR(-ENODEV);
>> +
>> +io_mm = domain->ops->mm_alloc(domain, mm, flags);
>> +if (IS_ERR(io_mm))
>> +return io_mm;
>> +if (!io_mm)
>> +return ERR_PTR(-ENOMEM);
>> +
>> +/*
>> + * The mm must not be freed until after the driver frees the io_mm
>> + * (which may involve unpinning the CPU ASID for instance, requiring a
>> + * valid mm struct.)
>> + */
>> +mmgrab(mm);
>> +
>> +io_mm->flags= flags;
>> +io_mm->mm   = mm;
>> +io_mm->release  = domain->ops->mm_free;
>> +INIT_LIST_HEAD(_mm->devices);
>> +
>> +idr_preload(GFP_KERNEL);
>> +spin_lock(_sva_lock);
>> +pasid = idr_alloc(_pasid_idr, io_mm, param->min_pasid,
>> +  param->max_pasid + 1, GFP_ATOMIC);
> 
> I'd expect the IDR cleanup to be in io_mm_free as that would 'match'
> against io_mm_alloc but it's in io_mm_release just before the io_mm_free
> call, perhaps move it or am I missing something?
> 
> Hmm. This is reworked in patch 5 to use call rcu to do the free.  I suppose
> we may be burning an idr entry if we take a while to get round to the
> free..  If so a comment to explain this would be great.

Ok, I'll see if I can come up with some comments for both patch 3 and 5.

>> +io_mm->pasid = pasid;
>> +spin_unlock(_sva_lock);
>> +idr_preload_end();
>> +
>> +if (pasid < 0) {
>> +ret = pasid;
>> +goto err_free_mm;
>> +}
>> +
>> +/* TODO: keep track of mm. For the moment, abort. */
> 
> From later patches, I can now see why we didn't init the kref
> here, but perhaps a comment would make that clear rather than
> people checking it is correctly used throughout?  Actually just grab
> the comment from patch 5 and put it in this one and that will
> do the job nicely.

Ok

>> +ret = -ENOSYS;
>> +spin_lock(_sva_lock);
>> +idr_remove(_pasid_idr, io_mm->pasid);
>> +spin_unlock(_sva_lock);
>> +
>> +err_free_mm:
>> +domain->ops->mm_free(io_mm);
> 
> Really minor, but you now have io_mm->release set so to keep
> this obviously the same as the io_mm_free path, perhaps call
> that rather than mm_free directly.

Yes, makes sense

>> +static void io_mm_detach_locked(struct iommu_bond *bond)
>> +{
>> +struct iommu_bond *tmp;
>> +bool detach_domain = true;
>> +struct iommu_domain *domain = bond->domain;
>> +
>> +list_for_each_entry(tmp, >mm_list, domain_head) {
>> +if (tmp->io_mm == bond->io_mm && tmp->dev != bond->dev) {
>> +detach_domain = false;
>> +break;
>> +}
>> +}
>> +
>> +domain->ops->mm_detach(domain, bond->dev, bond->io_mm, detach_domain);
>> +
> 
> I can't see an immediate reason to have a different order in her to the 
> reverse of
> the attach above.   So I think you should be detaching after the list_del 
> calls.
> If there is a reason, can we have a comment so I don't ask on v10.

I don't see a reason either right now, I'll see if it can be moved

> 
>> +list_del(>mm_head);
>> +list_del(>domain_head);
>> +list_del(>dev_head);
>> +io_mm_put_locked(bond->io_mm);


>> +/* If an io_mm already exists, use it */
>> +spin_lock(_sva_lock);
>> +idr_for_each_entry(_pasid_idr, io_mm, i) {
>> +if (io_mm->mm == mm && io_mm_get_locked(io_mm)) {
>> +/* ... Unless it's already bound to this device */
>> +list_for_each_entry(tmp, _mm->devices, mm_head) {
>> +if (tmp->dev == dev) {
>> +bond = tmp;
> 
> Using bond for this is clear in a sense, but can we not just use ret
> so it is obvious here that we are going to return -EEXIST?
> At first glance I thought you were going to carry on with this bond
> and couldn't work out why it would ever make sense to have two bonds
> between a device an an io_mm (which it doesn't!)

Yes, using ret is nicer

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jonathan Cameron]

On Fri, 11 May 2018 20:06:04 +0100
Jean-Philippe Brucker  wrote:

> Allocate IOMMU mm structures and binding them to devices. Four operations
> are added to IOMMU drivers:
> 
> * mm_alloc(): to create an io_mm structure and perform architecture-
>   specific operations required to grab the process (for instance on ARM,
>   pin down the CPU ASID so that the process doesn't get assigned a new
>   ASID on rollover).
> 
>   There is a single valid io_mm structure per Linux mm. Future extensions
>   may also use io_mm for kernel-managed address spaces, populated with
>   map()/unmap() calls instead of bound to process address spaces. This
>   patch focuses on "shared" io_mm.
> 
> * mm_attach(): attach an mm to a device. The IOMMU driver checks that the
>   device is capable of sharing an address space, and writes the PASID
>   table entry to install the pgd.
> 
>   Some IOMMU drivers will have a single PASID table per domain, for
>   convenience. Other can implement it differently but to help these
>   drivers, mm_attach and mm_detach take 'attach_domain' and
>   'detach_domain' parameters, that tell whether they need to set and clear
>   the PASID entry or only send the required TLB invalidations.
> 
> * mm_detach(): detach an mm from a device. The IOMMU driver removes the
>   PASID table entry and invalidates the IOTLBs.
> 
> * mm_free(): free a structure allocated by mm_alloc(), and let arch
>   release the process.
> 
> mm_attach and mm_detach operations are serialized with a spinlock. When
> trying to optimize this code, we should at least prevent concurrent
> attach()/detach() on the same domain (so multi-level PASID table code can
> allocate tables lazily). mm_alloc() can sleep, but mm_free must not
> (because we'll have to call it from call_srcu later on).
> 
> At the moment we use an IDR for allocating PASIDs and retrieving contexts.
> We also use a single spinlock. These can be refined and optimized later (a
> custom allocator will be needed for top-down PASID allocation).
> 
> Keeping track of address spaces requires the use of MMU notifiers.
> Handling process exit with regard to unbind() is tricky, so it is left for
> another patch and we explicitly fail mm_alloc() for the moment.
> 
> Signed-off-by: Jean-Philippe Brucker 

A few minor bits and bobs inline.  Looks good in general + nice diags!

Thanks,

Jonathan

> 
> ---
> v1->v2: sanity-check of flags
> ---
>  drivers/iommu/iommu-sva.c | 380 +-
>  drivers/iommu/iommu.c |   1 +
>  include/linux/iommu.h |  28 +++
>  3 files changed, 406 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 8d98f9c09864..6ac679c48f3c 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -5,8 +5,298 @@
>   * Copyright (C) 2018 ARM Ltd.
>   */
>  
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
> +
> +/**
> + * DOC: io_mm model
> + *
> + * The io_mm keeps track of process address spaces shared between CPU and 
> IOMMU.
> + * The following example illustrates the relation between structures
> + * iommu_domain, io_mm and iommu_bond. An iommu_bond is a link between io_mm 
> and
> + * device. A device can have multiple io_mm and an io_mm may be bound to
> + * multiple devices.
> + *  ___
> + * |  IOMMU domain A   |
> + * |   |
> + * | |  IOMMU group   |+--- io_pgtables
> + * | |||
> + * | |   dev 00:00.0 +--- bond --- io_mm X
> + * | ||   \|
> + * |   '- bond ---.
> + * |___|   \
> + *  ___ \
> + * |  IOMMU domain B   |   io_mm Y
> + * |   |   / /
> + * | |  IOMMU group   ||  / /
> + * | ||| / /
> + * | |   dev 00:01.0  bond -' /
> + * | |   dev 00:01.1  bond --'
> + * | |||
> + * |   +--- io_pgtables
> + * |___|
> + *
> + * In this example, device 00:00.0 is in domain A, devices 00:01.* are in 
> domain
> + * B. All devices within the same domain access the same address spaces. 
> Device
> + * 00:00.0 accesses address spaces X and Y, each corresponding to an 
> mm_struct.
> + * Devices 00:01.* only access address space Y. In addition each
> + * IOMMU_DOMAIN_DMA domain has a private address space, io_pgtable, that is
> + * managed with iommu_map()/iommu_unmap(), and isn't shared with the CPU MMU.
> + *
> + * To obtain the above configuration, 

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

On 17/05/18 00:31, Jacob Pan wrote:
> On Fri, 11 May 2018 20:06:04 +0100
> I am a little confused about domain vs. pasid relationship. If
> each domain represents a address space, should there be a domain for
> each pasid?

I don't think there is a formal definition, but from previous discussion
the consensus seems to be: domains are a collection of devices that have
the same virtual address spaces (one or many).

Keeping that definition makes things easier, in my opinion. Some time
ago, I did try to represent PASIDs using "subdomains" (introducing a
hierarchy of struct iommu_domain), but it required invasive changes in
the IOMMU subsystem and probably all over the tree.

You do need some kind of "root domain" for each device, so that
"iommu_get_domain_for_dev()" still makes sense. That root domain doesn't
have a single address space but a collection of subdomains. If you need
this anyway, representing a PASID with an iommu_domain doesn't seem
preferable than using a different structure (io_mm), because they don't
have anything in common.

Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jacob Pan]

On Fri, 11 May 2018 20:06:04 +0100
Jean-Philippe Brucker  wrote:

> Allocate IOMMU mm structures and binding them to devices. Four
> operations are added to IOMMU drivers:
> 
> * mm_alloc(): to create an io_mm structure and perform architecture-
>   specific operations required to grab the process (for instance on
> ARM, pin down the CPU ASID so that the process doesn't get assigned a
> new ASID on rollover).
> 
>   There is a single valid io_mm structure per Linux mm. Future
> extensions may also use io_mm for kernel-managed address spaces,
> populated with map()/unmap() calls instead of bound to process
> address spaces. This patch focuses on "shared" io_mm.
> 
> * mm_attach(): attach an mm to a device. The IOMMU driver checks that
> the device is capable of sharing an address space, and writes the
> PASID table entry to install the pgd.
> 
>   Some IOMMU drivers will have a single PASID table per domain, for
>   convenience. Other can implement it differently but to help these
>   drivers, mm_attach and mm_detach take 'attach_domain' and
>   'detach_domain' parameters, that tell whether they need to set and
> clear the PASID entry or only send the required TLB invalidations.
> 
> * mm_detach(): detach an mm from a device. The IOMMU driver removes
> the PASID table entry and invalidates the IOTLBs.
> 
> * mm_free(): free a structure allocated by mm_alloc(), and let arch
>   release the process.
> 
> mm_attach and mm_detach operations are serialized with a spinlock.
> When trying to optimize this code, we should at least prevent
> concurrent attach()/detach() on the same domain (so multi-level PASID
> table code can allocate tables lazily). mm_alloc() can sleep, but
> mm_free must not (because we'll have to call it from call_srcu later
> on).
> 
> At the moment we use an IDR for allocating PASIDs and retrieving
> contexts. We also use a single spinlock. These can be refined and
> optimized later (a custom allocator will be needed for top-down PASID
> allocation).
> 
> Keeping track of address spaces requires the use of MMU notifiers.
> Handling process exit with regard to unbind() is tricky, so it is
> left for another patch and we explicitly fail mm_alloc() for the
> moment.
> 
> Signed-off-by: Jean-Philippe Brucker 
> 
> ---
> v1->v2: sanity-check of flags
> ---
>  drivers/iommu/iommu-sva.c | 380
> +- drivers/iommu/iommu.c |
> 1 + include/linux/iommu.h |  28 +++
>  3 files changed, 406 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index 8d98f9c09864..6ac679c48f3c 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -5,8 +5,298 @@
>   * Copyright (C) 2018 ARM Ltd.
>   */
>  
> +#include 
>  #include 
> +#include 
>  #include 
> +#include 
> +
> +/**
> + * DOC: io_mm model
> + *
> + * The io_mm keeps track of process address spaces shared between
> CPU and IOMMU.
> + * The following example illustrates the relation between structures
> + * iommu_domain, io_mm and iommu_bond. An iommu_bond is a link
> between io_mm and
> + * device. A device can have multiple io_mm and an io_mm may be
> bound to
> + * multiple devices.
> + *  ___
> + * |  IOMMU domain A   |
> + * |   |
> + * | |  IOMMU group   |+--- io_pgtables
> + * | |||
> + * | |   dev 00:00.0 +--- bond --- io_mm X
> + * | ||   \|
> + * |   '- bond ---.
> + * |___|   \
> + *  ___ \
> + * |  IOMMU domain B   |   io_mm Y
> + * |   |   / /
> + * | |  IOMMU group   ||  / /
> + * | ||| / /
> + * | |   dev 00:01.0  bond -' /
> + * | |   dev 00:01.1  bond --'
> + * | |||
> + * |   +--- io_pgtables
> + * |___|
> + *
> + * In this example, device 00:00.0 is in domain A, devices 00:01.*
> are in domain
> + * B. All devices within the same domain access the same address
> spaces. Device
> + * 00:00.0 accesses address spaces X and Y, each corresponding to an
> mm_struct.
> + * Devices 00:01.* only access address space Y. In addition each
> + * IOMMU_DOMAIN_DMA domain has a private address space, io_pgtable,
> that is
> + * managed with iommu_map()/iommu_unmap(), and isn't shared with the
> CPU MMU.
> + *
> + * To obtain the above configuration, users would for instance issue
> the
> + * following calls:
> + *
> + * iommu_sva_bind_device(dev 00:00.0, 

[PATCH v2 03/40] iommu/sva: Manage process address spaces

[Jean-Philippe Brucker]

Allocate IOMMU mm structures and binding them to devices. Four operations
are added to IOMMU drivers:

* mm_alloc(): to create an io_mm structure and perform architecture-
  specific operations required to grab the process (for instance on ARM,
  pin down the CPU ASID so that the process doesn't get assigned a new
  ASID on rollover).

  There is a single valid io_mm structure per Linux mm. Future extensions
  may also use io_mm for kernel-managed address spaces, populated with
  map()/unmap() calls instead of bound to process address spaces. This
  patch focuses on "shared" io_mm.

* mm_attach(): attach an mm to a device. The IOMMU driver checks that the
  device is capable of sharing an address space, and writes the PASID
  table entry to install the pgd.

  Some IOMMU drivers will have a single PASID table per domain, for
  convenience. Other can implement it differently but to help these
  drivers, mm_attach and mm_detach take 'attach_domain' and
  'detach_domain' parameters, that tell whether they need to set and clear
  the PASID entry or only send the required TLB invalidations.

* mm_detach(): detach an mm from a device. The IOMMU driver removes the
  PASID table entry and invalidates the IOTLBs.

* mm_free(): free a structure allocated by mm_alloc(), and let arch
  release the process.

mm_attach and mm_detach operations are serialized with a spinlock. When
trying to optimize this code, we should at least prevent concurrent
attach()/detach() on the same domain (so multi-level PASID table code can
allocate tables lazily). mm_alloc() can sleep, but mm_free must not
(because we'll have to call it from call_srcu later on).

At the moment we use an IDR for allocating PASIDs and retrieving contexts.
We also use a single spinlock. These can be refined and optimized later (a
custom allocator will be needed for top-down PASID allocation).

Keeping track of address spaces requires the use of MMU notifiers.
Handling process exit with regard to unbind() is tricky, so it is left for
another patch and we explicitly fail mm_alloc() for the moment.

Signed-off-by: Jean-Philippe Brucker 

---
v1->v2: sanity-check of flags
---
 drivers/iommu/iommu-sva.c | 380 +-
 drivers/iommu/iommu.c |   1 +
 include/linux/iommu.h |  28 +++
 3 files changed, 406 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 8d98f9c09864..6ac679c48f3c 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -5,8 +5,298 @@
  * Copyright (C) 2018 ARM Ltd.
  */
 
+#include 
 #include 
+#include 
 #include 
+#include 
+
+/**
+ * DOC: io_mm model
+ *
+ * The io_mm keeps track of process address spaces shared between CPU and 
IOMMU.
+ * The following example illustrates the relation between structures
+ * iommu_domain, io_mm and iommu_bond. An iommu_bond is a link between io_mm 
and
+ * device. A device can have multiple io_mm and an io_mm may be bound to
+ * multiple devices.
+ *  ___
+ * |  IOMMU domain A   |
+ * |   |
+ * | |  IOMMU group   |+--- io_pgtables
+ * | |||
+ * | |   dev 00:00.0 +--- bond --- io_mm X
+ * | ||   \|
+ * |   '- bond ---.
+ * |___|   \
+ *  ___ \
+ * |  IOMMU domain B   |   io_mm Y
+ * |   |   / /
+ * | |  IOMMU group   ||  / /
+ * | ||| / /
+ * | |   dev 00:01.0  bond -' /
+ * | |   dev 00:01.1  bond --'
+ * | |||
+ * |   +--- io_pgtables
+ * |___|
+ *
+ * In this example, device 00:00.0 is in domain A, devices 00:01.* are in 
domain
+ * B. All devices within the same domain access the same address spaces. Device
+ * 00:00.0 accesses address spaces X and Y, each corresponding to an mm_struct.
+ * Devices 00:01.* only access address space Y. In addition each
+ * IOMMU_DOMAIN_DMA domain has a private address space, io_pgtable, that is
+ * managed with iommu_map()/iommu_unmap(), and isn't shared with the CPU MMU.
+ *
+ * To obtain the above configuration, users would for instance issue the
+ * following calls:
+ *
+ * iommu_sva_bind_device(dev 00:00.0, mm X, ...) -> PASID 1
+ * iommu_sva_bind_device(dev 00:00.0, mm Y, ...) -> PASID 2
+ * iommu_sva_bind_device(dev 00:01.0, mm Y, ...) -> PASID 2
+ * iommu_sva_bind_device(dev 00:01.1, mm Y, ...) -> PASID 2
+ *
+ * A single Process Address Space ID (PASID) is allocated for each mm. In the
+ *