Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-07 Thread Jacob Pan
On Tue, 5 May 2020 11:15:31 +0200
Jean-Philippe Brucker  wrote:

> On Mon, May 04, 2020 at 01:47:23PM -0700, Jacob Pan wrote:
> > > > > + arm_smmu_write_ctx_desc(smmu_domain, mm->pasid,
> > > > > _cd); +
> > > > > + arm_smmu_tlb_inv_asid(smmu_domain->smmu,
> > > > > smmu_mn->cd->asid);
> > > > > + /* TODO: invalidate ATS */
> > > > > +
> > > > If mm release is called after tlb invalidate range, is it still
> > > > necessary to invalidate again?
> > > 
> > > No, provided all mappings from the address space are unmapped and
> > > invalidated. I'll double check, but in my tests invalidate range
> > > didn't seem to be called for all mappings on mm exit, so I
> > > believe we do need this.
> > >   
> > I think it is safe to invalidate again. There was a concern that mm
> > release may delete IOMMU driver from the notification list and miss
> > tlb invalidate range. I had a hard time to confirm that with ftrace
> > while killing a process, many lost events.
> >   
> 
> If it helps, I have a test that generates small DMA transactions on a
> SMMU model. This is the trace for a job on a 8kB mmap'd buffer:
> 
>   smmu_bind_alloc: dev=:00:03.0 pasid=1
>   dev_fault: IOMMU::00:03.0 type=2 reason=0
> addr=0x860e6000 pasid=1 group=74 flags=3 prot=2
> dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=74
> dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0x860e7000
> pasid=1 group=143 flags=3 prot=2 dev_page_response:
> IOMMU::00:03.0 code=0 pasid=1 group=143 smmu_mm_invalidate:
> pasid=1 start=0x860e6000 end=0x860e8000 smmu_mm_invalidate:
> pasid=1 start=0x860e6000 end=0x860e8000 smmu_mm_invalidate:
> pasid=1 start=0x860e8000 end=0x860ea000 smmu_mm_invalidate:
> pasid=1 start=0x860e8000 end=0x860ea000 smmu_unbind_free:
> dev=:00:03.0 pasid=1
> 
> And this is the same job, but the process immediately kills itself
> after launching it.
> 
>   smmu_bind_alloc: dev=:00:03.0 pasid=1
>   dev_fault: IOMMU::00:03.0 type=2 reason=0
> addr=0xb9d15000 pasid=1 group=259 flags=3 prot=2
> smmu_mm_release: pasid=1 dev_page_response: IOMMU::00:03.0 code=0
> pasid=1 group=259 dev_fault: IOMMU::00:03.0 type=2 reason=0
> addr=0xb9d15000 pasid=1 group=383 flags=3 prot=2
> dev_page_response: IOMMU::00:03.0 code=1 pasid=1 group=383
> smmu_unbind_free: dev=:00:03.0 pasid=1
> 
> We don't get any invalidate_range notification in this case.
> 
Thanks for the confirmation. We do need to invalidate here.

> Thanks,
> Jean

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-05 Thread Jean-Philippe Brucker
On Mon, May 04, 2020 at 01:47:23PM -0700, Jacob Pan wrote:
> > > > +   arm_smmu_write_ctx_desc(smmu_domain, mm->pasid,
> > > > _cd); +
> > > > +   arm_smmu_tlb_inv_asid(smmu_domain->smmu,
> > > > smmu_mn->cd->asid);
> > > > +   /* TODO: invalidate ATS */
> > > > +  
> > > If mm release is called after tlb invalidate range, is it still
> > > necessary to invalidate again?  
> > 
> > No, provided all mappings from the address space are unmapped and
> > invalidated. I'll double check, but in my tests invalidate range
> > didn't seem to be called for all mappings on mm exit, so I believe we
> > do need this.
> > 
> I think it is safe to invalidate again. There was a concern that mm
> release may delete IOMMU driver from the notification list and miss tlb
> invalidate range. I had a hard time to confirm that with ftrace while
> killing a process, many lost events.
> 

If it helps, I have a test that generates small DMA transactions on a SMMU
model. This is the trace for a job on a 8kB mmap'd buffer:

  smmu_bind_alloc: dev=:00:03.0 pasid=1
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0x860e6000 pasid=1 
group=74 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=74
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0x860e7000 pasid=1 
group=143 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=143
  smmu_mm_invalidate: pasid=1 start=0x860e6000 end=0x860e8000
  smmu_mm_invalidate: pasid=1 start=0x860e6000 end=0x860e8000
  smmu_mm_invalidate: pasid=1 start=0x860e8000 end=0x860ea000
  smmu_mm_invalidate: pasid=1 start=0x860e8000 end=0x860ea000
  smmu_unbind_free: dev=:00:03.0 pasid=1

And this is the same job, but the process immediately kills itself after
launching it.

  smmu_bind_alloc: dev=:00:03.0 pasid=1
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0xb9d15000 pasid=1 
group=259 flags=3 prot=2
  smmu_mm_release: pasid=1
  dev_page_response: IOMMU::00:03.0 code=0 pasid=1 group=259
  dev_fault: IOMMU::00:03.0 type=2 reason=0 addr=0xb9d15000 pasid=1 
group=383 flags=3 prot=2
  dev_page_response: IOMMU::00:03.0 code=1 pasid=1 group=383
  smmu_unbind_free: dev=:00:03.0 pasid=1

We don't get any invalidate_range notification in this case.

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-04 Thread Jacob Pan
On Mon, 4 May 2020 18:43:51 +0200
Jean-Philippe Brucker  wrote:

> On Thu, Apr 30, 2020 at 02:16:17PM -0700, Jacob Pan wrote:
> > > +static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn,
> > > +  struct mm_struct *mm,
> > > +  unsigned long start,
> > > unsigned long end) +{
> > > + /* TODO: invalidate ATS */
> > > +}
> > > +
> > > +static void arm_smmu_mm_release(struct mmu_notifier *mn, struct
> > > mm_struct *mm) +{
> > > + struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn);
> > > + struct arm_smmu_domain *smmu_domain;
> > > +
> > > + mutex_lock(_smmu_sva_lock);
> > > + if (smmu_mn->cleared) {
> > > + mutex_unlock(_smmu_sva_lock);
> > > + return;
> > > + }
> > > +
> > > + smmu_domain = smmu_mn->domain;
> > > +
> > > + /*
> > > +  * DMA may still be running. Keep the cd valid but
> > > disable
> > > +  * translation, so that new events will still result in
> > > stall.
> > > +  */  
> > Does "disable translation" also disable translated requests?  
> 
> No it doesn't disable translated requests, it only prevents the SMMU
> from accessing the pgd.
> 
OK. same as VT-d.

> > I guess
> > release is called after tlb invalidate range, so assuming no more
> > devTLB left to generate translated request?  
> 
> I'm counting on the invalidate below (here a TODO, implemented in next
> patch) to drop all devTLB entries. After that invalidate, the device:
> * issues a Translation Request, returns with R=W=0 because we disabled
>   translation (and it isn't present in the SMMU TLB).
> * issues a Page Request, returns with InvalidRequest because
>   mmget_not_zero() fails.
> 
Same flow. Thanks for the explanation.

> >   
> > > + arm_smmu_write_ctx_desc(smmu_domain, mm->pasid,
> > > _cd); +
> > > + arm_smmu_tlb_inv_asid(smmu_domain->smmu,
> > > smmu_mn->cd->asid);
> > > + /* TODO: invalidate ATS */
> > > +  
> > If mm release is called after tlb invalidate range, is it still
> > necessary to invalidate again?  
> 
> No, provided all mappings from the address space are unmapped and
> invalidated. I'll double check, but in my tests invalidate range
> didn't seem to be called for all mappings on mm exit, so I believe we
> do need this.
> 
I think it is safe to invalidate again. There was a concern that mm
release may delete IOMMU driver from the notification list and miss tlb
invalidate range. I had a hard time to confirm that with ftrace while
killing a process, many lost events.


> Thanks,
> Jean
> 

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-04 Thread Jean-Philippe Brucker
On Thu, Apr 30, 2020 at 02:16:17PM -0700, Jacob Pan wrote:
> > +static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn,
> > +struct mm_struct *mm,
> > +unsigned long start,
> > unsigned long end) +{
> > +   /* TODO: invalidate ATS */
> > +}
> > +
> > +static void arm_smmu_mm_release(struct mmu_notifier *mn, struct
> > mm_struct *mm) +{
> > +   struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn);
> > +   struct arm_smmu_domain *smmu_domain;
> > +
> > +   mutex_lock(_smmu_sva_lock);
> > +   if (smmu_mn->cleared) {
> > +   mutex_unlock(_smmu_sva_lock);
> > +   return;
> > +   }
> > +
> > +   smmu_domain = smmu_mn->domain;
> > +
> > +   /*
> > +* DMA may still be running. Keep the cd valid but disable
> > +* translation, so that new events will still result in
> > stall.
> > +*/
> Does "disable translation" also disable translated requests?

No it doesn't disable translated requests, it only prevents the SMMU from
accessing the pgd.

> I guess
> release is called after tlb invalidate range, so assuming no more
> devTLB left to generate translated request?

I'm counting on the invalidate below (here a TODO, implemented in next
patch) to drop all devTLB entries. After that invalidate, the device:
* issues a Translation Request, returns with R=W=0 because we disabled
  translation (and it isn't present in the SMMU TLB).
* issues a Page Request, returns with InvalidRequest because
  mmget_not_zero() fails.

> 
> > +   arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, _cd);
> > +
> > +   arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_mn->cd->asid);
> > +   /* TODO: invalidate ATS */
> > +
> If mm release is called after tlb invalidate range, is it still
> necessary to invalidate again?

No, provided all mappings from the address space are unmapped and
invalidated. I'll double check, but in my tests invalidate range didn't
seem to be called for all mappings on mm exit, so I believe we do need
this.

Thanks,
Jean

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-04 Thread Jean-Philippe Brucker
On Fri, May 01, 2020 at 09:55:13AM -0300, Jason Gunthorpe wrote:
> On Fri, May 01, 2020 at 05:15:52AM -0700, Christoph Hellwig wrote:
> > > @@ -432,6 +432,7 @@ config ARM_SMMU_V3
> > >   tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
> > >   depends on ARM64
> > >   select IOMMU_API
> > > + select IOMMU_SVA
> > >   select IOMMU_IO_PGTABLE_LPAE
> > >   select GENERIC_MSI_IRQ_DOMAIN
> > 
> > Doesn't this need to select MMU_NOTIFIER now?
> > 
> > > + struct mmu_notifier_ops mn_ops;
> > 
> > Note: not a pointer.
> > 
> > > + /* If bind() was already called for this (dev, mm) pair, reuse it. */
> > > + list_for_each_entry(bond, >bonds, list) {
> > > + if (bond->mm == mm) {
> > > + refcount_inc(>refs);
> > > + return >sva;
> > > + }
> > > + }
> 
> I also would like it if searching for mms in linked lists was not
> necessary, this is kind of the point of 'get'
> 
> Is this a side effect of the earlier remark to get rid of the linked
> list inside the notifier?
> 
> > Or we could enhance the mmu_notifier_get to pass a private
> > oaque instance ID pointer, which is checked in addition to the ops,
> > and you could probably kill off the bonds list and lookup.
> 
> This might be the best option if it can absorb the above search..

It wouldn't, the above search is separate. I currently register the MMU
notifier on (IOMMU domain, mm). The (dev, mm) search above is to follow
the iommu_sva_bind_device() API doc, that states we should return the same
handle for a given (dev, mm) pair.

Thanks,
Jean

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-04 Thread Jean-Philippe Brucker
On Fri, May 01, 2020 at 05:15:52AM -0700, Christoph Hellwig wrote:
> > @@ -432,6 +432,7 @@ config ARM_SMMU_V3
> > tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
> > depends on ARM64
> > select IOMMU_API
> > +   select IOMMU_SVA
> > select IOMMU_IO_PGTABLE_LPAE
> > select GENERIC_MSI_IRQ_DOMAIN
> 
> Doesn't this need to select MMU_NOTIFIER now?

Yes, will fix

> > +   struct mmu_notifier_ops mn_ops;
> 
> Note: not a pointer.
> 
> > +   /* If bind() was already called for this (dev, mm) pair, reuse it. */
> > +   list_for_each_entry(bond, >bonds, list) {
> > +   if (bond->mm == mm) {
> > +   refcount_inc(>refs);
> > +   return >sva;
> > +   }
> > +   }
> > +
> > +   mn = mmu_notifier_get(_domain->mn_ops, mm);
> > +   if (IS_ERR(mn))
> > +   return ERR_CAST(mn);
> 
> Which seems to be to avoid mmu_notifier_get reusing notifiers registered
> by other arm_smmu_master instance right?

Yes, although I'm registering a single mmu notifier per (domain, mm) pair,
not (master, mm), because the SMMU driver keeps one set of PASID tables
per IOMMU domain.

> Either you could just use plain old mmu_notifier_register to avoid
> the reuse.  Or we could enhance the mmu_notifier_get to pass a private
> oaque instance ID pointer, which is checked in addition to the ops,
> and you could probably kill off the bonds list and lookup.

Going back to mmu_notifier_register() seems better for now. I don't want
to change the core APIs just for this driver, because it's likely to
change again when more hardware starts appearing and we optimize it.

Thanks,
Jean

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-01 Thread Jason Gunthorpe
On Fri, May 01, 2020 at 05:15:52AM -0700, Christoph Hellwig wrote:
> > @@ -432,6 +432,7 @@ config ARM_SMMU_V3
> > tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
> > depends on ARM64
> > select IOMMU_API
> > +   select IOMMU_SVA
> > select IOMMU_IO_PGTABLE_LPAE
> > select GENERIC_MSI_IRQ_DOMAIN
> 
> Doesn't this need to select MMU_NOTIFIER now?
> 
> > +   struct mmu_notifier_ops mn_ops;
> 
> Note: not a pointer.
> 
> > +   /* If bind() was already called for this (dev, mm) pair, reuse it. */
> > +   list_for_each_entry(bond, >bonds, list) {
> > +   if (bond->mm == mm) {
> > +   refcount_inc(>refs);
> > +   return >sva;
> > +   }
> > +   }

I also would like it if searching for mms in linked lists was not
necessary, this is kind of the point of 'get'

Is this a side effect of the earlier remark to get rid of the linked
list inside the notifier?

> Or we could enhance the mmu_notifier_get to pass a private
> oaque instance ID pointer, which is checked in addition to the ops,
> and you could probably kill off the bonds list and lookup.

This might be the best option if it can absorb the above search..

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-05-01 Thread Christoph Hellwig
> @@ -432,6 +432,7 @@ config ARM_SMMU_V3
>   tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>   depends on ARM64
>   select IOMMU_API
> + select IOMMU_SVA
>   select IOMMU_IO_PGTABLE_LPAE
>   select GENERIC_MSI_IRQ_DOMAIN

Doesn't this need to select MMU_NOTIFIER now?

> + struct mmu_notifier_ops mn_ops;

Note: not a pointer.

> + /* If bind() was already called for this (dev, mm) pair, reuse it. */
> + list_for_each_entry(bond, >bonds, list) {
> + if (bond->mm == mm) {
> + refcount_inc(>refs);
> + return >sva;
> + }
> + }
> +
> + mn = mmu_notifier_get(_domain->mn_ops, mm);
> + if (IS_ERR(mn))
> + return ERR_CAST(mn);

Which seems to be to avoid mmu_notifier_get reusing notifiers registered
by other arm_smmu_master instance right?

Either you could just use plain old mmu_notifier_register to avoid
the reuse.  Or we could enhance the mmu_notifier_get to pass a private
oaque instance ID pointer, which is checked in addition to the ops,
and you could probably kill off the bonds list and lookup.

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


Re: [PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-04-30 Thread Jacob Pan
Hi Jean,

A couple question on how SMMU handles CD.v and translation disable.

On Thu, 30 Apr 2020 16:34:16 +0200
Jean-Philippe Brucker  wrote:

> The sva_bind() function allows devices to access process address
> spaces using a PASID (aka SSID).
> 
> (1) bind() allocates or gets an existing MMU notifier tied to the
> (domain, mm) pair. Each mm gets one PASID.
> 
> (2) Any change to the address space calls invalidate_range() which
> sends ATC invalidations (in a subsequent patch).
> 
> (3) When the process address space dies, the release() notifier
> disables the CD to allow reclaiming the page tables. Since release()
> has to be light we do not instruct device drivers to stop DMA here,
> we just ignore incoming page faults.
> 
> To avoid any event 0x0a print (C_BAD_CD) we disable translation
> without clearing CD.V. PCIe Translation Requests and Page Requests
> are silently denied. Don't clear the R bit because the S bit can't
> be cleared when STALL_MODEL==0b10 (forced), and clearing R without
> clearing S is useless. Faulting transactions will stall and will
> be aborted by the IOPF handler.
> 
> (4) After stopping DMA, the device driver releases the bond by calling
> unbind(). We release the MMU notifier, free the PASID and the
> bond.
> 
> Three structures keep track of bonds:
> * arm_smmu_bond: one per (device, mm) pair, the handle returned to the
>   device driver for a bind() request.
> * arm_smmu_mmu_notifier: one per (domain, mm) pair, deals with ATS/TLB
>   invalidations and clearing the context descriptor on mm exit.
> * arm_smmu_ctx_desc: one per mm, holds the pinned ASID and pgd.
> 
> Signed-off-by: Jean-Philippe Brucker 
> ---
> v5->v6:
> * Implement bind() directly instead of going through io_mm_ops
> * Don't clear S and R bits in step (3), it doesn't work with
>   STALL_FORCE.
> ---
>  drivers/iommu/Kconfig   |   1 +
>  drivers/iommu/arm-smmu-v3.c | 256
> +++- 2 files changed, 253
> insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 1e64ee6592e16..f863c4562feeb 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -432,6 +432,7 @@ config ARM_SMMU_V3
>   tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
>   depends on ARM64
>   select IOMMU_API
> + select IOMMU_SVA
>   select IOMMU_IO_PGTABLE_LPAE
>   select GENERIC_MSI_IRQ_DOMAIN
>   help
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index c7942d0540599..00e5b69bb81a5 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -36,6 +37,7 @@
>  #include 
>  
>  #include "io-pgtable-arm.h"
> +#include "iommu-sva.h"
>  
>  /* MMIO registers */
>  #define ARM_SMMU_IDR00x0
> @@ -731,8 +733,31 @@ struct arm_smmu_domain {
>  
>   struct list_headdevices;
>   spinlock_t  devices_lock;
> +
> + struct mmu_notifier_ops mn_ops;
>  };
>  
> +struct arm_smmu_mmu_notifier {
> + struct mmu_notifier mn;
> + struct arm_smmu_ctx_desc*cd;
> + boolcleared;
> + refcount_t  refs;
> + struct arm_smmu_domain  *domain;
> +};
> +
> +#define mn_to_smmu(mn) container_of(mn, struct
> arm_smmu_mmu_notifier, mn) +
> +struct arm_smmu_bond {
> + struct iommu_svasva;
> + struct mm_struct*mm;
> + struct arm_smmu_mmu_notifier*smmu_mn;
> + struct list_headlist;
> + refcount_t  refs;
> +};
> +
> +#define sva_to_bond(handle) \
> + container_of(handle, struct arm_smmu_bond, sva)
> +
>  struct arm_smmu_option_prop {
>   u32 opt;
>   const char *prop;
> @@ -742,6 +767,13 @@ static DEFINE_XARRAY_ALLOC1(asid_xa);
>  static DEFINE_SPINLOCK(contexts_lock);
>  static DEFINE_MUTEX(arm_smmu_sva_lock);
>  
> +/*
> + * When a process dies, DMA is still running but we need to clear
> the pgd. If we
> + * simply cleared the valid bit from the context descriptor, we'd
> get event 0x0a
> + * which are not recoverable.
> + */
> +static struct arm_smmu_ctx_desc invalid_cd = { 0 };
> +
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>   { ARM_SMMU_OPT_SKIP_PREFETCH,
> "hisilicon,broken-prefetch-cmd" }, { ARM_SMMU_OPT_PAGE0_REGS_ONLY,
> "cavium,cn9900-broken-page1-regspace"}, @@ -1652,7 +1684,9 @@ static
> int __arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain,
>* (2) Install a secondary CD, for SID+SSID traffic.
>* (3) Update ASID of a CD. Atomically write the first 64
> bits of the
>* CD, then invalidate the old entry and mappings.
> -  * (4) Remove a secondary CD.
> +  * (4) Quiesce the context without clearing the valid bit.
> 

[PATCH v6 17/25] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()

2020-04-30 Thread Jean-Philippe Brucker
The sva_bind() function allows devices to access process address spaces
using a PASID (aka SSID).

(1) bind() allocates or gets an existing MMU notifier tied to the
(domain, mm) pair. Each mm gets one PASID.

(2) Any change to the address space calls invalidate_range() which sends
ATC invalidations (in a subsequent patch).

(3) When the process address space dies, the release() notifier disables
the CD to allow reclaiming the page tables. Since release() has to
be light we do not instruct device drivers to stop DMA here, we just
ignore incoming page faults.

To avoid any event 0x0a print (C_BAD_CD) we disable translation
without clearing CD.V. PCIe Translation Requests and Page Requests
are silently denied. Don't clear the R bit because the S bit can't
be cleared when STALL_MODEL==0b10 (forced), and clearing R without
clearing S is useless. Faulting transactions will stall and will be
aborted by the IOPF handler.

(4) After stopping DMA, the device driver releases the bond by calling
unbind(). We release the MMU notifier, free the PASID and the bond.

Three structures keep track of bonds:
* arm_smmu_bond: one per (device, mm) pair, the handle returned to the
  device driver for a bind() request.
* arm_smmu_mmu_notifier: one per (domain, mm) pair, deals with ATS/TLB
  invalidations and clearing the context descriptor on mm exit.
* arm_smmu_ctx_desc: one per mm, holds the pinned ASID and pgd.

Signed-off-by: Jean-Philippe Brucker 
---
v5->v6:
* Implement bind() directly instead of going through io_mm_ops
* Don't clear S and R bits in step (3), it doesn't work with
  STALL_FORCE.
---
 drivers/iommu/Kconfig   |   1 +
 drivers/iommu/arm-smmu-v3.c | 256 +++-
 2 files changed, 253 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1e64ee6592e16..f863c4562feeb 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -432,6 +432,7 @@ config ARM_SMMU_V3
tristate "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
depends on ARM64
select IOMMU_API
+   select IOMMU_SVA
select IOMMU_IO_PGTABLE_LPAE
select GENERIC_MSI_IRQ_DOMAIN
help
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c7942d0540599..00e5b69bb81a5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -36,6 +37,7 @@
 #include 
 
 #include "io-pgtable-arm.h"
+#include "iommu-sva.h"
 
 /* MMIO registers */
 #define ARM_SMMU_IDR0  0x0
@@ -731,8 +733,31 @@ struct arm_smmu_domain {
 
struct list_headdevices;
spinlock_t  devices_lock;
+
+   struct mmu_notifier_ops mn_ops;
 };
 
+struct arm_smmu_mmu_notifier {
+   struct mmu_notifier mn;
+   struct arm_smmu_ctx_desc*cd;
+   boolcleared;
+   refcount_t  refs;
+   struct arm_smmu_domain  *domain;
+};
+
+#define mn_to_smmu(mn) container_of(mn, struct arm_smmu_mmu_notifier, mn)
+
+struct arm_smmu_bond {
+   struct iommu_svasva;
+   struct mm_struct*mm;
+   struct arm_smmu_mmu_notifier*smmu_mn;
+   struct list_headlist;
+   refcount_t  refs;
+};
+
+#define sva_to_bond(handle) \
+   container_of(handle, struct arm_smmu_bond, sva)
+
 struct arm_smmu_option_prop {
u32 opt;
const char *prop;
@@ -742,6 +767,13 @@ static DEFINE_XARRAY_ALLOC1(asid_xa);
 static DEFINE_SPINLOCK(contexts_lock);
 static DEFINE_MUTEX(arm_smmu_sva_lock);
 
+/*
+ * When a process dies, DMA is still running but we need to clear the pgd. If 
we
+ * simply cleared the valid bit from the context descriptor, we'd get event 
0x0a
+ * which are not recoverable.
+ */
+static struct arm_smmu_ctx_desc invalid_cd = { 0 };
+
 static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
@@ -1652,7 +1684,9 @@ static int __arm_smmu_write_ctx_desc(struct 
arm_smmu_domain *smmu_domain,
 * (2) Install a secondary CD, for SID+SSID traffic.
 * (3) Update ASID of a CD. Atomically write the first 64 bits of the
 * CD, then invalidate the old entry and mappings.
-* (4) Remove a secondary CD.
+* (4) Quiesce the context without clearing the valid bit. Disable
+* translation, and ignore any translation fault.
+* (5) Remove a secondary CD.
 */
u64 val;
bool cd_live;
@@ -1669,8 +1703,10 @@ static int __arm_smmu_write_ctx_desc(struct 
arm_smmu_domain *smmu_domain,
val = le64_to_cpu(cdptr[0]);
cd_live = !!(val &