[PATCH] iommufd: Put constants for all the uAPI enums

[PATCH] iommufd: Put constants for all the uAPI enums

[Jason Gunthorpe]

Relying on position in the enum makes it subtly harder when doing merge
resolutions or backporting as it is easy to grab a patch and not notice it
is a uAPI change with a differently ordered enum. This may become a bigger
problem in next cycles when iommu_hwpt_invalidate_data_type and other
per-driver enums have patches flowing through different trees.

So lets start including constants for all the uAPI enums to make this
safer.

No functional change.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 include/uapi/linux/iommufd.h | 40 ++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index e31385b75d0be8..4dde745cfb7e29 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -37,20 +37,20 @@
 enum {
 	IOMMUFD_CMD_BASE = 0x80,
 	IOMMUFD_CMD_DESTROY = IOMMUFD_CMD_BASE,
-	IOMMUFD_CMD_IOAS_ALLOC,
-	IOMMUFD_CMD_IOAS_ALLOW_IOVAS,
-	IOMMUFD_CMD_IOAS_COPY,
-	IOMMUFD_CMD_IOAS_IOVA_RANGES,
-	IOMMUFD_CMD_IOAS_MAP,
-	IOMMUFD_CMD_IOAS_UNMAP,
-	IOMMUFD_CMD_OPTION,
-	IOMMUFD_CMD_VFIO_IOAS,
-	IOMMUFD_CMD_HWPT_ALLOC,
-	IOMMUFD_CMD_GET_HW_INFO,
-	IOMMUFD_CMD_HWPT_SET_DIRTY_TRACKING,
-	IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP,
-	IOMMUFD_CMD_HWPT_INVALIDATE,
-	IOMMUFD_CMD_FAULT_QUEUE_ALLOC,
+	IOMMUFD_CMD_IOAS_ALLOC = 0x81,
+	IOMMUFD_CMD_IOAS_ALLOW_IOVAS = 0x82,
+	IOMMUFD_CMD_IOAS_COPY = 0x83,
+	IOMMUFD_CMD_IOAS_IOVA_RANGES = 0x84,
+	IOMMUFD_CMD_IOAS_MAP = 0x85,
+	IOMMUFD_CMD_IOAS_UNMAP = 0x86,
+	IOMMUFD_CMD_OPTION = 0x87,
+	IOMMUFD_CMD_VFIO_IOAS = 0x88,
+	IOMMUFD_CMD_HWPT_ALLOC = 0x89,
+	IOMMUFD_CMD_GET_HW_INFO = 0x8a,
+	IOMMUFD_CMD_HWPT_SET_DIRTY_TRACKING = 0x8b,
+	IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP = 0x8c,
+	IOMMUFD_CMD_HWPT_INVALIDATE = 0x8d,
+	IOMMUFD_CMD_FAULT_QUEUE_ALLOC = 0x8e,
 };
 
 /**
@@ -400,8 +400,8 @@ struct iommu_hwpt_vtd_s1 {
  * @IOMMU_HWPT_DATA_VTD_S1: Intel VT-d stage-1 page table
  */
 enum iommu_hwpt_data_type {
-	IOMMU_HWPT_DATA_NONE,
-	IOMMU_HWPT_DATA_VTD_S1,
+	IOMMU_HWPT_DATA_NONE = 0,
+	IOMMU_HWPT_DATA_VTD_S1 = 1,
 };
 
 /**
@@ -491,8 +491,8 @@ struct iommu_hw_info_vtd {
  * @IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type
  */
 enum iommu_hw_info_type {
-	IOMMU_HW_INFO_TYPE_NONE,
-	IOMMU_HW_INFO_TYPE_INTEL_VTD,
+	IOMMU_HW_INFO_TYPE_NONE = 0,
+	IOMMU_HW_INFO_TYPE_INTEL_VTD = 1,
 };
 
 /**
@@ -629,7 +629,7 @@ struct iommu_hwpt_get_dirty_bitmap {
  * @IOMMU_HWPT_INVALIDATE_DATA_VTD_S1: Invalidation data for VTD_S1
  */
 enum iommu_hwpt_invalidate_data_type {
-	IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
+	IOMMU_HWPT_INVALIDATE_DATA_VTD_S1 = 0,
 };
 
 /**
@@ -768,7 +768,7 @@ struct iommu_hwpt_pgfault {
  */
 enum iommufd_page_response_code {
 	IOMMUFD_PAGE_RESP_SUCCESS = 0,
-	IOMMUFD_PAGE_RESP_INVALID,
+	IOMMUFD_PAGE_RESP_INVALID = 1,
 };
 
 /**

base-commit: c7a0991733cc7b05ca32217c156f7030617f5879
-- 
2.45.2

RE: [PATCH] iommufd: Put constants for all the uAPI enums

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, July 12, 2024 8:11 AM
> 
> Relying on position in the enum makes it subtly harder when doing merge
> resolutions or backporting as it is easy to grab a patch and not notice it
> is a uAPI change with a differently ordered enum. This may become a bigger
> problem in next cycles when iommu_hwpt_invalidate_data_type and other
> per-driver enums have patches flowing through different trees.
> 
> So lets start including constants for all the uAPI enums to make this
> safer.
> 
> No functional change.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Re: [PATCH] iommufd: Put constants for all the uAPI enums

[Yi Liu]

On 2024/7/12 08:11, Jason Gunthorpe wrote:
> Relying on position in the enum makes it subtly harder when doing merge
> resolutions or backporting as it is easy to grab a patch and not notice it
> is a uAPI change with a differently ordered enum. This may become a bigger
> problem in next cycles when iommu_hwpt_invalidate_data_type and other
> per-driver enums have patches flowing through different trees.
> 
> So lets start including constants for all the uAPI enums to make this
> safer.
> 
> No functional change.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   include/uapi/linux/iommufd.h | 40 ++++++++++++++++++------------------
>   1 file changed, 20 insertions(+), 20 deletions(-)

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

Apply this patch to 6.10 and apply a WA patch[1] to make the iommufd
selftest functional. This does not inlcude Baolu's PRI series. Did two
tests:
  - Boot a VM which uses iommu nested translation, and run a test within
    VM.
  - Run the iommufd selftest (iommufd and iommufd_fail_nth).

Both results look good, hence,

Tested-by: Yi Liu <yi.l.liu@intel.com>

[1] 
https://lore.kernel.org/linux-iommu/20240111073213.180020-1-baolu.lu@linux.intel.com/

> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index e31385b75d0be8..4dde745cfb7e29 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -37,20 +37,20 @@
>   enum {
>   	IOMMUFD_CMD_BASE = 0x80,
>   	IOMMUFD_CMD_DESTROY = IOMMUFD_CMD_BASE,
> -	IOMMUFD_CMD_IOAS_ALLOC,
> -	IOMMUFD_CMD_IOAS_ALLOW_IOVAS,
> -	IOMMUFD_CMD_IOAS_COPY,
> -	IOMMUFD_CMD_IOAS_IOVA_RANGES,
> -	IOMMUFD_CMD_IOAS_MAP,
> -	IOMMUFD_CMD_IOAS_UNMAP,
> -	IOMMUFD_CMD_OPTION,
> -	IOMMUFD_CMD_VFIO_IOAS,
> -	IOMMUFD_CMD_HWPT_ALLOC,
> -	IOMMUFD_CMD_GET_HW_INFO,
> -	IOMMUFD_CMD_HWPT_SET_DIRTY_TRACKING,
> -	IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP,
> -	IOMMUFD_CMD_HWPT_INVALIDATE,
> -	IOMMUFD_CMD_FAULT_QUEUE_ALLOC,
> +	IOMMUFD_CMD_IOAS_ALLOC = 0x81,
> +	IOMMUFD_CMD_IOAS_ALLOW_IOVAS = 0x82,
> +	IOMMUFD_CMD_IOAS_COPY = 0x83,
> +	IOMMUFD_CMD_IOAS_IOVA_RANGES = 0x84,
> +	IOMMUFD_CMD_IOAS_MAP = 0x85,
> +	IOMMUFD_CMD_IOAS_UNMAP = 0x86,
> +	IOMMUFD_CMD_OPTION = 0x87,
> +	IOMMUFD_CMD_VFIO_IOAS = 0x88,
> +	IOMMUFD_CMD_HWPT_ALLOC = 0x89,
> +	IOMMUFD_CMD_GET_HW_INFO = 0x8a,
> +	IOMMUFD_CMD_HWPT_SET_DIRTY_TRACKING = 0x8b,
> +	IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP = 0x8c,
> +	IOMMUFD_CMD_HWPT_INVALIDATE = 0x8d,
> +	IOMMUFD_CMD_FAULT_QUEUE_ALLOC = 0x8e,
>   };
>   
>   /**
> @@ -400,8 +400,8 @@ struct iommu_hwpt_vtd_s1 {
>    * @IOMMU_HWPT_DATA_VTD_S1: Intel VT-d stage-1 page table
>    */
>   enum iommu_hwpt_data_type {
> -	IOMMU_HWPT_DATA_NONE,
> -	IOMMU_HWPT_DATA_VTD_S1,
> +	IOMMU_HWPT_DATA_NONE = 0,
> +	IOMMU_HWPT_DATA_VTD_S1 = 1,
>   };
>   
>   /**
> @@ -491,8 +491,8 @@ struct iommu_hw_info_vtd {
>    * @IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type
>    */
>   enum iommu_hw_info_type {
> -	IOMMU_HW_INFO_TYPE_NONE,
> -	IOMMU_HW_INFO_TYPE_INTEL_VTD,
> +	IOMMU_HW_INFO_TYPE_NONE = 0,
> +	IOMMU_HW_INFO_TYPE_INTEL_VTD = 1,
>   };
>   
>   /**
> @@ -629,7 +629,7 @@ struct iommu_hwpt_get_dirty_bitmap {
>    * @IOMMU_HWPT_INVALIDATE_DATA_VTD_S1: Invalidation data for VTD_S1
>    */
>   enum iommu_hwpt_invalidate_data_type {
> -	IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
> +	IOMMU_HWPT_INVALIDATE_DATA_VTD_S1 = 0,
>   };
>   
>   /**
> @@ -768,7 +768,7 @@ struct iommu_hwpt_pgfault {
>    */
>   enum iommufd_page_response_code {
>   	IOMMUFD_PAGE_RESP_SUCCESS = 0,
> -	IOMMUFD_PAGE_RESP_INVALID,
> +	IOMMUFD_PAGE_RESP_INVALID = 1,
>   };
>   
>   /**
> 
> base-commit: c7a0991733cc7b05ca32217c156f7030617f5879

-- 
Regards,
Yi Liu

Re: [PATCH] iommufd: Put constants for all the uAPI enums

[Jason Gunthorpe]

On Thu, Jul 11, 2024 at 09:11:03PM -0300, Jason Gunthorpe wrote:
> Relying on position in the enum makes it subtly harder when doing merge
> resolutions or backporting as it is easy to grab a patch and not notice it
> is a uAPI change with a differently ordered enum. This may become a bigger
> problem in next cycles when iommu_hwpt_invalidate_data_type and other
> per-driver enums have patches flowing through different trees.
> 
> So lets start including constants for all the uAPI enums to make this
> safer.
> 
> No functional change.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  include/uapi/linux/iommufd.h | 40 ++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)

Applied

Jason