On Mon, Jan 22, 2024 at 01:42:56PM +0800, Lu Baolu wrote:
> struct iommu_fault_page_request and struct iommu_page_response are not
> part of uAPI anymore. Convert them to data structures for kAPI.
>
> Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> Reviewed-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> Tested-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Tested-by: Longfang Liu <liulongfang@xxxxxxxxxx>
> ---
> include/linux/iommu.h | 27 +++++++++++----------------
> drivers/iommu/io-pgfault.c | 1 -
> drivers/iommu/iommu.c | 4 ----
> 3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index c960c4fae3bc..829bcb5a8e23 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -71,12 +71,12 @@ struct iommu_fault_page_request {
> #define IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE (1 << 1)
> #define IOMMU_FAULT_PAGE_REQUEST_PRIV_DATA (1 << 2)
> #define IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID (1 << 3)
> - __u32 flags;
> - __u32 pasid;
> - __u32 grpid;
> - __u32 perm;
> - __u64 addr;
> - __u64 private_data[2];
> + u32 flags;
> + u32 pasid;
> + u32 grpid;
> + u32 perm;
> + u64 addr;
> + u64 private_data[2];
> };
>
> /**
> @@ -85,7 +85,7 @@ struct iommu_fault_page_request {
> * @prm: Page Request message, when @type is %IOMMU_FAULT_PAGE_REQ
> */
> struct iommu_fault {
> - __u32 type;
> + u32 type;
> struct iommu_fault_page_request prm;
> };
>
> @@ -106,8 +106,6 @@ enum iommu_page_response_code {
>
> /**
> * struct iommu_page_response - Generic page response information
> - * @argsz: User filled size of this data
> - * @version: API version of this structure
> * @flags: encodes whether the corresponding fields are valid
> * (IOMMU_FAULT_PAGE_RESPONSE_* values)
> * @pasid: Process Address Space ID
> @@ -115,14 +113,11 @@ enum iommu_page_response_code {
> * @code: response code from &enum iommu_page_response_code
> */
> struct iommu_page_response {
> - __u32 argsz;
> -#define IOMMU_PAGE_RESP_VERSION_1 1
> - __u32 version;
> #define IOMMU_PAGE_RESP_PASID_VALID (1 << 0)
> - __u32 flags;
> - __u32 pasid;
> - __u32 grpid;
> - __u32 code;
> + u32 flags;
> + u32 pasid;
> + u32 grpid;
> + u32 code;
> };
>
>
> diff --git a/drivers/iommu/io-pgfault.c b/drivers/iommu/io-pgfault.c
> index e5b8b9110c13..24b5545352ae 100644
> --- a/drivers/iommu/io-pgfault.c
> +++ b/drivers/iommu/io-pgfault.c
> @@ -56,7 +56,6 @@ static int iopf_complete_group(struct device *dev, struct iopf_fault *iopf,
> enum iommu_page_response_code status)
> {
> struct iommu_page_response resp = {
> - .version = IOMMU_PAGE_RESP_VERSION_1,
> .pasid = iopf->fault.prm.pasid,
> .grpid = iopf->fault.prm.grpid,
> .code = status,
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 68e648b55767..b88dc3e0595c 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1494,10 +1494,6 @@ int iommu_page_response(struct device *dev,
> if (!param || !param->fault_param)
> return -EINVAL;
>
> - if (msg->version != IOMMU_PAGE_RESP_VERSION_1 ||
> - msg->flags & ~IOMMU_PAGE_RESP_PASID_VALID)
> - return -EINVAL;
> -
I see that this function `iommu_page_response` eventually lands in
drivers/iommu/io-pgfault.c as `iopf_group_response`. But it seems that
the check for IOMMU_PAGE_RESP_PASID_VALID is dropped.
I see that after applying [1] and [2] there are only three places where
IOMMU_PAGE_RESP_PASID_VALID appears in the code: One is the definition
and the other two are just setting the value. We effectively dropped the
check. Is the drop intended? and if so, should we just get rid of
IOMMU_PAGE_RESP_PASID_VALID?
Best
[1] https://lore.kernel.org/all/20240122054308.23901-1-baolu.lu@xxxxxxxxxxxxxxx
[2] https://lore.kernel.org/all/20240122073903.24406-1-baolu.lu@xxxxxxxxxxxxxxx
> /* Only send response if there is a fault report pending */
> mutex_lock(¶m->fault_param->lock);
> if (list_empty(¶m->fault_param->faults)) {
> --
> 2.34.1
>
--
Joel Granados
Attachment:
signature.asc
Description: PGP signature
