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

2022-05-24 Thread Tian, Kevin
> From: Lu Baolu 
> Sent: Thursday, May 19, 2022 3:21 PM
> 
> 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 

Reviewed-by: Kevin Tian 

> ---
>  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 

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 

[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)