Re: [PATCH v10 01/16] iommu: Move iommu fault data to linux/iommu.h

Joel Granados
2024-01-25T09:17:39Z

[-- Attachment #1: Type: text/plain, Size: 13331 bytes --]

On Mon, Jan 22, 2024 at 01:42:53PM +0800, Lu Baolu wrote:
> The iommu fault data is currently defined in uapi/linux/iommu.h, but is
> only used inside the iommu subsystem. Move it to linux/iommu.h, where it
> will be more accessible to kernel drivers.
>
> With this done, uapi/linux/iommu.h becomes empty and can be removed from
> the tree.

The reason for removing this [1] is that it is only being used by
internal code in the kernel. What happens with usespace code that have
used these definitions? Should we deprecate instead of just removing?

Best

[1] https://lore.kernel.org/all/20230711010642.19707-2-baolu.lu@linux.intel.com/
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
> Tested-by: Yan Zhao <yan.y.zhao@intel.com>
> Tested-by: Longfang Liu <liulongfang@huawei.com>
> ---
> include/linux/iommu.h | 152 +++++++++++++++++++++++++++++++++-
> include/uapi/linux/iommu.h | 161 -------------------------------------
> MAINTAINERS | 1 -
> 3 files changed, 151 insertions(+), 163 deletions(-)
> delete mode 100644 include/uapi/linux/iommu.h
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 1ea2a820e1eb..472a8ce029b1 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -14,7 +14,6 @@
> #include <linux/err.h>
> #include <linux/of.h>
> #include <linux/iova_bitmap.h>
> -#include <uapi/linux/iommu.h>
>
> #define IOMMU_READ (1 << 0)
> #define IOMMU_WRITE (1 << 1)
> @@ -44,6 +43,157 @@ struct iommu_sva;
> struct iommu_fault_event;
> struct iommu_dma_cookie;
>
> +#define IOMMU_FAULT_PERM_READ (1 << 0) /* read */
> +#define IOMMU_FAULT_PERM_WRITE (1 << 1) /* write */
> +#define IOMMU_FAULT_PERM_EXEC (1 << 2) /* exec */
> +#define IOMMU_FAULT_PERM_PRIV (1 << 3) /* privileged */
> +
> +/* Generic fault types, can be expanded IRQ remapping fault */
> +enum iommu_fault_type {
> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
> + IOMMU_FAULT_PAGE_REQ, /* page request fault */
> +};
> +
> +enum iommu_fault_reason {
> + IOMMU_FAULT_REASON_UNKNOWN = 0,
> +
> + /* Could not access the PASID table (fetch caused external abort) */
> + IOMMU_FAULT_REASON_PASID_FETCH,
> +
> + /* PASID entry is invalid or has configuration errors */
> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
> +
> + /*
> + * PASID is out of range (e.g. exceeds the maximum PASID
> + * supported by the IOMMU) or disabled.
> + */
> + IOMMU_FAULT_REASON_PASID_INVALID,
> +
> + /*
> + * An external abort occurred fetching (or updating) a translation
> + * table descriptor
> + */
> + IOMMU_FAULT_REASON_WALK_EABT,
> +
> + /*
> + * Could not access the page table entry (Bad address),
> + * actual translation fault
> + */
> + IOMMU_FAULT_REASON_PTE_FETCH,
> +
> + /* Protection flag check failed */
> + IOMMU_FAULT_REASON_PERMISSION,
> +
> + /* access flag check failed */
> + IOMMU_FAULT_REASON_ACCESS,
> +
> + /* Output address of a translation stage caused Address Size fault */
> + IOMMU_FAULT_REASON_OOR_ADDRESS,
> +};
> +
> +/**
> + * struct iommu_fault_unrecoverable - Unrecoverable fault data
> + * @reason: reason of the fault, from &enum iommu_fault_reason
> + * @flags: parameters of this fault (IOMMU_FAULT_UNRECOV_* values)
> + * @pasid: Process Address Space ID
> + * @perm: requested permission access using by the incoming transaction
> + * (IOMMU_FAULT_PERM_* values)
> + * @addr: offending page address
> + * @fetch_addr: address that caused a fetch abort, if any
> + */
> +struct iommu_fault_unrecoverable {
> + __u32 reason;
> +#define IOMMU_FAULT_UNRECOV_PASID_VALID (1 << 0)
> +#define IOMMU_FAULT_UNRECOV_ADDR_VALID (1 << 1)
> +#define IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID (1 << 2)
> + __u32 flags;
> + __u32 pasid;
> + __u32 perm;
> + __u64 addr;
> + __u64 fetch_addr;
> +};
> +
> +/**
> + * struct iommu_fault_page_request - Page Request data
> + * @flags: encodes whether the corresponding fields are valid and whether this
> + * is the last page in group (IOMMU_FAULT_PAGE_REQUEST_* values).
> + * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response
> + * must have the same PASID value as the page request. When it is clear,
> + * the page response should not have a PASID.
> + * @pasid: Process Address Space ID
> + * @grpid: Page Request Group Index
> + * @perm: requested page permissions (IOMMU_FAULT_PERM_* values)
> + * @addr: page address
> + * @private_data: device-specific private information
> + */
> +struct iommu_fault_page_request {
> +#define IOMMU_FAULT_PAGE_REQUEST_PASID_VALID (1 << 0)
> +#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];
> +};
> +
> +/**
> + * struct iommu_fault - Generic fault data
> + * @type: fault type from &enum iommu_fault_type
> + * @padding: reserved for future use (should be zero)
> + * @event: fault event, when @type is %IOMMU_FAULT_DMA_UNRECOV
> + * @prm: Page Request message, when @type is %IOMMU_FAULT_PAGE_REQ
> + * @padding2: sets the fault size to allow for future extensions
> + */
> +struct iommu_fault {
> + __u32 type;
> + __u32 padding;
> + union {
> + struct iommu_fault_unrecoverable event;
> + struct iommu_fault_page_request prm;
> + __u8 padding2[56];
> + };
> +};
> +
> +/**
> + * enum iommu_page_response_code - Return status of fault handlers
> + * @IOMMU_PAGE_RESP_SUCCESS: Fault has been handled and the page tables
> + * populated, retry the access. This is "Success" in PCI PRI.
> + * @IOMMU_PAGE_RESP_FAILURE: General error. Drop all subsequent faults from
> + * this device if possible. This is "Response Failure" in PCI PRI.
> + * @IOMMU_PAGE_RESP_INVALID: Could not handle this fault, don't retry the
> + * access. This is "Invalid Request" in PCI PRI.
> + */
> +enum iommu_page_response_code {
> + IOMMU_PAGE_RESP_SUCCESS = 0,
> + IOMMU_PAGE_RESP_INVALID,
> + IOMMU_PAGE_RESP_FAILURE,
> +};
> +
> +/**
> + * 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
> + * @grpid: Page Request Group Index
> + * @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;
> +};
> +
> +
> /* iommu fault flags */
> #define IOMMU_FAULT_READ 0x0
> #define IOMMU_FAULT_WRITE 0x1
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> deleted file mode 100644
> index 65d8b0234f69..000000000000
> --- a/include/uapi/linux/iommu.h
> +++ /dev/null
> @@ -1,161 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> -/*
> - * IOMMU user API definitions
> - */
> -
> -#ifndef _UAPI_IOMMU_H
> -#define _UAPI_IOMMU_H
> -
> -#include <linux/types.h>
> -
> -#define IOMMU_FAULT_PERM_READ (1 << 0) /* read */
> -#define IOMMU_FAULT_PERM_WRITE (1 << 1) /* write */
> -#define IOMMU_FAULT_PERM_EXEC (1 << 2) /* exec */
> -#define IOMMU_FAULT_PERM_PRIV (1 << 3) /* privileged */
> -
> -/* Generic fault types, can be expanded IRQ remapping fault */
> -enum iommu_fault_type {
> - IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
> - IOMMU_FAULT_PAGE_REQ, /* page request fault */
> -};
> -
> -enum iommu_fault_reason {
> - IOMMU_FAULT_REASON_UNKNOWN = 0,
> -
> - /* Could not access the PASID table (fetch caused external abort) */
> - IOMMU_FAULT_REASON_PASID_FETCH,
> -
> - /* PASID entry is invalid or has configuration errors */
> - IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
> -
> - /*
> - * PASID is out of range (e.g. exceeds the maximum PASID
> - * supported by the IOMMU) or disabled.
> - */
> - IOMMU_FAULT_REASON_PASID_INVALID,
> -
> - /*
> - * An external abort occurred fetching (or updating) a translation
> - * table descriptor
> - */
> - IOMMU_FAULT_REASON_WALK_EABT,
> -
> - /*
> - * Could not access the page table entry (Bad address),
> - * actual translation fault
> - */
> - IOMMU_FAULT_REASON_PTE_FETCH,
> -
> - /* Protection flag check failed */
> - IOMMU_FAULT_REASON_PERMISSION,
> -
> - /* access flag check failed */
> - IOMMU_FAULT_REASON_ACCESS,
> -
> - /* Output address of a translation stage caused Address Size fault */
> - IOMMU_FAULT_REASON_OOR_ADDRESS,
> -};
> -
> -/**
> - * struct iommu_fault_unrecoverable - Unrecoverable fault data
> - * @reason: reason of the fault, from &enum iommu_fault_reason
> - * @flags: parameters of this fault (IOMMU_FAULT_UNRECOV_* values)
> - * @pasid: Process Address Space ID
> - * @perm: requested permission access using by the incoming transaction
> - * (IOMMU_FAULT_PERM_* values)
> - * @addr: offending page address
> - * @fetch_addr: address that caused a fetch abort, if any
> - */
> -struct iommu_fault_unrecoverable {
> - __u32 reason;
> -#define IOMMU_FAULT_UNRECOV_PASID_VALID (1 << 0)
> -#define IOMMU_FAULT_UNRECOV_ADDR_VALID (1 << 1)
> -#define IOMMU_FAULT_UNRECOV_FETCH_ADDR_VALID (1 << 2)
> - __u32 flags;
> - __u32 pasid;
> - __u32 perm;
> - __u64 addr;
> - __u64 fetch_addr;
> -};
> -
> -/**
> - * struct iommu_fault_page_request - Page Request data
> - * @flags: encodes whether the corresponding fields are valid and whether this
> - * is the last page in group (IOMMU_FAULT_PAGE_REQUEST_* values).
> - * When IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID is set, the page response
> - * must have the same PASID value as the page request. When it is clear,
> - * the page response should not have a PASID.
> - * @pasid: Process Address Space ID
> - * @grpid: Page Request Group Index
> - * @perm: requested page permissions (IOMMU_FAULT_PERM_* values)
> - * @addr: page address
> - * @private_data: device-specific private information
> - */
> -struct iommu_fault_page_request {
> -#define IOMMU_FAULT_PAGE_REQUEST_PASID_VALID (1 << 0)
> -#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];
> -};
> -
> -/**
> - * struct iommu_fault - Generic fault data
> - * @type: fault type from &enum iommu_fault_type
> - * @padding: reserved for future use (should be zero)
> - * @event: fault event, when @type is %IOMMU_FAULT_DMA_UNRECOV
> - * @prm: Page Request message, when @type is %IOMMU_FAULT_PAGE_REQ
> - * @padding2: sets the fault size to allow for future extensions
> - */
> -struct iommu_fault {
> - __u32 type;
> - __u32 padding;
> - union {
> - struct iommu_fault_unrecoverable event;
> - struct iommu_fault_page_request prm;
> - __u8 padding2[56];
> - };
> -};
> -
> -/**
> - * enum iommu_page_response_code - Return status of fault handlers
> - * @IOMMU_PAGE_RESP_SUCCESS: Fault has been handled and the page tables
> - * populated, retry the access. This is "Success" in PCI PRI.
> - * @IOMMU_PAGE_RESP_FAILURE: General error. Drop all subsequent faults from
> - * this device if possible. This is "Response Failure" in PCI PRI.
> - * @IOMMU_PAGE_RESP_INVALID: Could not handle this fault, don't retry the
> - * access. This is "Invalid Request" in PCI PRI.
> - */
> -enum iommu_page_response_code {
> - IOMMU_PAGE_RESP_SUCCESS = 0,
> - IOMMU_PAGE_RESP_INVALID,
> - IOMMU_PAGE_RESP_FAILURE,
> -};
> -
> -/**
> - * 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
> - * @grpid: Page Request Group Index
> - * @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;
> -};
> -
> -#endif /* _UAPI_IOMMU_H */
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8d1052fa6a69..97846088d34d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11239,7 +11239,6 @@ F: drivers/iommu/
> F: include/linux/iommu.h
> F: include/linux/iova.h
> F: include/linux/of_iommu.h
> -F: include/uapi/linux/iommu.h
>
> IOMMUFD
> M: Jason Gunthorpe <jgg@nvidia.com>
> --
> 2.34.1
>

--

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]