Re: [PATCH] iommu/dma: Add config for PCI SAC address trick

2022-05-19 Thread Christoph Hellwig
On Wed, May 18, 2022 at 06:36:59PM +0100, Robin Murphy wrote:
> +config IOMMU_DMA_PCI_SAC_OPT
> + bool "Enable 64-bit legacy PCI optimisation by default"
> + depends on IOMMU_DMA
> + default X86
> + help
> +   Enable by default an IOMMU optimisation for 64-bit legacy PCI devices,
> +   wherein the DMA API layer will always first try to allocate a 32-bit
> +   DMA address suitable for a single address cycle, before falling back
> +   to allocating from the full usable address range. If your system has
> +   64-bit legacy PCI devices in 32-bit slots where using dual address
> +   cycles reduces DMA throughput significantly, this optimisation may be
> +   beneficial to overall performance.

The config option name sounds odd.  Yes, maybe for actual 64-bit PCI
this actualy is an optimization.  But I'd think of it more as a
workaround. and I'd probably word it as such.  I also would not not
default to true for x86, just allow for that.  There is nothing
fundamental about x86 wanting that, just that people use more crap
drivers on x86.  An the fact that AMD SEV sets the high bit for
encrypted memory has been weeding out at least some of them.

> +bool iommu_dma_forcedac __read_mostly = 
> !IS_ENABLED(CONFIG_IOMMU_DMA_PCI_SAC_OPT);

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


[PATCH v7 01/10] iommu: Add pasids field in struct iommu_device

2022-05-19 Thread Lu Baolu
Use this field to keep the number of supported PASIDs that an IOMMU
hardware is able to support. This is a generic attribute of an IOMMU
and lifting it into the per-IOMMU device structure makes it possible
to allocate a PASID for device without calls into the IOMMU drivers.
Any iommu driver which suports PASID related features should set this
field before enabling them on the devices.

Signed-off-by: Lu Baolu 
Reviewed-by: Jean-Philippe Brucker 
---
 include/linux/iommu.h   | 2 ++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
 drivers/iommu/intel/dmar.c  | 4 
 3 files changed, 7 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e1afe169549..da423e87f248 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -318,12 +318,14 @@ struct iommu_domain_ops {
  * @list: Used by the iommu-core to keep a list of registered iommus
  * @ops: iommu-ops for talking to this iommu
  * @dev: struct device for sysfs handling
+ * @pasids: number of supported PASIDs
  */
 struct iommu_device {
struct list_head list;
const struct iommu_ops *ops;
struct fwnode_handle *fwnode;
struct device *dev;
+   u32 pasids;
 };
 
 /**
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 88817a3376ef..6e2cd082c670 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3546,6 +3546,7 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
/* SID/SSID sizes */
smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);
smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
+   smmu->iommu.pasids = smmu->ssid_bits;
 
/*
 * If the SMMU supports fewer bits than would fill a single L2 stream
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 4de960834a1b..1c3cf267934d 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1126,6 +1126,10 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
 
raw_spin_lock_init(&iommu->register_lock);
 
+   /* Supports full 20-bit PASID in scalable mode. */
+   if (ecap_pasid(iommu->ecap))
+   iommu->iommu.pasids = 1UL << 20;
+
/*
 * This is only for hotplug; at boot time intel_iommu_enabled won't
 * be set yet. When intel_iommu_init() runs, it registers the units
-- 
2.25.1

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


[PATCH v7 00/10] iommu: SVA and IOPF refactoring

2022-05-19 Thread Lu Baolu
Hi folks,

The former part of this series refactors the IOMMU SVA code by assigning
an SVA type of iommu_domain to a shared virtual address and replacing
sva_bind/unbind iommu ops with attach/detach_dev_pasid domain ops.

The latter part changes the existing I/O page fault handling framework
from only serving SVA to a generic one. Any driver or component could
handle the I/O page faults for its domain in its own way by installing
an I/O page fault handler.

This series has been functionally tested on an x86 machine and compile
tested for other architectures.

This series is also available on github:
[2] https://github.com/LuBaolu/intel-iommu/commits/iommu-sva-refactoring-v7

Please review and suggest.

Best regards,
baolu

Change log:
v7:
 - Remove duplicate array for sva domain.
 - Rename detach_dev_pasid to block_dev_pasid.
 - Add raw device driver interfaces for iommufd.
 - Other misc refinements and patch reorganization.
 - Drop "dmaengine: idxd: Separate user and kernel pasid enabling" which
   has been picked for dmaengine tree.

v6:
 - 
https://lore.kernel.org/linux-iommu/20220510061738.2761430-1-baolu...@linux.intel.com/
 - Refine the SVA basic data structures.
   Link: https://lore.kernel.org/linux-iommu/YnFv0ps0Ad8v+7uH@myrica/
 - Refine arm smmuv3 sva domain allocation.
 - Fix a possible lock issue.
   Link: https://lore.kernel.org/linux-iommu/YnFydE8j8l7Q4m+b@myrica/

v5:
 - 
https://lore.kernel.org/linux-iommu/20220502014842.991097-1-baolu...@linux.intel.com/
 - Address review comments from Jean-Philippe Brucker. Very appreciated!
 - Remove redundant pci aliases check in
   device_group_immutable_singleton().
 - Treat all buses exept PCI as static in immutable singleton check.
 - As the sva_bind/unbind() have already guaranteed sva domain free only
   after iopf_queue_flush_dev(), remove the unnecessary domain refcount.
 - Move domain get() out of the list iteration in iopf_handle_group().

v4:
 - 
https://lore.kernel.org/linux-iommu/20220421052121.3464100-1-baolu...@linux.intel.com/
 - Solve the overlap with another series and make this series
   self-contained.
 - No objection to the abstraction of data structure during v3 review.
   Hence remove the RFC subject prefix.
 - Refine the immutable singleton group code according to Kevin's
   comments.

v3:
 - 
https://lore.kernel.org/linux-iommu/20220410102443.294128-1-baolu...@linux.intel.com/
 - Rework iommu_group_singleton_lockdown() by adding a flag to the group
   that positively indicates the group can never have more than one
   member, even after hot plug.
 - Abstract the data structs used for iommu sva in a separated patches to
   make it easier for review.
 - I still keep the RFC prefix in this series as above two significant
   changes need at least another round review to be finalized.
 - Several misc refinements.

v2:
 - 
https://lore.kernel.org/linux-iommu/20220329053800.3049561-1-baolu...@linux.intel.com/
 - Add sva domain life cycle management to avoid race between unbind and
   page fault handling.
 - Use a single domain for each mm.
 - Return a single sva handler for the same binding.
 - Add a new helper to meet singleton group requirement.
 - Rework the SVA domain allocation for arm smmu v3 driver and move the
   pasid_bit initialization to device probe.
 - Drop the patch "iommu: Handle IO page faults directly".
 - Add mmget_not_zero(mm) in SVA page fault handler.

v1:
 - 
https://lore.kernel.org/linux-iommu/20220320064030.2936936-1-baolu...@linux.intel.com/
 - Initial post.

Lu Baolu (10):
  iommu: Add pasids field in struct iommu_device
  iommu: Remove SVM_FLAG_SUPERVISOR_MODE support
  iommu/sva: Add iommu_sva_domain support
  iommu/vt-d: Add SVA domain support
  arm-smmu-v3/sva: Add SVA domain support
  iommu/sva: Refactoring iommu_sva_bind/unbind_device()
  iommu: Remove SVA related callbacks from iommu ops
  iommu: Prepare IOMMU domain for IOPF
  iommu: Per-domain I/O page fault handling
  iommu: Rename iommu-sva-lib.{c,h}

 include/linux/intel-iommu.h   |   8 +-
 include/linux/intel-svm.h |  13 -
 include/linux/iommu.h | 113 +--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  21 +-
 .../iommu/{iommu-sva-lib.h => iommu-sva.h}|  16 +
 drivers/dma/idxd/cdev.c   |   2 +-
 drivers/dma/idxd/init.c   |  24 +-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  89 +++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  12 +-
 drivers/iommu/intel/dmar.c|   4 +
 drivers/iommu/intel/iommu.c   |   9 +-
 drivers/iommu/intel/svm.c | 135 +++-
 drivers/iommu/io-pgfault.c|  73 +
 drivers/iommu/iommu-sva-lib.c |  71 -
 drivers/iommu/iommu-sva.c | 297 ++
 drivers/iommu/iommu.c | 191 +--
 drivers/misc/uacce/uacce.c|   2 +-
 drivers/iommu/Makefi

[PATCH v7 02/10] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support

2022-05-19 Thread Lu Baolu
The current kernel DMA with PASID support is based on the SVA with a flag
SVM_FLAG_SUPERVISOR_MODE. The IOMMU driver binds the kernel memory address
space to a PASID of the device. The device driver programs the device with
kernel virtual address (KVA) for DMA access. There have been security and
functional issues with this approach:

- The lack of IOTLB synchronization upon kernel page table updates.
  (vmalloc, module/BPF loading, CONFIG_DEBUG_PAGEALLOC etc.)
- Other than slight more protection, using kernel virtual address (KVA)
  has little advantage over physical address. There are also no use
  cases yet where DMA engines need kernel virtual addresses for in-kernel
  DMA.

This removes SVM_FLAG_SUPERVISOR_MODE support from the IOMMU interface.
The device drivers are suggested to handle kernel DMA with PASID through
the kernel DMA APIs.

The drvdata parameter in iommu_sva_bind_device() and all callbacks is not
needed anymore. Cleanup them as well.

Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
Signed-off-by: Jacob Pan 
Signed-off-by: Lu Baolu 
Reviewed-by: Jason Gunthorpe 
---
 include/linux/intel-iommu.h   |  3 +-
 include/linux/intel-svm.h | 13 -
 include/linux/iommu.h |  8 +--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  5 +-
 drivers/dma/idxd/cdev.c   |  2 +-
 drivers/dma/idxd/init.c   | 24 +---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  3 +-
 drivers/iommu/intel/svm.c | 57 +--
 drivers/iommu/iommu.c |  5 +-
 drivers/misc/uacce/uacce.c|  2 +-
 10 files changed, 26 insertions(+), 96 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 4f29139bbfc3..df23300cfa88 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -739,8 +739,7 @@ struct intel_iommu *device_to_iommu(struct device *dev, u8 
*bus, u8 *devfn);
 extern void intel_svm_check(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
 extern int intel_svm_finish_prq(struct intel_iommu *iommu);
-struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
-void *drvdata);
+struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm);
 void intel_svm_unbind(struct iommu_sva *handle);
 u32 intel_svm_get_pasid(struct iommu_sva *handle);
 int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 207ef06ba3e1..f9a0d44f6fdb 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -13,17 +13,4 @@
 #define PRQ_RING_MASK  ((0x1000 << PRQ_ORDER) - 0x20)
 #define PRQ_DEPTH  ((0x1000 << PRQ_ORDER) >> 5)
 
-/*
- * The SVM_FLAG_SUPERVISOR_MODE flag requests a PASID which can be used only
- * for access to kernel addresses. No IOTLB flushes are automatically done
- * for kernel mappings; it is valid only for access to the kernel's static
- * 1:1 mapping of physical memory — not to vmalloc or even module mappings.
- * A future API addition may permit the use of such ranges, by means of an
- * explicit IOTLB flush call (akin to the DMA API's unmap method).
- *
- * It is unlikely that we will ever hook into flush_tlb_kernel_range() to
- * do such IOTLB flushes automatically.
- */
-#define SVM_FLAG_SUPERVISOR_MODE   BIT(0)
-
 #endif /* __INTEL_SVM_H__ */
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index da423e87f248..0c358b7c583b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -243,8 +243,7 @@ struct iommu_ops {
int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
 
-   struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
- void *drvdata);
+   struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm);
void (*sva_unbind)(struct iommu_sva *handle);
u32 (*sva_get_pasid)(struct iommu_sva *handle);
 
@@ -667,8 +666,7 @@ int iommu_dev_disable_feature(struct device *dev, enum 
iommu_dev_features f);
 bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
 
 struct iommu_sva *iommu_sva_bind_device(struct device *dev,
-   struct mm_struct *mm,
-   void *drvdata);
+   struct mm_struct *mm);
 void iommu_sva_unbind_device(struct iommu_sva *handle);
 u32 iommu_sva_get_pasid(struct iommu_sva *handle);
 
@@ -1010,7 +1008,7 @@ iommu_dev_disable_feature(struct device *dev, enum 
iommu_dev_features feat)
 }
 
 static inline struct iommu_sva *
-iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
+io

[PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support

2022-05-19 Thread Lu Baolu
The iommu_sva_domain represents a hardware pagetable that the IOMMU
hardware could use for SVA translation. This adds some infrastructure
to support SVA domain in the iommu common layer. It includes:

- Add a new struct iommu_sva_domain and new IOMMU_DOMAIN_SVA domain
  type.
- Add a new domain ops pointer in iommu_ops. The IOMMU drivers that
  support SVA should provide the callbacks.
- Add helpers to allocate and free an SVA domain.
- Add helpers to set an SVA domain to a device and the reverse
  operation.

Some buses, like PCI, route packets without considering the PASID value.
Thus a DMA target address with PASID might be treated as P2P if the
address falls into the MMIO BAR of other devices in the group. To make
things simple, the attach/detach interfaces only apply to devices
belonging to the singleton groups, and the singleton is immutable in
fabric i.e. not affected by hotplug.

The iommu_set/block_device_pasid() can be used for other purposes,
such as kernel DMA with pasid, mediation device, etc. Hence, it is put
in the iommu.c.

Suggested-by: Jean-Philippe Brucker 
Suggested-by: Jason Gunthorpe 
Signed-off-by: Lu Baolu 
---
 include/linux/iommu.h | 51 +
 drivers/iommu/iommu-sva-lib.h | 15 
 drivers/iommu/iommu-sva-lib.c | 48 +++
 drivers/iommu/iommu.c | 71 +++
 4 files changed, 185 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 0c358b7c583b..e8cf82d46ce1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -64,6 +64,9 @@ struct iommu_domain_geometry {
 #define __IOMMU_DOMAIN_PT  (1U << 2)  /* Domain is identity mapped   */
 #define __IOMMU_DOMAIN_DMA_FQ  (1U << 3)  /* DMA-API uses flush queue*/
 
+#define __IOMMU_DOMAIN_SHARED  (1U << 4)  /* Page table shared from CPU  */
+#define __IOMMU_DOMAIN_HOST_VA (1U << 5)  /* Host CPU virtual address */
+
 /*
  * This are the possible domain-types
  *
@@ -86,6 +89,8 @@ struct iommu_domain_geometry {
 #define IOMMU_DOMAIN_DMA_FQ(__IOMMU_DOMAIN_PAGING |\
 __IOMMU_DOMAIN_DMA_API |   \
 __IOMMU_DOMAIN_DMA_FQ)
+#define IOMMU_DOMAIN_SVA   (__IOMMU_DOMAIN_SHARED |\
+__IOMMU_DOMAIN_HOST_VA)
 
 struct iommu_domain {
unsigned type;
@@ -254,6 +259,7 @@ struct iommu_ops {
int (*def_domain_type)(struct device *dev);
 
const struct iommu_domain_ops *default_domain_ops;
+   const struct iommu_domain_ops *sva_domain_ops;
unsigned long pgsize_bitmap;
struct module *owner;
 };
@@ -262,6 +268,8 @@ struct iommu_ops {
  * struct iommu_domain_ops - domain specific operations
  * @attach_dev: attach an iommu domain to a device
  * @detach_dev: detach an iommu domain from a device
+ * @set_dev_pasid: set an iommu domain to a pasid of device
+ * @block_dev_pasid: block pasid of device from using iommu domain
  * @map: map a physically contiguous memory region to an iommu domain
  * @map_pages: map a physically contiguous set of pages of the same size to
  * an iommu domain.
@@ -282,6 +290,10 @@ struct iommu_ops {
 struct iommu_domain_ops {
int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
+   int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+ioasid_t pasid);
+   void (*block_dev_pasid)(struct iommu_domain *domain, struct device *dev,
+   ioasid_t pasid);
 
int (*map)(struct iommu_domain *domain, unsigned long iova,
   phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
@@ -677,6 +689,10 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, 
void *owner);
 void iommu_group_release_dma_owner(struct iommu_group *group);
 bool iommu_group_dma_owner_claimed(struct iommu_group *group);
 
+int iommu_set_device_pasid(struct iommu_domain *domain, struct device *dev,
+  ioasid_t pasid);
+void iommu_block_device_pasid(struct iommu_domain *domain, struct device *dev,
+ ioasid_t pasid);
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -1050,6 +1066,17 @@ static inline bool iommu_group_dma_owner_claimed(struct 
iommu_group *group)
 {
return false;
 }
+
+static inline int iommu_set_device_pasid(struct iommu_domain *domain,
+struct device *dev, ioasid_t pasid)
+{
+   return -ENODEV;
+}
+
+static inline void iommu_block_device_pasid(struct iommu_domain *domain,
+   struct device *dev, ioasid_t pasid)
+{
+}
 #endif /* CONFIG_IOMMU_API */
 
 /**
@@ -1075,4 +1102,28 @@ void iommu_debugfs_setup(void);
 static inline void iommu_debugfs_setup(void) {}
 #endif
 
+#ifdef CONFIG_IOMMU_SVA
+struct iommu_domain 

[PATCH v7 04/10] iommu/vt-d: Add SVA domain support

2022-05-19 Thread Lu Baolu
Add support for domain ops callbacks for an SVA domain.

Signed-off-by: Lu Baolu 
---
 include/linux/intel-iommu.h |  4 
 drivers/iommu/intel/iommu.c |  4 
 drivers/iommu/intel/svm.c   | 37 -
 3 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index df23300cfa88..5e88eaa245aa 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -744,6 +744,10 @@ void intel_svm_unbind(struct iommu_sva *handle);
 u32 intel_svm_get_pasid(struct iommu_sva *handle);
 int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
struct iommu_page_response *msg);
+int intel_svm_attach_dev_pasid(struct iommu_domain *domain,
+  struct device *dev, ioasid_t pasid);
+void intel_svm_detach_dev_pasid(struct iommu_domain *domain,
+   struct device *dev, ioasid_t pasid);
 
 struct intel_svm_dev {
struct list_head list;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e56b3a4b6998..2b6a52c87c73 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4923,6 +4923,10 @@ const struct iommu_ops intel_iommu_ops = {
.sva_unbind = intel_svm_unbind,
.sva_get_pasid  = intel_svm_get_pasid,
.page_response  = intel_svm_page_response,
+   .sva_domain_ops = &(const struct iommu_domain_ops) {
+   .set_dev_pasid  = intel_svm_attach_dev_pasid,
+   .block_dev_pasid= intel_svm_detach_dev_pasid,
+   },
 #endif
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = intel_iommu_attach_device,
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index d04880a291c3..d575792441f3 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -323,6 +323,7 @@ static int intel_svm_alloc_pasid(struct device *dev, struct 
mm_struct *mm)
 
 static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
   struct device *dev,
+  ioasid_t pasid,
   struct mm_struct *mm)
 {
struct device_domain_info *info = dev_iommu_priv_get(dev);
@@ -331,13 +332,13 @@ static struct iommu_sva *intel_svm_bind_mm(struct 
intel_iommu *iommu,
struct intel_svm *svm;
int ret = 0;
 
-   svm = pasid_private_find(mm->pasid);
+   svm = pasid_private_find(pasid);
if (!svm) {
svm = kzalloc(sizeof(*svm), GFP_KERNEL);
if (!svm)
return ERR_PTR(-ENOMEM);
 
-   svm->pasid = mm->pasid;
+   svm->pasid = pasid;
svm->mm = mm;
INIT_LIST_HEAD_RCU(&svm->devs);
 
@@ -387,7 +388,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct 
intel_iommu *iommu,
/* Setup the pasid table: */
sflags = cpu_feature_enabled(X86_FEATURE_LA57) ? PASID_FLAG_FL5LP : 0;
spin_lock_irqsave(&iommu->lock, iflags);
-   ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd, mm->pasid,
+   ret = intel_pasid_setup_first_level(iommu, dev, mm->pgd, pasid,
FLPT_DEFAULT_DID, sflags);
spin_unlock_irqrestore(&iommu->lock, iflags);
 
@@ -403,7 +404,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct 
intel_iommu *iommu,
 free_svm:
if (list_empty(&svm->devs)) {
mmu_notifier_unregister(&svm->notifier, mm);
-   pasid_private_remove(mm->pasid);
+   pasid_private_remove(pasid);
kfree(svm);
}
 
@@ -822,7 +823,7 @@ struct iommu_sva *intel_svm_bind(struct device *dev, struct 
mm_struct *mm)
return ERR_PTR(ret);
}
 
-   sva = intel_svm_bind_mm(iommu, dev, mm);
+   sva = intel_svm_bind_mm(iommu, dev, mm->pasid, mm);
mutex_unlock(&pasid_mutex);
 
return sva;
@@ -931,3 +932,29 @@ int intel_svm_page_response(struct device *dev,
mutex_unlock(&pasid_mutex);
return ret;
 }
+
+int intel_svm_attach_dev_pasid(struct iommu_domain *domain,
+  struct device *dev, ioasid_t pasid)
+{
+   struct device_domain_info *info = dev_iommu_priv_get(dev);
+   struct mm_struct *mm = domain_to_mm(domain);
+   struct intel_iommu *iommu = info->iommu;
+   struct iommu_sva *sva;
+   int ret = 0;
+
+   mutex_lock(&pasid_mutex);
+   sva = intel_svm_bind_mm(iommu, dev, pasid, mm);
+   if (IS_ERR(sva))
+   ret = PTR_ERR(sva);
+   mutex_unlock(&pasid_mutex);
+
+   return ret;
+}
+
+void intel_svm_detach_dev_pasid(struct iommu_domain *domain,
+   struct device *dev, ioasid_t pasid)
+{
+   mutex_lock(&pasid_mutex);
+  

[PATCH v7 05/10] arm-smmu-v3/sva: Add SVA domain support

2022-05-19 Thread Lu Baolu
Add support for domain ops callbacks for an SVA domain.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  4 ++
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 46 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  6 +++
 3 files changed, 56 insertions(+)

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 d2ba86470c42..ec77f6a51ff9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -758,6 +758,10 @@ struct iommu_sva *arm_smmu_sva_bind(struct device *dev, 
struct mm_struct *mm);
 void arm_smmu_sva_unbind(struct iommu_sva *handle);
 u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle);
 void arm_smmu_sva_notifier_synchronize(void);
+int arm_smmu_sva_attach_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t id);
+void arm_smmu_sva_detach_dev_pasid(struct iommu_domain *domain,
+  struct device *dev, ioasid_t id);
 #else /* CONFIG_ARM_SMMU_V3_SVA */
 static inline bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
 {
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 f155d406c5d5..6969974ca89e 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
@@ -549,3 +549,49 @@ void arm_smmu_sva_notifier_synchronize(void)
 */
mmu_notifier_synchronize();
 }
+
+int arm_smmu_sva_attach_dev_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t id)
+{
+   int ret = 0;
+   struct mm_struct *mm;
+   struct iommu_sva *handle;
+
+   if (domain->type != IOMMU_DOMAIN_SVA)
+   return -EINVAL;
+
+   mm = domain_to_mm(domain);
+   if (WARN_ON(!mm))
+   return -ENODEV;
+
+   mutex_lock(&sva_lock);
+   handle = __arm_smmu_sva_bind(dev, mm);
+   if (IS_ERR(handle))
+   ret = PTR_ERR(handle);
+   mutex_unlock(&sva_lock);
+
+   return ret;
+}
+
+void arm_smmu_sva_detach_dev_pasid(struct iommu_domain *domain,
+  struct device *dev, ioasid_t id)
+{
+   struct arm_smmu_bond *bond = NULL, *t;
+   struct mm_struct *mm = domain_to_mm(domain);
+   struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+
+   mutex_lock(&sva_lock);
+   list_for_each_entry(t, &master->bonds, list) {
+   if (t->mm == mm) {
+   bond = t;
+   break;
+   }
+   }
+
+   if (!WARN_ON(!bond) && refcount_dec_and_test(&bond->refs)) {
+   list_del(&bond->list);
+   arm_smmu_mmu_notifier_put(bond->smmu_mn);
+   kfree(bond);
+   }
+   mutex_unlock(&sva_lock);
+}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 6e2cd082c670..4ad3ca70cf89 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2858,6 +2858,12 @@ static struct iommu_ops arm_smmu_ops = {
.page_response  = arm_smmu_page_response,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
.owner  = THIS_MODULE,
+#ifdef CONFIG_ARM_SMMU_V3_SVA
+   .sva_domain_ops = &(const struct iommu_domain_ops) {
+   .set_dev_pasid  = arm_smmu_sva_attach_dev_pasid,
+   .block_dev_pasid= arm_smmu_sva_detach_dev_pasid,
+   },
+#endif
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = arm_smmu_attach_dev,
.map_pages  = arm_smmu_map_pages,
-- 
2.25.1

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


[PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-19 Thread Lu Baolu
The existing iommu SVA interfaces are implemented by calling the SVA
specific iommu ops provided by the IOMMU drivers. There's no need for
any SVA specific ops in iommu_ops vector anymore as we can achieve
this through the generic attach/detach_dev_pasid domain ops.

This refactors the IOMMU SVA interfaces implementation by using the
set/block_pasid_dev ops and align them with the concept of the SVA
iommu domain. Put the new SVA code in the sva related file in order
to make it self-contained.

Signed-off-by: Lu Baolu 
---
 include/linux/iommu.h |  48 --
 drivers/iommu/iommu-sva-lib.h |   1 +
 drivers/iommu/iommu-sva-lib.c | 113 
 drivers/iommu/iommu.c | 119 --
 4 files changed, 170 insertions(+), 111 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e8cf82d46ce1..d9ac5ebe5bbb 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -635,6 +635,7 @@ struct iommu_fwspec {
  */
 struct iommu_sva {
struct device   *dev;
+   refcount_t  users;
 };
 
 int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
@@ -677,11 +678,6 @@ int iommu_dev_enable_feature(struct device *dev, enum 
iommu_dev_features f);
 int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
 bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features f);
 
-struct iommu_sva *iommu_sva_bind_device(struct device *dev,
-   struct mm_struct *mm);
-void iommu_sva_unbind_device(struct iommu_sva *handle);
-u32 iommu_sva_get_pasid(struct iommu_sva *handle);
-
 int iommu_device_use_default_domain(struct device *dev);
 void iommu_device_unuse_default_domain(struct device *dev);
 
@@ -693,6 +689,8 @@ int iommu_set_device_pasid(struct iommu_domain *domain, 
struct device *dev,
   ioasid_t pasid);
 void iommu_block_device_pasid(struct iommu_domain *domain, struct device *dev,
  ioasid_t pasid);
+struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -1023,21 +1021,6 @@ iommu_dev_disable_feature(struct device *dev, enum 
iommu_dev_features feat)
return -ENODEV;
 }
 
-static inline struct iommu_sva *
-iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
-{
-   return NULL;
-}
-
-static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
-{
-}
-
-static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
-{
-   return IOMMU_PASID_INVALID;
-}
-
 static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
 {
return NULL;
@@ -1077,6 +1060,12 @@ static inline void iommu_block_device_pasid(struct 
iommu_domain *domain,
struct device *dev, ioasid_t pasid)
 {
 }
+
+static inline struct iommu_domain *
+iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
+{
+   return NULL;
+}
 #endif /* CONFIG_IOMMU_API */
 
 /**
@@ -1108,6 +1097,10 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
mm_struct *mm);
 void iommu_sva_free_domain(struct iommu_domain *domain);
 int iommu_sva_set_domain(struct iommu_domain *domain, struct device *dev,
 ioasid_t pasid);
+struct iommu_sva *iommu_sva_bind_device(struct device *dev,
+   struct mm_struct *mm);
+void iommu_sva_unbind_device(struct iommu_sva *handle);
+u32 iommu_sva_get_pasid(struct iommu_sva *handle);
 #else /* CONFIG_IOMMU_SVA */
 static inline struct iommu_domain *
 iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm)
@@ -1124,6 +1117,21 @@ static inline int iommu_sva_set_domain(struct 
iommu_domain *domain,
 {
return -EINVAL;
 }
+
+static inline struct iommu_sva *
+iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
+{
+   return NULL;
+}
+
+static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
+{
+}
+
+static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
+{
+   return IOMMU_PASID_INVALID;
+}
 #endif /* CONFIG_IOMMU_SVA */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
index 1be21e6b93ec..ebab5a8cb126 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva-lib.h
@@ -20,6 +20,7 @@ struct iopf_queue;
 struct iommu_sva_domain {
struct iommu_domain domain;
struct mm_struct*mm;
+   struct iommu_svabond;
 };
 
 #define to_sva_domain(d) container_of_safe(d, struct iommu_sva_domain, domain)
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
index 210c376f6043..568e0f64edac 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva-lib.c
@@ -4,6 +4,8 @@
  */
 #include 
 #include 
+#include 
+#include 
 
 #include "i

[PATCH v7 07/10] iommu: Remove SVA related callbacks from iommu ops

2022-05-19 Thread Lu Baolu
These ops'es have been replaced with the dev_attach/detach_pasid domain
ops'es. There's no need for them anymore. Remove them to avoid dead
code.

Signed-off-by: Lu Baolu 
Reviewed-by: Jean-Philippe Brucker 
---
 include/linux/intel-iommu.h   |  3 --
 include/linux/iommu.h |  7 ---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   | 16 --
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 40 ---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  3 --
 drivers/iommu/intel/iommu.c   |  3 --
 drivers/iommu/intel/svm.c | 49 ---
 7 files changed, 121 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 5e88eaa245aa..536f229fd274 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -739,9 +739,6 @@ struct intel_iommu *device_to_iommu(struct device *dev, u8 
*bus, u8 *devfn);
 extern void intel_svm_check(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
 extern int intel_svm_finish_prq(struct intel_iommu *iommu);
-struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm);
-void intel_svm_unbind(struct iommu_sva *handle);
-u32 intel_svm_get_pasid(struct iommu_sva *handle);
 int intel_svm_page_response(struct device *dev, struct iommu_fault_event *evt,
struct iommu_page_response *msg);
 int intel_svm_attach_dev_pasid(struct iommu_domain *domain,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d9ac5ebe5bbb..e4ce2fe0e144 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -212,9 +212,6 @@ struct iommu_iotlb_gather {
  * @dev_has/enable/disable_feat: per device entries to check/enable/disable
  *   iommu specific features.
  * @dev_feat_enabled: check enabled feature
- * @sva_bind: Bind process address space to device
- * @sva_unbind: Unbind process address space from device
- * @sva_get_pasid: Get PASID associated to a SVA handle
  * @page_response: handle page request response
  * @def_domain_type: device default domain type, return value:
  * - IOMMU_DOMAIN_IDENTITY: must use an identity domain
@@ -248,10 +245,6 @@ struct iommu_ops {
int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
 
-   struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm);
-   void (*sva_unbind)(struct iommu_sva *handle);
-   u32 (*sva_get_pasid)(struct iommu_sva *handle);
-
int (*page_response)(struct device *dev,
 struct iommu_fault_event *evt,
 struct iommu_page_response *msg);
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 ec77f6a51ff9..0f0f5ba26dd5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -754,9 +754,6 @@ 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);
 bool arm_smmu_master_iopf_supported(struct arm_smmu_master *master);
-struct iommu_sva *arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm);
-void arm_smmu_sva_unbind(struct iommu_sva *handle);
-u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle);
 void arm_smmu_sva_notifier_synchronize(void);
 int arm_smmu_sva_attach_dev_pasid(struct iommu_domain *domain,
  struct device *dev, ioasid_t id);
@@ -793,19 +790,6 @@ static inline bool arm_smmu_master_iopf_supported(struct 
arm_smmu_master *master
return false;
 }
 
-static inline struct iommu_sva *
-arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm)
-{
-   return ERR_PTR(-ENODEV);
-}
-
-static inline void arm_smmu_sva_unbind(struct iommu_sva *handle) {}
-
-static inline u32 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 6969974ca89e..8290d66569f3 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
@@ -344,11 +344,6 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct 
*mm)
if (!bond)
return ERR_PTR(-ENOMEM);
 
-   /* Allocate a PASID for this mm if necessary */
-   ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) - 1);
-   if (ret)
-   goto err_free_bond;
-
bond->mm = mm;
bond->sva.dev = dev;
refcount_set(&bond->refs, 1);
@@ -367,41 +362,6 @@ __arm_smmu_sva

[PATCH v7 08/10] iommu: Prepare IOMMU domain for IOPF

2022-05-19 Thread Lu Baolu
This adds some mechanisms around the iommu_domain so that the I/O page
fault handling framework could route a page fault to the domain and
call the fault handler from it.

Add pointers to the page fault handler and its private data in struct
iommu_domain. The fault handler will be called with the private data
as a parameter once a page fault is routed to the domain. Any kernel
component which owns an iommu domain could install handler and its
private parameter so that the page fault could be further routed and
handled.

This also prepares the SVA implementation to be the first consumer of
the per-domain page fault handling model.

Suggested-by: Jean-Philippe Brucker 
Signed-off-by: Lu Baolu 
---
 include/linux/iommu.h |  3 ++
 drivers/iommu/io-pgfault.c|  7 
 drivers/iommu/iommu-sva-lib.c | 65 +++
 3 files changed, 75 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e4ce2fe0e144..45f274b2640d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -100,6 +100,9 @@ struct iommu_domain {
void *handler_token;
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
+   enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
+ void *data);
+   void *fault_data;
 };
 
 static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 1df8c1dcae77..aee9e033012f 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -181,6 +181,13 @@ static void iopf_handle_group(struct work_struct *work)
  * request completes, outstanding faults will have been dealt with by the time
  * the PASID is freed.
  *
+ * Any valid page fault will be eventually routed to an iommu domain and the
+ * page fault handler installed there will get called. The users of this
+ * handling framework should guarantee that the iommu domain could only be
+ * freed after the device has stopped generating page faults (or the iommu
+ * hardware has been set to block the page faults) and the pending page faults
+ * have been flushed.
+ *
  * Return: 0 on success and <0 on error.
  */
 int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
index 568e0f64edac..317ab8e8c149 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva-lib.c
@@ -72,6 +72,69 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid)
 }
 EXPORT_SYMBOL_GPL(iommu_sva_find);
 
+/*
+ * I/O page fault handler for SVA
+ *
+ * Copied from io-pgfault.c with mmget_not_zero() added before
+ * mmap_read_lock().
+ */
+static enum iommu_page_response_code
+iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
+{
+   vm_fault_t ret;
+   struct mm_struct *mm;
+   struct vm_area_struct *vma;
+   unsigned int access_flags = 0;
+   struct iommu_domain *domain = data;
+   unsigned int fault_flags = FAULT_FLAG_REMOTE;
+   struct iommu_fault_page_request *prm = &fault->prm;
+   enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
+
+   if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
+   return status;
+
+   mm = domain_to_mm(domain);
+   if (IS_ERR_OR_NULL(mm) || !mmget_not_zero(mm))
+   return status;
+
+   mmap_read_lock(mm);
+
+   vma = find_extend_vma(mm, prm->addr);
+   if (!vma)
+   /* Unmapped area */
+   goto out_put_mm;
+
+   if (prm->perm & IOMMU_FAULT_PERM_READ)
+   access_flags |= VM_READ;
+
+   if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
+   access_flags |= VM_WRITE;
+   fault_flags |= FAULT_FLAG_WRITE;
+   }
+
+   if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
+   access_flags |= VM_EXEC;
+   fault_flags |= FAULT_FLAG_INSTRUCTION;
+   }
+
+   if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
+   fault_flags |= FAULT_FLAG_USER;
+
+   if (access_flags & ~vma->vm_flags)
+   /* Access fault */
+   goto out_put_mm;
+
+   ret = handle_mm_fault(vma, prm->addr, fault_flags, NULL);
+   status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
+   IOMMU_PAGE_RESP_SUCCESS;
+
+out_put_mm:
+   mmap_read_unlock(mm);
+   mmput(mm);
+
+   return status;
+}
+
 /*
  * IOMMU SVA driver-oriented interfaces
  */
@@ -94,6 +157,8 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
mm_struct *mm)
domain = &sva_domain->domain;
domain->type = IOMMU_DOMAIN_SVA;
domain->ops = bus->iommu_ops->sva_domain_ops;
+   domain->iopf_handler = iommu_sva_handle_iopf;
+   domain->fault_data = domain;
 
return domain;
 }
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation

[PATCH v7 09/10] iommu: Per-domain I/O page fault handling

2022-05-19 Thread Lu Baolu
Tweak the I/O page fault handling framework to route the page faults to
the domain and call the page fault handler retrieved from the domain.
This makes the I/O page fault handling framework possible to serve more
usage scenarios as long as they have an IOMMU domain and install a page
fault handler in it. Some unused functions are also removed to avoid
dead code.

The iommu_get_domain_for_dev_pasid() which retrieves attached domain
for a {device, PASID} pair is used. It will be used by the page fault
handling framework which knows {device, PASID} reported from the iommu
driver. We have a guarantee that the SVA domain doesn't go away during
IOPF handling, because unbind() waits for pending faults with
iopf_queue_flush_dev() before freeing the domain. Hence, there's no need
to synchronize life cycle of the iommu domains between the unbind() and
the interrupt threads.

Signed-off-by: Lu Baolu 
Reviewed-by: Jean-Philippe Brucker 
---
 drivers/iommu/io-pgfault.c | 64 +-
 1 file changed, 7 insertions(+), 57 deletions(-)

diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index aee9e033012f..4f24ec703479 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -69,69 +69,18 @@ static int iopf_complete_group(struct device *dev, struct 
iopf_fault *iopf,
return iommu_page_response(dev, &resp);
 }
 
-static enum iommu_page_response_code
-iopf_handle_single(struct iopf_fault *iopf)
-{
-   vm_fault_t ret;
-   struct mm_struct *mm;
-   struct vm_area_struct *vma;
-   unsigned int access_flags = 0;
-   unsigned int fault_flags = FAULT_FLAG_REMOTE;
-   struct iommu_fault_page_request *prm = &iopf->fault.prm;
-   enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
-
-   if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
-   return status;
-
-   mm = iommu_sva_find(prm->pasid);
-   if (IS_ERR_OR_NULL(mm))
-   return status;
-
-   mmap_read_lock(mm);
-
-   vma = find_extend_vma(mm, prm->addr);
-   if (!vma)
-   /* Unmapped area */
-   goto out_put_mm;
-
-   if (prm->perm & IOMMU_FAULT_PERM_READ)
-   access_flags |= VM_READ;
-
-   if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
-   access_flags |= VM_WRITE;
-   fault_flags |= FAULT_FLAG_WRITE;
-   }
-
-   if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
-   access_flags |= VM_EXEC;
-   fault_flags |= FAULT_FLAG_INSTRUCTION;
-   }
-
-   if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
-   fault_flags |= FAULT_FLAG_USER;
-
-   if (access_flags & ~vma->vm_flags)
-   /* Access fault */
-   goto out_put_mm;
-
-   ret = handle_mm_fault(vma, prm->addr, fault_flags, NULL);
-   status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
-   IOMMU_PAGE_RESP_SUCCESS;
-
-out_put_mm:
-   mmap_read_unlock(mm);
-   mmput(mm);
-
-   return status;
-}
-
 static void iopf_handle_group(struct work_struct *work)
 {
struct iopf_group *group;
+   struct iommu_domain *domain;
struct iopf_fault *iopf, *next;
enum iommu_page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
 
group = container_of(work, struct iopf_group, work);
+   domain = iommu_get_domain_for_dev_pasid(group->dev,
+   group->last_fault.fault.prm.pasid);
+   if (!domain || !domain->iopf_handler)
+   status = IOMMU_PAGE_RESP_INVALID;
 
list_for_each_entry_safe(iopf, next, &group->faults, list) {
/*
@@ -139,7 +88,8 @@ static void iopf_handle_group(struct work_struct *work)
 * faults in the group if there is an error.
 */
if (status == IOMMU_PAGE_RESP_SUCCESS)
-   status = iopf_handle_single(iopf);
+   status = domain->iopf_handler(&iopf->fault,
+ domain->fault_data);
 
if (!(iopf->fault.prm.flags &
  IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE))
-- 
2.25.1

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


[PATCH v7 10/10] iommu: Rename iommu-sva-lib.{c,h}

2022-05-19 Thread Lu Baolu
Rename iommu-sva-lib.c[h] to iommu-sva.c[h] as it contains all code
for SVA implementation in iommu core.

Signed-off-by: Lu Baolu 
Reviewed-by: Jean-Philippe Brucker 
---
 drivers/iommu/{iommu-sva-lib.h => iommu-sva.h}  | 0
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
 drivers/iommu/intel/iommu.c | 2 +-
 drivers/iommu/intel/svm.c   | 2 +-
 drivers/iommu/io-pgfault.c  | 2 +-
 drivers/iommu/{iommu-sva-lib.c => iommu-sva.c}  | 2 +-
 drivers/iommu/Makefile  | 2 +-
 8 files changed, 7 insertions(+), 7 deletions(-)
 rename drivers/iommu/{iommu-sva-lib.h => iommu-sva.h} (100%)
 rename drivers/iommu/{iommu-sva-lib.c => iommu-sva.c} (99%)

diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva.h
similarity index 100%
rename from drivers/iommu/iommu-sva-lib.h
rename to drivers/iommu/iommu-sva.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 8290d66569f3..b238b09c4999 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
@@ -10,7 +10,7 @@
 #include 
 
 #include "arm-smmu-v3.h"
-#include "../../iommu-sva-lib.h"
+#include "../../iommu-sva.h"
 #include "../../io-pgtable-arm.h"
 
 struct arm_smmu_mmu_notifier {
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index b74f8964cc13..e1b80b7ac858 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -31,7 +31,7 @@
 #include 
 
 #include "arm-smmu-v3.h"
-#include "../../iommu-sva-lib.h"
+#include "../../iommu-sva.h"
 
 static bool disable_bypass = true;
 module_param(disable_bypass, bool, 0444);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 10e07d59d4c8..2289939725a7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -27,7 +27,7 @@
 #include 
 
 #include "../irq_remapping.h"
-#include "../iommu-sva-lib.h"
+#include "../iommu-sva.h"
 #include "pasid.h"
 #include "cap_audit.h"
 
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index e412a442d9a4..2c45579e5e81 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -25,7 +25,7 @@
 
 #include "pasid.h"
 #include "perf.h"
-#include "../iommu-sva-lib.h"
+#include "../iommu-sva.h"
 
 static irqreturn_t prq_event_thread(int irq, void *d);
 static void intel_svm_drain_prq(struct device *dev, u32 pasid);
diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
index 4f24ec703479..91b1c6bd01d4 100644
--- a/drivers/iommu/io-pgfault.c
+++ b/drivers/iommu/io-pgfault.c
@@ -11,7 +11,7 @@
 #include 
 #include 
 
-#include "iommu-sva-lib.h"
+#include "iommu-sva.h"
 
 /**
  * struct iopf_queue - IO Page Fault queue
diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva.c
similarity index 99%
rename from drivers/iommu/iommu-sva-lib.c
rename to drivers/iommu/iommu-sva.c
index 317ab8e8c149..0e4f7c889f54 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva.c
@@ -7,7 +7,7 @@
 #include 
 #include 
 
-#include "iommu-sva-lib.h"
+#include "iommu-sva.h"
 
 static DEFINE_MUTEX(iommu_sva_lock);
 static DECLARE_IOASID_SET(iommu_sva_pasid);
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 44475a9b3eea..c1763476162b 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -27,6 +27,6 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
 obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
-obj-$(CONFIG_IOMMU_SVA) += iommu-sva-lib.o io-pgfault.o
+obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o io-pgfault.o
 obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
 obj-$(CONFIG_APPLE_DART) += apple-dart.o
-- 
2.25.1

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


Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-19 Thread Joerg Roedel
On Wed, May 18, 2022 at 04:14:46PM -0300, Jason Gunthorpe wrote:
> Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must always assign 
> a domain")
> Reported-by: Eric Farman 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/vfio/vfio.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)

That fix will be taken care of by Alex? Or does it need to go through
the IOMMU tree?

Regards,

-- 
Jörg Rödel
jroe...@suse.de

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
 
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

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


Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers

2022-05-19 Thread Yi Liu

Hi Jason,

On 2022/3/19 01:27, Jason Gunthorpe wrote:


+/**
+ * iommufd_device_attach - Connect a device to an iommu_domain
+ * @idev: device to attach
+ * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
+ * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
+ * @flags: Optional flags
+ *
+ * This connects the device to an iommu_domain, either automatically or 
manually
+ * selected. Once this completes the device could do DMA.
+ *
+ * The caller should return the resulting pt_id back to userspace.
+ * This function is undone by calling iommufd_device_detach().
+ */
+int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id,
+ unsigned int flags)
+{
+   struct iommufd_hw_pagetable *hwpt;
+   int rc;
+
+   refcount_inc(&idev->obj.users);
+
+   hwpt = iommufd_hw_pagetable_from_id(idev->ictx, *pt_id, idev->dev);
+   if (IS_ERR(hwpt)) {
+   rc = PTR_ERR(hwpt);
+   goto out_users;
+   }
+
+   mutex_lock(&hwpt->devices_lock);
+   /* FIXME: Use a device-centric iommu api. For now check if the
+* hw_pagetable already has a device of the same group joined to tell if
+* we are the first and need to attach the group. */
+   if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+   phys_addr_t sw_msi_start = 0;
+
+   rc = iommu_attach_group(hwpt->domain, idev->group);
+   if (rc)
+   goto out_unlock;
+
+   /*
+* hwpt is now the exclusive owner of the group so this is the
+* first time enforce is called for this group.
+*/
+   rc = iopt_table_enforce_group_resv_regions(
+   &hwpt->ioas->iopt, idev->group, &sw_msi_start);
+   if (rc)
+   goto out_detach;
+   rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start, flags);
+   if (rc)
+   goto out_iova;
+   }
+
+   idev->hwpt = hwpt;
+   if (list_empty(&hwpt->devices)) {
+   rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
+   if (rc)
+   goto out_iova;
+   }
+   list_add(&idev->devices_item, &hwpt->devices);


Just double check here.
This API doesn't prevent caller from calling this API multiple times with
the same @idev and @pt_id. right? Note that idev has only one device_item
list head. If caller does do multiple callings, then there should be
problem. right? If so, this API assumes caller should take care of it and
not do such bad function call. Is this the design here?


+   mutex_unlock(&hwpt->devices_lock);
+
+   *pt_id = idev->hwpt->obj.id;
+   return 0;
+
+out_iova:
+   iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->group);
+out_detach:
+   iommu_detach_group(hwpt->domain, idev->group);
+out_unlock:
+   mutex_unlock(&hwpt->devices_lock);
+   iommufd_hw_pagetable_put(idev->ictx, hwpt);
+out_users:
+   refcount_dec(&idev->obj.users);
+   return rc;
+}
+EXPORT_SYMBOL_GPL(iommufd_device_attach);
+
+void iommufd_device_detach(struct iommufd_device *idev)
+{
+   struct iommufd_hw_pagetable *hwpt = idev->hwpt;
+
+   mutex_lock(&hwpt->devices_lock);
+   list_del(&idev->devices_item);
+   if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+   iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->group);
+   iommu_detach_group(hwpt->domain, idev->group);
+   }
+   if (list_empty(&hwpt->devices))
+   iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain);
+   mutex_unlock(&hwpt->devices_lock);
+
+   iommufd_hw_pagetable_put(idev->ictx, hwpt);
+   idev->hwpt = NULL;
+
+   refcount_dec(&idev->obj.users);
+}
+EXPORT_SYMBOL_GPL(iommufd_device_detach);
diff --git a/drivers/iommu/iommufd/iommufd_private.h 
b/drivers/iommu/iommufd/iommufd_private.h
index c5c9650cc86818..e5c717231f851e 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -96,6 +96,7 @@ static inline int iommufd_ucmd_respond(struct iommufd_ucmd 
*ucmd,
  enum iommufd_object_type {
IOMMUFD_OBJ_NONE,
IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE,
+   IOMMUFD_OBJ_DEVICE,
IOMMUFD_OBJ_HW_PAGETABLE,
IOMMUFD_OBJ_IOAS,
IOMMUFD_OBJ_MAX,
@@ -196,6 +197,7 @@ struct iommufd_hw_pagetable {
struct iommufd_object obj;
struct iommufd_ioas *ioas;
struct iommu_domain *domain;
+   bool msi_cookie;
/* Head at iommufd_ioas::auto_domains */
struct list_head auto_domains_item;
struct mutex devices_lock;
@@ -209,4 +211,6 @@ void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
  struct iommufd_hw_pagetable *hwpt);
  void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
  
+void iommufd_device_destroy(struct iommufd_object *obj);

+

Re: [PATCH] iommu/dma: Add config for PCI SAC address trick

2022-05-19 Thread John Garry via iommu

On 18/05/2022 18:36, Robin Murphy wrote:

For devices stuck behind a conventional PCI bus, saving extra cycles at
33MHz is probably fairly significant. However since native PCI Express
is now the norm for high-performance devices, the optimisation to always
prefer 32-bit addresses for the sake of avoiding DAC is starting to look
rather anachronistic. Technically 32-bit addresses do have shorter TLPs
on PCIe, but unless the device is saturating its link bandwidth with
small transfers it seems unlikely that the difference is appreciable.

What definitely is appreciable, however, is that the IOVA allocator
doesn't behave all that well once the 32-bit space starts getting full.
As DMA working sets get bigger, this optimisation increasingly backfires
and adds considerable overhead to the dma_map path for use-cases like
high-bandwidth networking. We've increasingly bandaged the allocator
in attempts to mitigate this, but it remains fundamentally at odds with
other valid requirements to try as hard as possible to satisfy a request
within the given limit; what we really need is to just avoid this odd
notion of a speculative allocation when it isn't beneficial anyway.

Unfortunately that's where things get awkward... Having been present on
x86 for 15 years or so now, it turns out there are systems which fail to
properly define the upper limit of usable IOVA space for certain devices
and this trick was the only thing letting them work OK. I had a similar
ulterior motive for a couple of early arm64 systems when originally
adding it to iommu-dma, but those really should now be fixed with proper
firmware bindings, and other arm64 users really need it out of the way,
so let's just leave it default-on for x86.

Signed-off-by: Robin Murphy 
---
  drivers/iommu/Kconfig | 24 
  drivers/iommu/dma-iommu.c |  2 +-


It might be worth printing this default value always and not just for 
when it is set from commandline, like what we do for default domain type 
and IOTLB invalidation policy



  2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c79a0df090c0..bf9b295f1c89 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -144,6 +144,30 @@ config IOMMU_DMA
select IRQ_MSI_IOMMU
select NEED_SG_DMA_LENGTH
  
+config IOMMU_DMA_PCI_SAC_OPT

+   bool "Enable 64-bit legacy PCI optimisation by default"
+   depends on IOMMU_DMA
+   default X86


Do we have a strategy for if and when issues start popping up on other 
architectures? Is it to simply tell them to just turn this flag on (and 
also fix your platform)?



+   help
+ Enable by default an IOMMU optimisation for 64-bit legacy PCI devices,
+ wherein the DMA API layer will always first try to allocate a 32-bit
+ DMA address suitable for a single address cycle, before falling back
+ to allocating from the full usable address range. If your system has
+ 64-bit legacy PCI devices in 32-bit slots where using dual address
+ cycles reduces DMA throughput significantly, this optimisation may be
+ beneficial to overall performance.
+
+ If you have a modern PCI Express based system, it should usually be
+ safe to say "n" here and avoid the potential extra allocation 
overhead.
+ However, beware that this optimisation has also historically papered
+ over bugs where the IOMMU address range above 32 bits is not fully
+ usable. If device DMA problems and/or IOMMU faults start occurring
+ with IOMMU translation enabled after disabling this option, it is
+ likely a sign of a latent driver or firmware/BIOS bug.
+
+ If this option is not set, the optimisation can be enabled at
+ boot time with the "iommu.forcedac=0" command-line argument.
+


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


Re: [PATCH v7 01/10] iommu: Add pasids field in struct iommu_device

2022-05-19 Thread Jean-Philippe Brucker
Hi Baolu,

On Thu, May 19, 2022 at 03:20:38PM +0800, Lu Baolu wrote:
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 88817a3376ef..6e2cd082c670 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -3546,6 +3546,7 @@ static int arm_smmu_device_hw_probe(struct 
> arm_smmu_device *smmu)
>   /* SID/SSID sizes */
>   smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);
>   smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
> + smmu->iommu.pasids = smmu->ssid_bits;

This should be 1UL << smmu->ssid_bits

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


Re: [PATCH v7 01/10] iommu: Add pasids field in struct iommu_device

2022-05-19 Thread Baolu Lu

Hi Jean,

On 2022/5/19 18:37, Jean-Philippe Brucker wrote:

On Thu, May 19, 2022 at 03:20:38PM +0800, Lu Baolu wrote:

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 88817a3376ef..6e2cd082c670 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3546,6 +3546,7 @@ static int arm_smmu_device_hw_probe(struct 
arm_smmu_device *smmu)
/* SID/SSID sizes */
smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg);
smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg);
+   smmu->iommu.pasids = smmu->ssid_bits;

This should be 1UL << smmu->ssid_bits


Done. Thank you for the reminding.

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


Re: [syzbot] WARNING in __dma_map_sg_attrs

2022-05-19 Thread Dmitry Vyukov via iommu
On Tue, 8 Feb 2022 at 13:26, Daniel Vetter  wrote:
>
> On Sat, Feb 05, 2022 at 12:18:23PM -0800, syzbot wrote:
> > syzbot has found a reproducer for the following issue on:
> >
> > HEAD commit:0457e5153e0e Merge tag 'for-linus' of git://git.kernel.org..
> > git tree:   upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11b2637c70
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=6f043113811433a5
> > dashboard link: https://syzkaller.appspot.com/bug?extid=10e27961f4da37c443b2
> > compiler:   gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils 
> > for Debian) 2.35.2
> > syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=11c6554270
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1163f48070
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+10e27961f4da37c44...@syzkaller.appspotmail.com
>
> Adding Gerd, since this seems to blow up in udmabuf.
>
> I wonder why syzbot didn't figure this out, since it seems to have
> correctly added both dma-api and dma-buf people. Just not the maintainer
> for the begin_cpu_udmabuf function in the middle of the backtrace?

Hi Daniel,

syzbot selects only 1 file to get maintainers.
Do you suggest using all files in the stack trace? I think it may lead
to too many developers CCed since there can be something like 20 files
including something from scheduler, arch, fs, etc.



> > [ cut here ]
> > WARNING: CPU: 1 PID: 3595 at kernel/dma/mapping.c:188 
> > __dma_map_sg_attrs+0x181/0x1f0 kernel/dma/mapping.c:188
> > Modules linked in:
> > CPU: 0 PID: 3595 Comm: syz-executor249 Not tainted 
> > 5.17.0-rc2-syzkaller-00316-g0457e5153e0e #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS 
> > Google 01/01/2011
> > RIP: 0010:__dma_map_sg_attrs+0x181/0x1f0 kernel/dma/mapping.c:188
> > Code: 00 00 00 00 00 fc ff df 48 c1 e8 03 80 3c 10 00 75 71 4c 8b 3d c0 83 
> > b5 0d e9 db fe ff ff e8 b6 0f 13 00 0f 0b e8 af 0f 13 00 <0f> 0b 45 31 e4 
> > e9 54 ff ff ff e8 a0 0f 13 00 49 8d 7f 50 48 b8 00
> > RSP: 0018:c90002a07d68 EFLAGS: 00010293
> > RAX:  RBX:  RCX: 
> > RDX: 88807e25e2c0 RSI: 81649e91 RDI: 88801b848408
> > RBP: 88801b848000 R08: 0002 R09: 88801d86c74f
> > R10: 81649d72 R11: 0001 R12: 0002
> > R13: 88801d86c680 R14: 0001 R15: 
> > FS:  56e30300() GS:8880b9d0() knlGS:
> > CS:  0010 DS:  ES:  CR0: 80050033
> > CR2: 20cc CR3: 1d74a000 CR4: 003506e0
> > DR0:  DR1:  DR2: 
> > DR3:  DR6: fffe0ff0 DR7: 0400
> > Call Trace:
> >  
> >  dma_map_sgtable+0x70/0xf0 kernel/dma/mapping.c:264
> >  get_sg_table.isra.0+0xe0/0x160 drivers/dma-buf/udmabuf.c:72
> >  begin_cpu_udmabuf+0x130/0x1d0 drivers/dma-buf/udmabuf.c:126
> >  dma_buf_begin_cpu_access+0xfd/0x1d0 drivers/dma-buf/dma-buf.c:1164
> >  dma_buf_ioctl+0x259/0x2b0 drivers/dma-buf/dma-buf.c:363
> >  vfs_ioctl fs/ioctl.c:51 [inline]
> >  __do_sys_ioctl fs/ioctl.c:874 [inline]
> >  __se_sys_ioctl fs/ioctl.c:860 [inline]
> >  __x64_sys_ioctl+0x193/0x200 fs/ioctl.c:860
> >  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >  do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> >  entry_SYSCALL_64_after_hwframe+0x44/0xae
> > RIP: 0033:0x7f62fcf530f9
> > Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 
> > 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff 
> > ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> > RSP: 002b:7ffe3edab9b8 EFLAGS: 0246 ORIG_RAX: 0010
> > RAX: ffda RBX:  RCX: 7f62fcf530f9
> > RDX: 2200 RSI: 40086200 RDI: 0006
> > RBP: 7f62fcf170e0 R08:  R09: 
> > R10:  R11: 0246 R12: 7f62fcf17170
> > R13:  R14:  R15: 
> >  
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
> --
> You received this message because you are subscribed to the Google Groups 
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/syzkaller-bugs/YgJhjdAbRHdnCZ4T%40phenom.ffwll.local.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH RFC 10/12] iommufd: Add kAPI toward external drivers

2022-05-19 Thread Jason Gunthorpe via iommu
On Thu, May 19, 2022 at 05:45:06PM +0800, Yi Liu wrote:
> Hi Jason,
> 
> On 2022/3/19 01:27, Jason Gunthorpe wrote:
> 
> > +/**
> > + * iommufd_device_attach - Connect a device to an iommu_domain
> > + * @idev: device to attach
> > + * @pt_id: Input a IOMMUFD_OBJ_IOAS, or IOMMUFD_OBJ_HW_PAGETABLE
> > + * Output the IOMMUFD_OBJ_HW_PAGETABLE ID
> > + * @flags: Optional flags
> > + *
> > + * This connects the device to an iommu_domain, either automatically or 
> > manually
> > + * selected. Once this completes the device could do DMA.
> > + *
> > + * The caller should return the resulting pt_id back to userspace.
> > + * This function is undone by calling iommufd_device_detach().
> > + */
> > +int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id,
> > + unsigned int flags)
> > +{

> Just double check here.
> This API doesn't prevent caller from calling this API multiple times with
> the same @idev and @pt_id. right? Note that idev has only one device_item
> list head. If caller does do multiple callings, then there should be
> problem. right? If so, this API assumes caller should take care of it and
> not do such bad function call. Is this the design here?

Yes, caller must ensure strict pairing, we don't have an assertion to
check it.

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


Re: [PATCH v7 02/10] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:39PM +0800, Lu Baolu wrote:
> The current kernel DMA with PASID support is based on the SVA with a flag
> SVM_FLAG_SUPERVISOR_MODE. The IOMMU driver binds the kernel memory address
> space to a PASID of the device. The device driver programs the device with
> kernel virtual address (KVA) for DMA access. There have been security and
> functional issues with this approach:
> 
> - The lack of IOTLB synchronization upon kernel page table updates.
>   (vmalloc, module/BPF loading, CONFIG_DEBUG_PAGEALLOC etc.)
> - Other than slight more protection, using kernel virtual address (KVA)
>   has little advantage over physical address. There are also no use
>   cases yet where DMA engines need kernel virtual addresses for in-kernel
>   DMA.
> 
> This removes SVM_FLAG_SUPERVISOR_MODE support from the IOMMU interface.
> The device drivers are suggested to handle kernel DMA with PASID through
> the kernel DMA APIs.
> 
> The drvdata parameter in iommu_sva_bind_device() and all callbacks is not
> needed anymore. Cleanup them as well.
> 
> Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
> Signed-off-by: Jacob Pan 
> Signed-off-by: Lu Baolu 
> Reviewed-by: Jason Gunthorpe 

For the SMMU bits

Reviewed-by: Jean-Philippe Brucker 

> ---
>  include/linux/intel-iommu.h   |  3 +-
>  include/linux/intel-svm.h | 13 -
>  include/linux/iommu.h |  8 +--
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  5 +-
>  drivers/dma/idxd/cdev.c   |  2 +-
>  drivers/dma/idxd/init.c   | 24 +---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  3 +-

>  drivers/iommu/intel/svm.c | 57 +--
>  drivers/iommu/iommu.c |  5 +-
>  drivers/misc/uacce/uacce.c|  2 +-
>  10 files changed, 26 insertions(+), 96 deletions(-)
> 
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 4f29139bbfc3..df23300cfa88 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -739,8 +739,7 @@ struct intel_iommu *device_to_iommu(struct device *dev, 
> u8 *bus, u8 *devfn);
>  extern void intel_svm_check(struct intel_iommu *iommu);
>  extern int intel_svm_enable_prq(struct intel_iommu *iommu);
>  extern int intel_svm_finish_prq(struct intel_iommu *iommu);
> -struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
> -  void *drvdata);
> +struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm);
>  void intel_svm_unbind(struct iommu_sva *handle);
>  u32 intel_svm_get_pasid(struct iommu_sva *handle);
>  int intel_svm_page_response(struct device *dev, struct iommu_fault_event 
> *evt,
> diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
> index 207ef06ba3e1..f9a0d44f6fdb 100644
> --- a/include/linux/intel-svm.h
> +++ b/include/linux/intel-svm.h
> @@ -13,17 +13,4 @@
>  #define PRQ_RING_MASK((0x1000 << PRQ_ORDER) - 0x20)
>  #define PRQ_DEPTH((0x1000 << PRQ_ORDER) >> 5)
>  
> -/*
> - * The SVM_FLAG_SUPERVISOR_MODE flag requests a PASID which can be used only
> - * for access to kernel addresses. No IOTLB flushes are automatically done
> - * for kernel mappings; it is valid only for access to the kernel's static
> - * 1:1 mapping of physical memory — not to vmalloc or even module mappings.
> - * A future API addition may permit the use of such ranges, by means of an
> - * explicit IOTLB flush call (akin to the DMA API's unmap method).
> - *
> - * It is unlikely that we will ever hook into flush_tlb_kernel_range() to
> - * do such IOTLB flushes automatically.
> - */
> -#define SVM_FLAG_SUPERVISOR_MODE BIT(0)
> -
>  #endif /* __INTEL_SVM_H__ */
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index da423e87f248..0c358b7c583b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -243,8 +243,7 @@ struct iommu_ops {
>   int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
>   int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
>  
> - struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm,
> -   void *drvdata);
> + struct iommu_sva *(*sva_bind)(struct device *dev, struct mm_struct *mm);
>   void (*sva_unbind)(struct iommu_sva *handle);
>   u32 (*sva_get_pasid)(struct iommu_sva *handle);
>  
> @@ -667,8 +666,7 @@ int iommu_dev_disable_feature(struct device *dev, enum 
> iommu_dev_features f);
>  bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features 
> f);
>  
>  struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> - struct mm_struct *mm,
> - void *drvdata);
> + struct mm_struct *mm);
>  void iommu_sva_unbind_device(struct

Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:40PM +0800, Lu Baolu wrote:
> The iommu_sva_domain represents a hardware pagetable that the IOMMU
> hardware could use for SVA translation. This adds some infrastructure
> to support SVA domain in the iommu common layer. It includes:
> 
> - Add a new struct iommu_sva_domain and new IOMMU_DOMAIN_SVA domain
>   type.
> - Add a new domain ops pointer in iommu_ops. The IOMMU drivers that
>   support SVA should provide the callbacks.
> - Add helpers to allocate and free an SVA domain.
> - Add helpers to set an SVA domain to a device and the reverse
>   operation.
> 
> Some buses, like PCI, route packets without considering the PASID value.
> Thus a DMA target address with PASID might be treated as P2P if the
> address falls into the MMIO BAR of other devices in the group. To make
> things simple, the attach/detach interfaces only apply to devices
> belonging to the singleton groups, and the singleton is immutable in
> fabric i.e. not affected by hotplug.
> 
> The iommu_set/block_device_pasid() can be used for other purposes,
> such as kernel DMA with pasid, mediation device, etc. Hence, it is put
> in the iommu.c.
> 
> Suggested-by: Jean-Philippe Brucker 
> Suggested-by: Jason Gunthorpe 
> Signed-off-by: Lu Baolu 
> ---
>  include/linux/iommu.h | 51 +
>  drivers/iommu/iommu-sva-lib.h | 15 
>  drivers/iommu/iommu-sva-lib.c | 48 +++
>  drivers/iommu/iommu.c | 71 +++
>  4 files changed, 185 insertions(+)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 0c358b7c583b..e8cf82d46ce1 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -64,6 +64,9 @@ struct iommu_domain_geometry {
>  #define __IOMMU_DOMAIN_PT(1U << 2)  /* Domain is identity mapped   */
>  #define __IOMMU_DOMAIN_DMA_FQ(1U << 3)  /* DMA-API uses flush queue  
>   */
>  
> +#define __IOMMU_DOMAIN_SHARED(1U << 4)  /* Page table shared from 
> CPU  */
> +#define __IOMMU_DOMAIN_HOST_VA   (1U << 5)  /* Host CPU virtual address 
> */
> +
>  /*
>   * This are the possible domain-types
>   *
> @@ -86,6 +89,8 @@ struct iommu_domain_geometry {
>  #define IOMMU_DOMAIN_DMA_FQ  (__IOMMU_DOMAIN_PAGING |\
>__IOMMU_DOMAIN_DMA_API |   \
>__IOMMU_DOMAIN_DMA_FQ)
> +#define IOMMU_DOMAIN_SVA (__IOMMU_DOMAIN_SHARED |\
> +  __IOMMU_DOMAIN_HOST_VA)
>  
>  struct iommu_domain {
>   unsigned type;
> @@ -254,6 +259,7 @@ struct iommu_ops {
>   int (*def_domain_type)(struct device *dev);
>  
>   const struct iommu_domain_ops *default_domain_ops;
> + const struct iommu_domain_ops *sva_domain_ops;
>   unsigned long pgsize_bitmap;
>   struct module *owner;
>  };
> @@ -262,6 +268,8 @@ struct iommu_ops {
>   * struct iommu_domain_ops - domain specific operations
>   * @attach_dev: attach an iommu domain to a device
>   * @detach_dev: detach an iommu domain from a device
> + * @set_dev_pasid: set an iommu domain to a pasid of device
> + * @block_dev_pasid: block pasid of device from using iommu domain
>   * @map: map a physically contiguous memory region to an iommu domain
>   * @map_pages: map a physically contiguous set of pages of the same size to
>   * an iommu domain.
> @@ -282,6 +290,10 @@ struct iommu_ops {
>  struct iommu_domain_ops {
>   int (*attach_dev)(struct iommu_domain *domain, struct device *dev);
>   void (*detach_dev)(struct iommu_domain *domain, struct device *dev);
> + int (*set_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> +  ioasid_t pasid);
> + void (*block_dev_pasid)(struct iommu_domain *domain, struct device *dev,
> + ioasid_t pasid);
>  
>   int (*map)(struct iommu_domain *domain, unsigned long iova,
>  phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
> @@ -677,6 +689,10 @@ int iommu_group_claim_dma_owner(struct iommu_group 
> *group, void *owner);
>  void iommu_group_release_dma_owner(struct iommu_group *group);
>  bool iommu_group_dma_owner_claimed(struct iommu_group *group);
>  
> +int iommu_set_device_pasid(struct iommu_domain *domain, struct device *dev,
> +ioasid_t pasid);
> +void iommu_block_device_pasid(struct iommu_domain *domain, struct device 
> *dev,
> +   ioasid_t pasid);
>  #else /* CONFIG_IOMMU_API */
>  
>  struct iommu_ops {};
> @@ -1050,6 +1066,17 @@ static inline bool 
> iommu_group_dma_owner_claimed(struct iommu_group *group)
>  {
>   return false;
>  }
> +
> +static inline int iommu_set_device_pasid(struct iommu_domain *domain,
> +  struct device *dev, ioasid_t pasid)
> +{
> + return -ENODEV;
> +}
> +
> +static inline void iommu_block_device_pasid(struct iommu_domain *domain,
> + 

Re: [PATCH v7 05/10] arm-smmu-v3/sva: Add SVA domain support

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:42PM +0800, Lu Baolu wrote:
> Add support for domain ops callbacks for an SVA domain.
> 
> Signed-off-by: Lu Baolu 

Reviewed-by: Jean-Philippe Brucker 

(I'll try to take some time next cycle to clean up the driver following
this change)

> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  4 ++
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 46 +++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  6 +++
>  3 files changed, 56 insertions(+)
> 
> 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 d2ba86470c42..ec77f6a51ff9 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -758,6 +758,10 @@ struct iommu_sva *arm_smmu_sva_bind(struct device *dev, 
> struct mm_struct *mm);
>  void arm_smmu_sva_unbind(struct iommu_sva *handle);
>  u32 arm_smmu_sva_get_pasid(struct iommu_sva *handle);
>  void arm_smmu_sva_notifier_synchronize(void);
> +int arm_smmu_sva_attach_dev_pasid(struct iommu_domain *domain,
> +   struct device *dev, ioasid_t id);
> +void arm_smmu_sva_detach_dev_pasid(struct iommu_domain *domain,
> +struct device *dev, ioasid_t id);
>  #else /* CONFIG_ARM_SMMU_V3_SVA */
>  static inline bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
>  {
> 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 f155d406c5d5..6969974ca89e 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
> @@ -549,3 +549,49 @@ void arm_smmu_sva_notifier_synchronize(void)
>*/
>   mmu_notifier_synchronize();
>  }
> +
> +int arm_smmu_sva_attach_dev_pasid(struct iommu_domain *domain,
> +   struct device *dev, ioasid_t id)
> +{
> + int ret = 0;
> + struct mm_struct *mm;
> + struct iommu_sva *handle;
> +
> + if (domain->type != IOMMU_DOMAIN_SVA)
> + return -EINVAL;
> +
> + mm = domain_to_mm(domain);
> + if (WARN_ON(!mm))
> + return -ENODEV;
> +
> + mutex_lock(&sva_lock);
> + handle = __arm_smmu_sva_bind(dev, mm);
> + if (IS_ERR(handle))
> + ret = PTR_ERR(handle);
> + mutex_unlock(&sva_lock);
> +
> + return ret;
> +}
> +
> +void arm_smmu_sva_detach_dev_pasid(struct iommu_domain *domain,
> +struct device *dev, ioasid_t id)
> +{
> + struct arm_smmu_bond *bond = NULL, *t;
> + struct mm_struct *mm = domain_to_mm(domain);
> + struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> +
> + mutex_lock(&sva_lock);
> + list_for_each_entry(t, &master->bonds, list) {
> + if (t->mm == mm) {
> + bond = t;
> + break;
> + }
> + }
> +
> + if (!WARN_ON(!bond) && refcount_dec_and_test(&bond->refs)) {
> + list_del(&bond->list);
> + arm_smmu_mmu_notifier_put(bond->smmu_mn);
> + kfree(bond);
> + }
> + mutex_unlock(&sva_lock);
> +}
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c 
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 6e2cd082c670..4ad3ca70cf89 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2858,6 +2858,12 @@ static struct iommu_ops arm_smmu_ops = {
>   .page_response  = arm_smmu_page_response,
>   .pgsize_bitmap  = -1UL, /* Restricted during device attach */
>   .owner  = THIS_MODULE,
> +#ifdef CONFIG_ARM_SMMU_V3_SVA
> + .sva_domain_ops = &(const struct iommu_domain_ops) {
> + .set_dev_pasid  = arm_smmu_sva_attach_dev_pasid,
> + .block_dev_pasid= arm_smmu_sva_detach_dev_pasid,
> + },
> +#endif
>   .default_domain_ops = &(const struct iommu_domain_ops) {
>   .attach_dev = arm_smmu_attach_dev,
>   .map_pages  = arm_smmu_map_pages,
> -- 
> 2.25.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:43PM +0800, Lu Baolu wrote:
> The existing iommu SVA interfaces are implemented by calling the SVA
> specific iommu ops provided by the IOMMU drivers. There's no need for
> any SVA specific ops in iommu_ops vector anymore as we can achieve
> this through the generic attach/detach_dev_pasid domain ops.
> 
> This refactors the IOMMU SVA interfaces implementation by using the
> set/block_pasid_dev ops and align them with the concept of the SVA
> iommu domain. Put the new SVA code in the sva related file in order
> to make it self-contained.
> 
> Signed-off-by: Lu Baolu 
> ---
>  include/linux/iommu.h |  48 --
>  drivers/iommu/iommu-sva-lib.h |   1 +
>  drivers/iommu/iommu-sva-lib.c | 113 
>  drivers/iommu/iommu.c | 119 --
>  4 files changed, 170 insertions(+), 111 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e8cf82d46ce1..d9ac5ebe5bbb 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -635,6 +635,7 @@ struct iommu_fwspec {
>   */
>  struct iommu_sva {
>   struct device   *dev;
> + refcount_t  users;
>  };
>  
>  int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
> @@ -677,11 +678,6 @@ int iommu_dev_enable_feature(struct device *dev, enum 
> iommu_dev_features f);
>  int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
>  bool iommu_dev_feature_enabled(struct device *dev, enum iommu_dev_features 
> f);
>  
> -struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> - struct mm_struct *mm);
> -void iommu_sva_unbind_device(struct iommu_sva *handle);
> -u32 iommu_sva_get_pasid(struct iommu_sva *handle);
> -
>  int iommu_device_use_default_domain(struct device *dev);
>  void iommu_device_unuse_default_domain(struct device *dev);
>  
> @@ -693,6 +689,8 @@ int iommu_set_device_pasid(struct iommu_domain *domain, 
> struct device *dev,
>  ioasid_t pasid);
>  void iommu_block_device_pasid(struct iommu_domain *domain, struct device 
> *dev,
> ioasid_t pasid);
> +struct iommu_domain *
> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid);
>  #else /* CONFIG_IOMMU_API */
>  
>  struct iommu_ops {};
> @@ -1023,21 +1021,6 @@ iommu_dev_disable_feature(struct device *dev, enum 
> iommu_dev_features feat)
>   return -ENODEV;
>  }
>  
> -static inline struct iommu_sva *
> -iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
> -{
> - return NULL;
> -}
> -
> -static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
> -{
> -}
> -
> -static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> -{
> - return IOMMU_PASID_INVALID;
> -}
> -
>  static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
>  {
>   return NULL;
> @@ -1077,6 +1060,12 @@ static inline void iommu_block_device_pasid(struct 
> iommu_domain *domain,
>   struct device *dev, ioasid_t pasid)
>  {
>  }
> +
> +static inline struct iommu_domain *
> +iommu_get_domain_for_dev_pasid(struct device *dev, ioasid_t pasid)
> +{
> + return NULL;
> +}
>  #endif /* CONFIG_IOMMU_API */
>  
>  /**
> @@ -1108,6 +1097,10 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
> mm_struct *mm);
>  void iommu_sva_free_domain(struct iommu_domain *domain);
>  int iommu_sva_set_domain(struct iommu_domain *domain, struct device *dev,
>ioasid_t pasid);
> +struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> + struct mm_struct *mm);
> +void iommu_sva_unbind_device(struct iommu_sva *handle);
> +u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>  #else /* CONFIG_IOMMU_SVA */
>  static inline struct iommu_domain *
>  iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm)
> @@ -1124,6 +1117,21 @@ static inline int iommu_sva_set_domain(struct 
> iommu_domain *domain,
>  {
>   return -EINVAL;
>  }
> +
> +static inline struct iommu_sva *
> +iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
> +{
> + return NULL;
> +}
> +
> +static inline void iommu_sva_unbind_device(struct iommu_sva *handle)
> +{
> +}
> +
> +static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
> +{
> + return IOMMU_PASID_INVALID;
> +}
>  #endif /* CONFIG_IOMMU_SVA */
>  
>  #endif /* __LINUX_IOMMU_H */
> diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
> index 1be21e6b93ec..ebab5a8cb126 100644
> --- a/drivers/iommu/iommu-sva-lib.h
> +++ b/drivers/iommu/iommu-sva-lib.h
> @@ -20,6 +20,7 @@ struct iopf_queue;
>  struct iommu_sva_domain {
>   struct iommu_domain domain;
>   struct mm_struct*mm;
> + struct iommu_svabond;
>  };
>  
>  #define to_sva_domain(d) container_of_safe(d,

Re: [PATCH v7 08/10] iommu: Prepare IOMMU domain for IOPF

2022-05-19 Thread Jean-Philippe Brucker
On Thu, May 19, 2022 at 03:20:45PM +0800, Lu Baolu wrote:
> This adds some mechanisms around the iommu_domain so that the I/O page
> fault handling framework could route a page fault to the domain and
> call the fault handler from it.
> 
> Add pointers to the page fault handler and its private data in struct
> iommu_domain. The fault handler will be called with the private data
> as a parameter once a page fault is routed to the domain. Any kernel
> component which owns an iommu domain could install handler and its
> private parameter so that the page fault could be further routed and
> handled.
> 
> This also prepares the SVA implementation to be the first consumer of
> the per-domain page fault handling model.
> 
> Suggested-by: Jean-Philippe Brucker 
> Signed-off-by: Lu Baolu 
> ---
>  include/linux/iommu.h |  3 ++
>  drivers/iommu/io-pgfault.c|  7 
>  drivers/iommu/iommu-sva-lib.c | 65 +++
>  3 files changed, 75 insertions(+)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index e4ce2fe0e144..45f274b2640d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -100,6 +100,9 @@ struct iommu_domain {
>   void *handler_token;
>   struct iommu_domain_geometry geometry;
>   struct iommu_dma_cookie *iova_cookie;
> + enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
> +   void *data);
> + void *fault_data;
>  };
>  
>  static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> index 1df8c1dcae77..aee9e033012f 100644
> --- a/drivers/iommu/io-pgfault.c
> +++ b/drivers/iommu/io-pgfault.c
> @@ -181,6 +181,13 @@ static void iopf_handle_group(struct work_struct *work)
>   * request completes, outstanding faults will have been dealt with by the 
> time
>   * the PASID is freed.
>   *
> + * Any valid page fault will be eventually routed to an iommu domain and the
> + * page fault handler installed there will get called. The users of this
> + * handling framework should guarantee that the iommu domain could only be
> + * freed after the device has stopped generating page faults (or the iommu
> + * hardware has been set to block the page faults) and the pending page 
> faults
> + * have been flushed.
> + *
>   * Return: 0 on success and <0 on error.
>   */
>  int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
> diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c
> index 568e0f64edac..317ab8e8c149 100644
> --- a/drivers/iommu/iommu-sva-lib.c
> +++ b/drivers/iommu/iommu-sva-lib.c
> @@ -72,6 +72,69 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid)
>  }
>  EXPORT_SYMBOL_GPL(iommu_sva_find);
>  
> +/*
> + * I/O page fault handler for SVA
> + *
> + * Copied from io-pgfault.c with mmget_not_zero() added before
> + * mmap_read_lock().

Comment doesn't really belong here, maybe better in the commit message.
Apart from that

Reviewed-by: Jean-Philippe Brucker 

> + */
> +static enum iommu_page_response_code
> +iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
> +{
> + vm_fault_t ret;
> + struct mm_struct *mm;
> + struct vm_area_struct *vma;
> + unsigned int access_flags = 0;
> + struct iommu_domain *domain = data;
> + unsigned int fault_flags = FAULT_FLAG_REMOTE;
> + struct iommu_fault_page_request *prm = &fault->prm;
> + enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
> +
> + if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
> + return status;
> +
> + mm = domain_to_mm(domain);
> + if (IS_ERR_OR_NULL(mm) || !mmget_not_zero(mm))
> + return status;
> +
> + mmap_read_lock(mm);
> +
> + vma = find_extend_vma(mm, prm->addr);
> + if (!vma)
> + /* Unmapped area */
> + goto out_put_mm;
> +
> + if (prm->perm & IOMMU_FAULT_PERM_READ)
> + access_flags |= VM_READ;
> +
> + if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
> + access_flags |= VM_WRITE;
> + fault_flags |= FAULT_FLAG_WRITE;
> + }
> +
> + if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
> + access_flags |= VM_EXEC;
> + fault_flags |= FAULT_FLAG_INSTRUCTION;
> + }
> +
> + if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
> + fault_flags |= FAULT_FLAG_USER;
> +
> + if (access_flags & ~vma->vm_flags)
> + /* Access fault */
> + goto out_put_mm;
> +
> + ret = handle_mm_fault(vma, prm->addr, fault_flags, NULL);
> + status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
> + IOMMU_PAGE_RESP_SUCCESS;
> +
> +out_put_mm:
> + mmap_read_unlock(mm);
> + mmput(mm);
> +
> + return status;
> +}
> +
>  /*
>   * IOMMU SVA driver-oriented interfaces
>   */
> @@ -94,6 +157,8 @@ iommu_sva_alloc_domain(struct bus_type *bus, struct 
> mm_struct *mm

Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-19 Thread Alex Williamson
On Thu, 19 May 2022 09:32:05 +0200
Joerg Roedel  wrote:

> On Wed, May 18, 2022 at 04:14:46PM -0300, Jason Gunthorpe wrote:
> > Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must always 
> > assign a domain")
> > Reported-by: Eric Farman 
> > Signed-off-by: Jason Gunthorpe 
> > ---
> >  drivers/vfio/vfio.c | 15 ++-
> >  1 file changed, 10 insertions(+), 5 deletions(-)  
> 
> That fix will be taken care of by Alex? Or does it need to go through
> the IOMMU tree?

The comment suggested my tree.  Jason, were you planning to post this
on its own instead of buried in a reply or shall we take it from here?
Thanks,

Alex

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


Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-19 Thread Jason Gunthorpe via iommu
On Thu, May 19, 2022 at 10:51:47AM -0600, Alex Williamson wrote:
> On Thu, 19 May 2022 09:32:05 +0200
> Joerg Roedel  wrote:
> 
> > On Wed, May 18, 2022 at 04:14:46PM -0300, Jason Gunthorpe wrote:
> > > Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must always 
> > > assign a domain")
> > > Reported-by: Eric Farman 
> > > Signed-off-by: Jason Gunthorpe 
> > >  drivers/vfio/vfio.c | 15 ++-
> > >  1 file changed, 10 insertions(+), 5 deletions(-)  
> > 
> > That fix will be taken care of by Alex? Or does it need to go through
> > the IOMMU tree?
> 
> The comment suggested my tree.  Jason, were you planning to post this
> on its own instead of buried in a reply or shall we take it from
> here?

Let me repost it with a proper cc list, I think your tree is best.

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


[PATCH] vfio: Do not manipulate iommu dma_owner for fake iommu groups

2022-05-19 Thread Jason Gunthorpe via iommu
Since asserting dma ownership now causes the group to have its DMA blocked
the iommu layer requires a working iommu. This means the dma_owner APIs
cannot be used on the fake groups that VFIO creates. Test for this and
avoid calling them.

Otherwise asserting dma ownership will fail for VFIO mdev devices as a
BLOCKING iommu_domain cannot be allocated due to the NULL iommu ops.

Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must always assign a 
domain")
Reported-by: Eric Farman 
Tested-by: Eric Farman 
Signed-off-by: Jason Gunthorpe 
---
 drivers/vfio/vfio.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

Sort of a v2 from here:

https://lore.kernel.org/all/20220518191446.gu1343...@nvidia.com/

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index cfcff7764403fc..f5ed03897210c3 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -927,7 +927,8 @@ static void __vfio_group_unset_container(struct vfio_group 
*group)
driver->ops->detach_group(container->iommu_data,
  group->iommu_group);
 
-   iommu_group_release_dma_owner(group->iommu_group);
+   if (group->type == VFIO_IOMMU)
+   iommu_group_release_dma_owner(group->iommu_group);
 
group->container = NULL;
group->container_users = 0;
@@ -1001,9 +1002,11 @@ static int vfio_group_set_container(struct vfio_group 
*group, int container_fd)
goto unlock_out;
}
 
-   ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
-   if (ret)
-   goto unlock_out;
+   if (group->type == VFIO_IOMMU) {
+   ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
+   if (ret)
+   goto unlock_out;
+   }
 
driver = container->iommu_driver;
if (driver) {
@@ -1011,7 +1014,9 @@ static int vfio_group_set_container(struct vfio_group 
*group, int container_fd)
group->iommu_group,
group->type);
if (ret) {
-   iommu_group_release_dma_owner(group->iommu_group);
+   if (group->type == VFIO_IOMMU)
+   iommu_group_release_dma_owner(
+   group->iommu_group);
goto unlock_out;
}
}

base-commit: 7ab5e10eda02da1d9562ffde562c51055d368e9c
-- 
2.36.0

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


[PATCH] iommu/s390: tolerate repeat attach_dev calls

2022-05-19 Thread Matthew Rosato
Since commit 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must
always assign a domain") s390-iommu will get called to allocate multiple
unmanaged iommu domains for a vfio-pci device -- however the current
s390-iommu logic tolerates only one.  Recognize that multiple domains can
be allocated and handle switching between DMA or different iommu domain
tables during attach_dev.

Signed-off-by: Matthew Rosato 
---
 drivers/iommu/s390-iommu.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
index 3833e86c6e7b..c898bcbbce11 100644
--- a/drivers/iommu/s390-iommu.c
+++ b/drivers/iommu/s390-iommu.c
@@ -99,7 +99,7 @@ static int s390_iommu_attach_device(struct iommu_domain 
*domain,
if (!domain_device)
return -ENOMEM;
 
-   if (zdev->dma_table) {
+   if (zdev->dma_table && !zdev->s390_domain) {
cc = zpci_dma_exit_device(zdev);
if (cc) {
rc = -EIO;
@@ -107,6 +107,9 @@ static int s390_iommu_attach_device(struct iommu_domain 
*domain,
}
}
 
+   if (zdev->s390_domain)
+   zpci_unregister_ioat(zdev, 0);
+
zdev->dma_table = s390_domain->dma_table;
cc = zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma,
virt_to_phys(zdev->dma_table));
@@ -136,7 +139,13 @@ static int s390_iommu_attach_device(struct iommu_domain 
*domain,
return 0;
 
 out_restore:
-   zpci_dma_init_device(zdev);
+   if (!zdev->s390_domain) {
+   zpci_dma_init_device(zdev);
+   } else {
+   zdev->dma_table = zdev->s390_domain->dma_table;
+   zpci_register_ioat(zdev, 0, zdev->start_dma, zdev->end_dma,
+  virt_to_phys(zdev->dma_table));
+   }
 out_free:
kfree(domain_device);
 
@@ -167,7 +176,7 @@ static void s390_iommu_detach_device(struct iommu_domain 
*domain,
}
spin_unlock_irqrestore(&s390_domain->list_lock, flags);
 
-   if (found) {
+   if (found && (zdev->s390_domain == s390_domain)) {
zdev->s390_domain = NULL;
zpci_unregister_ioat(zdev, 0);
zpci_dma_init_device(zdev);
-- 
2.27.0

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


Re: [PATCH] iommu/s390: tolerate repeat attach_dev calls

2022-05-19 Thread Matthew Rosato

On 5/19/22 2:29 PM, Matthew Rosato wrote:

Since commit 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must
always assign a domain") s390-iommu will get called to allocate multiple
unmanaged iommu domains for a vfio-pci device -- however the current
s390-iommu logic tolerates only one.  Recognize that multiple domains can
be allocated and handle switching between DMA or different iommu domain
tables during attach_dev.

Signed-off-by: Matthew Rosato 
---


Tested in conjuction with

https://lore.kernel.org/kvm/0-v1-9cfc47edbcd4+13546-vfio_dma_owner_fix_...@nvidia.com/

Along with that patch, vfio{-pci,-ap,-ccw} on s390x for -next seem happy 
again.


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


Re: [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain

2022-05-19 Thread Jacob Pan
Hi Baolu,

On Thu, 19 May 2022 14:41:06 +0800, Baolu Lu 
wrote:

> > IOMMU group maintains a PASID array which stores the associated IOMMU
> > domains. This patch introduces a helper function to do domain to PASID
> > look up. It will be used by TLB flush and device-PASID attach
> > verification.  
> 
> Do you really need this?
> 
> The IOMMU driver has maintained a device tracking list for each domain.
> It has been used for cache invalidation when unmap() is called against
> dma domain.
Yes, I am aware of the device list. In v3, I stored DMA API PASID in device
list of device_domain_info. Since we already have a pasid_array, Jason
suggested to share the storage with the code. This helper is needed to
reverse look up the DMA PASID based on the domain attached.
Discussions here:
https://lore.kernel.org/lkml/20220511170025.gf49...@nvidia.com/t/#mf7cb7d54d89e6e732a020dc22435260da0a49580

Thanks,

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


Re: [PATCH v3 1/4] iommu/vt-d: Implement domain ops for attach_dev_pasid

2022-05-19 Thread Jacob Pan
Hi Jason,

On Wed, 18 May 2022 15:52:05 -0300, Jason Gunthorpe  wrote:

> On Wed, May 18, 2022 at 11:42:04AM -0700, Jacob Pan wrote:
> 
> > > Yes.. It seems inefficient to iterate over that xarray multiple times
> > > on the flush hot path, but maybe there is little choice. Try to use
> > > use the xas iterators under the xa_lock spinlock..
> > >   
> > xas_for_each takes a max range, here we don't really have one. So I
> > posted v4 w/o using the xas advanced API. Please let me know if you have
> > suggestions.  
> 
> You are supposed to use ULONG_MAX in cases like that.
> 
got it.
> > xa_for_each takes RCU read lock, it should be fast for tlb flush,
> > right? The worst case maybe over flush when we have stale data but
> > should be very rare.  
> 
> Not really, xa_for_each walks the tree for every iteration, it is
> slower than a linked list walk in any cases where the xarray is
> multi-node. xas_for_each is able to retain a pointer where it is in
> the tree so each iteration is usually just a pointer increment.
> 
Thanks for explaining, yeah if we have to iterate multiple times
xas_for_each() is better.

> The downside is you cannot sleep while doing xas_for_each
> 
will do under RCU read lock

> > > The challenge will be accessing the group xa in the first place, but
> > > maybe the core code can gain a function call to return a pointer to
> > > that XA or something..  
>  
> > I added a helper function to find the matching DMA API PASID in v4.  
> 
> Again, why are we focused on DMA API? Nothing you build here should be
> DMA API beyond the fact that the iommu_domain being attached is the
> default domain.
The helper is not DMA API specific. Just a domain-PASID look up. Sorry for
the confusion.

Thanks,

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


Re: [PATCH] iommu/s390: tolerate repeat attach_dev calls

2022-05-19 Thread Jason Gunthorpe via iommu
On Thu, May 19, 2022 at 02:29:29PM -0400, Matthew Rosato wrote:
> Since commit 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must
> always assign a domain") s390-iommu will get called to allocate multiple
> unmanaged iommu domains for a vfio-pci device -- however the current
> s390-iommu logic tolerates only one.  Recognize that multiple domains can
> be allocated and handle switching between DMA or different iommu domain
> tables during attach_dev.
> 
> Signed-off-by: Matthew Rosato 
> ---
>  drivers/iommu/s390-iommu.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Makes senese, thanks

Reviewed-by: Jason Gunthorpe 

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


Re: [PATCH v6 20/29] init/main: Delay initialization of the lockup detector after smp_init()

2022-05-19 Thread Nicholas Piggin
Excerpts from Ricardo Neri's message of May 14, 2022 9:16 am:
> On Tue, May 10, 2022 at 08:38:22PM +1000, Nicholas Piggin wrote:
>> Excerpts from Ricardo Neri's message of May 6, 2022 9:59 am:
>> > Certain implementations of the hardlockup detector require support for
>> > Inter-Processor Interrupt shorthands. On x86, support for these can only
>> > be determined after all the possible CPUs have booted once (in
>> > smp_init()). Other architectures may not need such check.
>> > 
>> > lockup_detector_init() only performs the initializations of data
>> > structures of the lockup detector. Hence, there are no dependencies on
>> > smp_init().
>> 
> 
> Thank you for your feedback Nicholas!
> 
>> I think this is the only real thing which affects other watchdog types?
> 
> Also patches 18 and 19 that decouple the NMI watchdog functionality from
> perf.
> 
>> 
>> Not sure if it's a big problem, the secondary CPUs coming up won't
>> have their watchdog active until quite late, and the primary could
>> implement its own timeout in __cpu_up for secondary coming up, and
>> IPI it to get traces if necessary which is probably more robust.
> 
> Indeed that could work. Another alternative I have been pondering is to boot
> the system with the perf-based NMI watchdog enabled. Once all CPUs are up
> and running, switch to the HPET-based NMI watchdog and free the PMU counters.

Just to cover smp_init()? Unless you could move the watchdog 
significantly earlier, I'd say it's probably not worth bothering
with.

Yes the boot CPU is doing *some* work that could lock up, but most 
complexity is in the secondaries coming up and they won't have their own 
watchdog coverage for a good chunk of that anyway.

If anything I would just add some timeout warning or IPI or something in
those wait loops in x86's __cpu_up code if you are worried about 
catching issues here. Actually the watchdog probably wouldn't catch any
of those anyway because they either run with interrupts enabled or
touch_nmi_watchdog()! So yeah that'd be pretty pointless.

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


RE: [PATCH] vfio: Do not manipulate iommu dma_owner for fake iommu groups

2022-05-19 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Friday, May 20, 2022 1:04 AM
> 
> Since asserting dma ownership now causes the group to have its DMA
> blocked
> the iommu layer requires a working iommu. This means the dma_owner APIs
> cannot be used on the fake groups that VFIO creates. Test for this and
> avoid calling them.
> 
> Otherwise asserting dma ownership will fail for VFIO mdev devices as a
> BLOCKING iommu_domain cannot be allocated due to the NULL iommu ops.
> 
> Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must
> always assign a domain")
> Reported-by: Eric Farman 
> Tested-by: Eric Farman 
> Signed-off-by: Jason Gunthorpe 

Reviewed-by: Kevin Tian 

> ---
>  drivers/vfio/vfio.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> Sort of a v2 from here:
> 
> https://lore.kernel.org/all/20220518191446.gu1343...@nvidia.com/
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index cfcff7764403fc..f5ed03897210c3 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -927,7 +927,8 @@ static void __vfio_group_unset_container(struct
> vfio_group *group)
>   driver->ops->detach_group(container->iommu_data,
> group->iommu_group);
> 
> - iommu_group_release_dma_owner(group->iommu_group);
> + if (group->type == VFIO_IOMMU)
> + iommu_group_release_dma_owner(group->iommu_group);
> 
>   group->container = NULL;
>   group->container_users = 0;
> @@ -1001,9 +1002,11 @@ static int vfio_group_set_container(struct
> vfio_group *group, int container_fd)
>   goto unlock_out;
>   }
> 
> - ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
> - if (ret)
> - goto unlock_out;
> + if (group->type == VFIO_IOMMU) {
> + ret = iommu_group_claim_dma_owner(group-
> >iommu_group, f.file);
> + if (ret)
> + goto unlock_out;
> + }
> 
>   driver = container->iommu_driver;
>   if (driver) {
> @@ -1011,7 +1014,9 @@ static int vfio_group_set_container(struct
> vfio_group *group, int container_fd)
>   group->iommu_group,
>   group->type);
>   if (ret) {
> - iommu_group_release_dma_owner(group-
> >iommu_group);
> + if (group->type == VFIO_IOMMU)
> + iommu_group_release_dma_owner(
> + group->iommu_group);
>   goto unlock_out;
>   }
>   }
> 
> base-commit: 7ab5e10eda02da1d9562ffde562c51055d368e9c
> --
> 2.36.0

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


Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support

2022-05-19 Thread Baolu Lu

On 2022/5/20 00:33, Jean-Philippe Brucker wrote:

diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
index 8909ea1094e3..1be21e6b93ec 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva-lib.h
@@ -7,6 +7,7 @@
  
  #include 

  #include 
+#include 
  
  int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max);

  struct mm_struct *iommu_sva_find(ioasid_t pasid);
@@ -16,6 +17,20 @@ struct device;
  struct iommu_fault;
  struct iopf_queue;
  
+struct iommu_sva_domain {

+   struct iommu_domain domain;
+   struct mm_struct*mm;
+};
+
+#define to_sva_domain(d) container_of_safe(d, struct iommu_sva_domain, domain)

Is there a reason to use the 'safe' version of container_of()?  Callers of
to_sva_domain() don't check the return value before dereferencing it so
they would break anyway if someone passes an error pointer as domain.  I
think it matters because there is no other user of container_of_safe() in
the kernel (the only user, lustre, went away in 2018) so someone will want
to remove it.


Fair enough. I wondered why there's no user in the tree. Thanks for the
explanation. I will replace it with container_of().



Apart from that

Reviewed-by: Jean-Philippe Brucker



Thank you!

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


Re: [PATCH] vfio: Do not manipulate iommu dma_owner for fake iommu groups

2022-05-19 Thread Christoph Hellwig
On Thu, May 19, 2022 at 02:03:48PM -0300, Jason Gunthorpe via iommu wrote:
> Since asserting dma ownership now causes the group to have its DMA blocked
> the iommu layer requires a working iommu. This means the dma_owner APIs
> cannot be used on the fake groups that VFIO creates. Test for this and
> avoid calling them.
> 
> Otherwise asserting dma ownership will fail for VFIO mdev devices as a
> BLOCKING iommu_domain cannot be allocated due to the NULL iommu ops.

Fake iommu groups come back to bite again, part 42..

The patch looks good:

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


Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()

2022-05-19 Thread Baolu Lu

On 2022/5/20 00:39, Jean-Philippe Brucker wrote:

+struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct 
*mm)
+{
+   struct iommu_sva_domain *sva_domain;
+   struct iommu_domain *domain;
+   ioasid_t max_pasid = 0;
+   int ret = -EINVAL;
+
+   /* Allocate mm->pasid if necessary. */
+   if (!dev->iommu->iommu_dev->pasids)
+   return ERR_PTR(-EOPNOTSUPP);
+
+   if (dev_is_pci(dev)) {
+   max_pasid = pci_max_pasids(to_pci_dev(dev));
+   if (max_pasid < 0)
+   return ERR_PTR(max_pasid);
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  &max_pasid);
+   if (ret)
+   return ERR_PTR(ret);
+   max_pasid = (1UL << max_pasid);
+   }

The IOMMU driver needs this PASID width information earlier, when creating
the PASID table (in .probe_device(), .attach_dev()). Since we're moving it
to the IOMMU core to avoid code duplication, it should be done earlier and
stored in dev->iommu


Yes, really. How about below changes?

From f1382579e8a15ca49acdf758d38fd36451ea174d Mon Sep 17 00:00:00 2001
From: Lu Baolu 
Date: Mon, 28 Feb 2022 15:01:35 +0800
Subject: [PATCH 1/1] iommu: Add pasids field in struct dev_iommu

Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/iommu.c | 15 +++
 include/linux/iommu.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e49c5a5b8cc1..6b731171d42f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -194,6 +195,8 @@ EXPORT_SYMBOL_GPL(iommu_device_unregister);
 static struct dev_iommu *dev_iommu_get(struct device *dev)
 {
struct dev_iommu *param = dev->iommu;
+   u32 max_pasids = 0;
+   int ret;

if (param)
return param;
@@ -202,6 +205,18 @@ static struct dev_iommu *dev_iommu_get(struct 
device *dev)

if (!param)
return NULL;

+   if (dev_is_pci(dev)) {
+   ret = pci_max_pasids(to_pci_dev(dev));
+   if (ret > 0)
+   max_pasids = ret;
+   } else {
+   ret = device_property_read_u32(dev, "pasid-num-bits",
+  &max_pasids);
+   if (!ret)
+   max_pasids = (1UL << max_pasids);
+   }
+   param->pasids = max_pasids;
+
mutex_init(¶m->lock);
dev->iommu = param;
return param;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 45f274b2640d..d4296136ba75 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -371,6 +371,7 @@ struct iommu_fault_param {
  * @fwspec: IOMMU fwspec data
  * @iommu_dev:  IOMMU device this device is linked to
  * @priv:   IOMMU Driver private data
+ * @pasids: number of supported PASIDs
  *
  * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
  * struct iommu_group  *iommu_group;
@@ -382,6 +383,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void*priv;
+   u32 pasids;
 };

 int iommu_device_register(struct iommu_device *iommu,
--
2.25.1

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