Re: [PATCH v10 12/13] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()
On Wed, Nov 25, 2020 at 10:27:49AM +0100, Jean-Philippe Brucker wrote:
> > I'm strongly
> > trying to discourage static lists matching mm's like smmu_mn is
> > doing. This is handled by the core code, don't open code it..
>
> We discussed this at v6, which wonkily stored the mn ops in the domain to
> obtain a unique notifier per {mm, domain}. A clean solution requires
> changing mm_notifier_get() to use an opaque token. Rather than testing
> {mm, ops} uniqueness it would compare {mm, ops, token}. I figured it
> wasn't worth the effort for a single driver, especially since the SMMU
> driver would still have one list matching because it needs to deal with
> both {mm, domain} and {mm, device} uniqueness.
> https://lore.kernel.org/linux-iommu/20200501121552.ga6...@infradead.org/
Oh, that was a long time ago.. OK
Thanks,
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 12/13] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()
On Tue, Nov 24, 2020 at 07:58:00PM -0400, Jason Gunthorpe wrote:
> On Fri, Sep 18, 2020 at 12:18:52PM +0200, Jean-Philippe Brucker wrote:
>
> > +/* Allocate or get existing MMU notifier for this {domain, mm} pair */
> > +static struct arm_smmu_mmu_notifier *
> > +arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
> > + struct mm_struct *mm)
> > +{
> > + int ret;
> > + struct arm_smmu_ctx_desc *cd;
> > + struct arm_smmu_mmu_notifier *smmu_mn;
> > +
> > + list_for_each_entry(smmu_mn, _domain->mmu_notifiers, list) {
> > + if (smmu_mn->mn.mm == mm) {
> > + refcount_inc(_mn->refs);
> > + return smmu_mn;
> > + }
> > + }
> > +
> > + cd = arm_smmu_alloc_shared_cd(mm);
> > + if (IS_ERR(cd))
> > + return ERR_CAST(cd);
> > +
> > + smmu_mn = kzalloc(sizeof(*smmu_mn), GFP_KERNEL);
> > + if (!smmu_mn) {
> > + ret = -ENOMEM;
> > + goto err_free_cd;
> > + }
> > +
> > + refcount_set(_mn->refs, 1);
> > + smmu_mn->cd = cd;
> > + smmu_mn->domain = smmu_domain;
> > + smmu_mn->mn.ops = _smmu_mmu_notifier_ops;
> > +
> > + ret = mmu_notifier_register(_mn->mn, mm);
> > + if (ret) {
> > + kfree(smmu_mn);
> > + goto err_free_cd;
> > + }
>
> I suppose this hasn't been applied yet, but someone asked me to look
> at this series..
It's queued for v5.11, but I could submit improvements for 5.12
> Why did you drop the change to mmu_notifier_get here?
Dropped at v6 when I got rid of the io_mm cruft:
https://lore.kernel.org/linux-iommu/20200430143424.2787566-1-jean-phili...@linaro.org/
> I'm strongly
> trying to discourage static lists matching mm's like smmu_mn is
> doing. This is handled by the core code, don't open code it..
We discussed this at v6, which wonkily stored the mn ops in the domain to
obtain a unique notifier per {mm, domain}. A clean solution requires
changing mm_notifier_get() to use an opaque token. Rather than testing
{mm, ops} uniqueness it would compare {mm, ops, token}. I figured it
wasn't worth the effort for a single driver, especially since the SMMU
driver would still have one list matching because it needs to deal with
both {mm, domain} and {mm, device} uniqueness.
https://lore.kernel.org/linux-iommu/20200501121552.ga6...@infradead.org/
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 12/13] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()
On Fri, Sep 18, 2020 at 12:18:52PM +0200, Jean-Philippe Brucker wrote:
> +/* Allocate or get existing MMU notifier for this {domain, mm} pair */
> +static struct arm_smmu_mmu_notifier *
> +arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
> + struct mm_struct *mm)
> +{
> + int ret;
> + struct arm_smmu_ctx_desc *cd;
> + struct arm_smmu_mmu_notifier *smmu_mn;
> +
> + list_for_each_entry(smmu_mn, _domain->mmu_notifiers, list) {
> + if (smmu_mn->mn.mm == mm) {
> + refcount_inc(_mn->refs);
> + return smmu_mn;
> + }
> + }
> +
> + cd = arm_smmu_alloc_shared_cd(mm);
> + if (IS_ERR(cd))
> + return ERR_CAST(cd);
> +
> + smmu_mn = kzalloc(sizeof(*smmu_mn), GFP_KERNEL);
> + if (!smmu_mn) {
> + ret = -ENOMEM;
> + goto err_free_cd;
> + }
> +
> + refcount_set(_mn->refs, 1);
> + smmu_mn->cd = cd;
> + smmu_mn->domain = smmu_domain;
> + smmu_mn->mn.ops = _smmu_mmu_notifier_ops;
> +
> + ret = mmu_notifier_register(_mn->mn, mm);
> + if (ret) {
> + kfree(smmu_mn);
> + goto err_free_cd;
> + }
I suppose this hasn't been applied yet, but someone asked me to look
at this series..
Why did you drop the change to mmu_notifier_get here? I'm strongly
trying to discourage static lists matching mm's like smmu_mn is
doing. This is handled by the core code, don't open code it..
Thanks,
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v10 12/13] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()
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 from this point onwards.
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
---
drivers/iommu/Kconfig | 2 +
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 28 +++
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 230 +-
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 32 ++-
4 files changed, 282 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index b1d592cd9984..a8eb8b7f35f7 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -316,6 +316,8 @@ config ARM_SMMU_V3
config ARM_SMMU_V3_SVA
bool "Shared Virtual Addressing support for the ARM SMMUv3"
depends on ARM_SMMU_V3
+ select IOMMU_SVA_LIB
+ select MMU_NOTIFIER
help
Support for sharing process address spaces with devices using the
SMMUv3.
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index ba34914813ff..6365c81a4614 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -677,10 +677,18 @@ struct arm_smmu_domain {
struct list_headdevices;
spinlock_t devices_lock;
+
+ struct list_headmmu_notifiers;
};
+static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
+{
+ return container_of(dom, struct arm_smmu_domain, domain);
+}
+
extern struct xarray arm_smmu_asid_xa;
extern struct mutex arm_smmu_asid_lock;
+extern struct arm_smmu_ctx_desc quiet_cd;
int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
struct arm_smmu_ctx_desc *cd);
@@ -693,6 +701,11 @@ bool arm_smmu_master_sva_supported(struct arm_smmu_master
*master);
bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master);
int arm_smmu_master_enable_sva(struct arm_smmu_master *master);
int arm_smmu_master_disable_sva(struct arm_smmu_master *master);
+struct iommu_sva *arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm,
+ void *drvdata);
+void arm_smmu_sva_unbind(struct iommu_sva *handle);
+int arm_smmu_sva_get_pasid(struct iommu_sva *handle);
+void arm_smmu_sva_notifier_synchronize(void);
#else /* CONFIG_ARM_SMMU_V3_SVA */
static inline bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
{
@@ -718,5 +731,20 @@ static inline int arm_smmu_master_disable_sva(struct
arm_smmu_master *master)
{
return -ENODEV;
}
+
+static inline struct iommu_sva *
+arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm, void *drvdata)
+{
+ return ERR_PTR(-ENODEV);
+}
+
+static inline void arm_smmu_sva_unbind(struct iommu_sva *handle) {}
+
+static inline int arm_smmu_sva_get_pasid(struct iommu_sva *handle)
+{
+ return IOMMU_PASID_INVALID;
+}
+
+static inline void arm_smmu_sva_notifier_synchronize(void) {}
#endif /* CONFIG_ARM_SMMU_V3_SVA */
#endif /* _ARM_SMMU_V3_H */
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 9255c9600fb8..f57c2bde68a4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -5,11 +5,35 @@
#include
#include
+#include
#include
#include "arm-smmu-v3.h"
+#include "../../iommu-sva-lib.h"
#include "../../io-pgtable-arm.h"
+struct arm_smmu_mmu_notifier {
+ struct mmu_notifier mn;
+ struct arm_smmu_ctx_desc
