RE: [PATCH v7 02/10] iommu: Remove SVM_FLAG_SUPERVISOR_MODE support
> 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
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
