Re: [PATCH v2 3/3] iommu: Remove ioasid infrastructure

From: Jean-Philippe Brucker
Date: Tue Feb 14 2023 - 04:32:53 EST


On Mon, Feb 13, 2023 at 10:44:14PM -0800, Jacob Pan wrote:
> This has no use anymore, delete it all.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>

Yes, a number of intended users haven't materialized yet. They can
re-introduce the allocator when they need it.

> ---
> v2:
> - fix compile issue w/o CONFIG_IOMMU_SVA
> - consolidate INVALID_IOASID w/ IOMMU_PASID_INVALID

Why not rename ioasid_t as well? Probably better to move all the renaming
to a separate patch

> ---
> Documentation/x86/sva.rst | 2 +-
> arch/x86/kernel/traps.c | 5 +-
> drivers/dma/idxd/device.c | 8 +-
> drivers/dma/idxd/idxd.h | 1 -
> drivers/dma/idxd/init.c | 2 +-
> drivers/dma/idxd/irq.c | 2 +-
> drivers/iommu/intel/dmar.c | 4 +-
> drivers/iommu/intel/iommu.c | 2 +-
> drivers/iommu/intel/iommu.h | 1 -
> drivers/iommu/intel/svm.c | 3 +-
> drivers/iommu/ioasid.c | 422 ------------------------------------
> drivers/iommu/iommu-sva.c | 2 +-
> drivers/iommu/iommu-sva.h | 1 -
> include/linux/ioasid.h | 83 -------
> include/linux/iommu.h | 11 +-
> include/linux/sched/mm.h | 4 +-
> include/uapi/linux/iommu.h | 1 +
> mm/init-mm.c | 4 +-

drivers/iommu/{Makefile,Kconfig} also need updates

> 18 files changed, 29 insertions(+), 529 deletions(-)
> delete mode 100644 drivers/iommu/ioasid.c
> delete mode 100644 include/linux/ioasid.h

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 46e1347bfa22..7cbcf37df8cd 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -13,7 +13,6 @@
> #include <linux/errno.h>
> #include <linux/err.h>
> #include <linux/of.h>
> -#include <linux/ioasid.h>
> #include <uapi/linux/iommu.h>
>
> #define IOMMU_READ (1 << 0)
> @@ -192,8 +191,14 @@ enum iommu_dev_features {
> IOMMU_DEV_FEAT_IOPF,
> };
>
> -#define IOMMU_PASID_INVALID (-1U)
> -
> +//#define IOMMU_PASID_INVALID (-1U)

Remove this

> +typedef unsigned int ioasid_t;
> +#ifdef CONFIG_IOMMU_SVA
> +static inline bool pasid_valid(ioasid_t ioasid)
> +{
> + return ioasid != IOMMU_PASID_INVALID;
> +}
> +#endif

This isn't specific to SVA, doesn't need an ifdef

> #ifdef CONFIG_IOMMU_API
>
> /**
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index cdc138086230..4c52835d3359 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -8,7 +8,7 @@
> #include <linux/mm_types.h>
> #include <linux/gfp.h>
> #include <linux/sync_core.h>
> -#include <linux/ioasid.h>
> +#include <uapi/linux/iommu.h>
>
> /*
> * Routines for handling mm_structs
> @@ -454,7 +454,7 @@ static inline void membarrier_update_current_mm(struct mm_struct *next_mm)
> #ifdef CONFIG_IOMMU_SVA
> static inline void mm_pasid_init(struct mm_struct *mm)
> {
> - mm->pasid = INVALID_IOASID;
> + mm->pasid = IOMMU_PASID_INVALID;
> }
>
> /* Associate a PASID with an mm_struct: */
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 65d8b0234f69..a95f07f0c98c 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -157,5 +157,6 @@ struct iommu_page_response {
> __u32 grpid;
> __u32 code;
> };
> +#define IOMMU_PASID_INVALID (-1U)

Why change UAPI?

Thanks,
Jean

>
> #endif /* _UAPI_IOMMU_H */
> diff --git a/mm/init-mm.c b/mm/init-mm.c
> index c9327abb771c..a084039f55d8 100644
> --- a/mm/init-mm.c
> +++ b/mm/init-mm.c
> @@ -10,7 +10,7 @@
>
> #include <linux/atomic.h>
> #include <linux/user_namespace.h>
> -#include <linux/ioasid.h>
> +#include <linux/iommu.h>
> #include <asm/mmu.h>
>
> #ifndef INIT_MM_CONTEXT
> @@ -40,7 +40,7 @@ struct mm_struct init_mm = {
> .user_ns = &init_user_ns,
> .cpu_bitmap = CPU_BITS_NONE,
> #ifdef CONFIG_IOMMU_SVA
> - .pasid = INVALID_IOASID,
> + .pasid = IOMMU_PASID_INVALID,
> #endif
> INIT_MM_CONTEXT(init_mm)
> };
> --
> 2.25.1
>