2024-01-11 04:10:50

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 0/8] Add iommufd nesting (part 2/2)

Nested translation is a hardware feature that is supported by many modern
IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
to get access to the physical address. stage-1 translation table is owned
by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
to stage-1 translation table should be followed by an IOTLB invalidation.

Take Intel VT-d as an example, the stage-1 translation table is I/O page
table. As the below diagram shows, guest I/O page table pointer in GPA
(guest physical address) is passed to host and be used to perform the stage-1
address translation. Along with it, modifications to present mappings in the
guest I/O page table should be followed with an IOTLB invalidation.

.-------------. .---------------------------.
| vIOMMU | | Guest I/O page table |
| | '---------------------------'
.----------------/
| PASID Entry |--- PASID cache flush --+
'-------------' |
| | V
| | I/O page table pointer in GPA
'-------------'
Guest
------| Shadow |---------------------------|--------
v v v
Host
.-------------. .------------------------.
| pIOMMU | | FS for GIOVA->GPA |
| | '------------------------'
.----------------/ |
| PASID Entry | V (Nested xlate)
'----------------\.----------------------------------.
| | | SS for GPA->HPA, unmanaged domain|
| | '----------------------------------'
'-------------'
Where:
- FS = First stage page tables
- SS = Second stage page tables
<Intel VT-d Nested translation>

This series is based on the first part which was merged [1], this series is to
add the cache invalidation interface or the userspace to invalidate cache after
modifying the stage-1 page table. This includes both the iommufd changes and the
VT-d driver changes.

Complete code can be found in [2], QEMU could can be found in [3].

At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
them for the help. ^_^. Look forward to your feedbacks.

[1] https://lore.kernel.org/linux-iommu/[email protected]/ - merged
[2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting
[3] https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting_rfcv1

Change log:

v11:
- Drop hw_error field in vtd cache invalidation uapi. devTLB invalidation
error is a serious security emergency requiring the host kernel to handle.
No need to expose it to userspace (especially given existing VMs doesn't
issue devTLB invalidation at all).
- The vtd qi_submit_sync() and related callers are reverted back to the
original state due to above drop.
- Align with the vtd path, drop the hw_error reporting in mock driver and
selftest as well since selftest is a demo of the real driver.
- Drop iommu_respond_struct_to_user_array() since no more driver want to
respond single entry in the user_array.
- Two typos from Wubinbin

v10: https://lore.kernel.org/all/[email protected]/
- Minor tweak to patch 07 (Kevin)
- Rebase on top of 6.7-rc8

v9: https://lore.kernel.org/linux-iommu/[email protected]/
- Add a test case which sets both IOMMU_TEST_INVALIDATE_FLAG_ALL and
IOMMU_TEST_INVALIDATE_FLAG_TRIGGER_ERROR in flags, and expect to succeed
and see an 'error'. (Kevin)
- Returns -ETIMEOUT in qi_check_fault() if caller is interested with the
fault when timeout happens. If not, the qi_submit_sync() will keep retry
hence unable to report the error back to user. For now, only the user cache
invalidation path has interest on the time out error. So this change only
affects the user cache invalidation path. Other path will still hang in
qi_submit_sync() when timeout happens. (Kevin)

v8: https://lore.kernel.org/linux-iommu/[email protected]/
- Pass invalidation hint to the cache invalidation helper in the cache_invalidate_user
op path (Kevin)
- Move the devTLB invalidation out of info->iommu loop (Kevin, Weijiang)
- Clear *fault per restart in qi_submit_sync() to avoid acroos submission error
accumulation. (Kevin)
- Define the vtd cache invalidation uapi structure in separate patch (Kevin)
- Rename inv_error to be hw_error (Kevin)
- Rename 'reqs_uptr', 'req_type', 'req_len' and 'req_num' to be 'data_uptr',
'data_type', "entry_len' and 'entry_num" (Kevin)
- Allow user to set IOMMU_TEST_INVALIDATE_FLAG_ALL and IOMMU_TEST_INVALIDATE_FLAG_TRIGGER_ERROR
in the same time (Kevin)

v7: https://lore.kernel.org/linux-iommu/[email protected]/
- Remove domain->ops->cache_invalidate_user check in hwpt alloc path due
to failure in bisect (Baolu)
- Remove out_driver_error_code from struct iommu_hwpt_invalidate after
discussion in v6. Should expect per-entry error code.
- Rework the selftest cache invalidation part to report a per-entry error
- Allow user to pass in an empty array to have a try-and-fail mechanism for
user to check if a given req_type is supported by the kernel (Jason)
- Define a separate enum type for cache invalidation data (Jason)
- Fix the IOMMU_HWPT_INVALIDATE to always update the req_num field before
returning (Nicolin)
- Merge the VT-d nesting part 2/2
https://lore.kernel.org/linux-iommu/[email protected]/
into this series to avoid defining empty enum in the middle of the series.
The major difference is adding the VT-d related invalidation uapi structures
together with the generic data structures in patch 02 of this series.
- VT-d driver was refined to report ICE/ITE error from the bottom cache
invalidation submit helpers, hence the cache_invalidate_user op could
report such errors via the per-entry error field to user. VT-d driver
will not stop the invalidation array walking due to the ICE/ITE errors
as such errors are defined by VT-d spec, userspace should be able to
handle it and let the real user (say Virtual Machine) know about it.
But for other errors like invalid uapi data structure configuration,
memory copy failure, such errors should stop the array walking as it
may have more issues if go on.
- Minor fixes per Jason and Kevin's review comments

v6: https://lore.kernel.org/linux-iommu/[email protected]/
- No much change, just rebase on top of 6.7-rc1 as part 1/2 is merged

v5: https://lore.kernel.org/linux-iommu/[email protected]/#t
- Split the iommufd nesting series into two parts of alloc_user and
invalidation (Jason)
- Split IOMMUFD_OBJ_HW_PAGETABLE to IOMMUFD_OBJ_HWPT_PAGING/_NESTED, and
do the same with the structures/alloc()/abort()/destroy(). Reworked the
selftest accordingly too. (Jason)
- Move hwpt/data_type into struct iommu_user_data from standalone op
arguments. (Jason)
- Rename hwpt_type to be data_type, the HWPT_TYPE to be HWPT_ALLOC_DATA,
_TYPE_DEFAULT to be _ALLOC_DATA_NONE (Jason, Kevin)
- Rename iommu_copy_user_data() to iommu_copy_struct_from_user() (Kevin)
- Add macro to the iommu_copy_struct_from_user() to calculate min_size
(Jason)
- Fix two bugs spotted by ZhaoYan

v4: https://lore.kernel.org/linux-iommu/[email protected]/
- Separate HWPT alloc/destroy/abort functions between user-managed HWPTs
and kernel-managed HWPTs
- Rework invalidate uAPI to be a multi-request array-based design
- Add a struct iommu_user_data_array and a helper for driver to sanitize
and copy the entry data from user space invalidation array
- Add a patch fixing TEST_LENGTH() in selftest program
- Drop IOMMU_RESV_IOVA_RANGES patches
- Update kdoc and inline comments
- Drop the code to add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation,
this does not change the rule that resv regions should only be added to the
kernel-managed HWPT. The IOMMU_RESV_SW_MSI stuff will be added in later series
as it is needed only by SMMU so far.

v3: https://lore.kernel.org/linux-iommu/[email protected]/
- Add new uAPI things in alphabetical order
- Pass in "enum iommu_hwpt_type hwpt_type" to op->domain_alloc_user for
sanity, replacing the previous op->domain_alloc_user_data_len solution
- Return ERR_PTR from domain_alloc_user instead of NULL
- Only add IOMMU_RESV_SW_MSI to kernel-managed HWPT in nested translation (Kevin)
- Add IOMMU_RESV_IOVA_RANGES to report resv iova ranges to userspace hence
userspace is able to exclude the ranges in the stage-1 HWPT (e.g. guest I/O
page table). (Kevin)
- Add selftest coverage for the new IOMMU_RESV_IOVA_RANGES ioctl
- Minor changes per Kevin's inputs

v2: https://lore.kernel.org/linux-iommu/[email protected]/
- Add union iommu_domain_user_data to include all user data structures to avoid
passing void * in kernel APIs.
- Add iommu op to return user data length for user domain allocation
- Rename struct iommu_hwpt_alloc::data_type to be hwpt_type
- Store the invalidation data length in iommu_domain_ops::cache_invalidate_user_data_len
- Convert cache_invalidate_user op to be int instead of void
- Remove @data_type in struct iommu_hwpt_invalidate
- Remove out_hwpt_type_bitmap in struct iommu_hw_info hence drop patch 08 of v1

v1: https://lore.kernel.org/linux-iommu/[email protected]/

Thanks,
Yi Liu

Lu Baolu (2):
iommu: Add cache_invalidate_user op
iommu/vt-d: Add iotlb flush for nested domain

Nicolin Chen (4):
iommu: Add iommu_copy_struct_from_user_array helper
iommufd/selftest: Add mock_domain_cache_invalidate_user support
iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op
iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl

Yi Liu (2):
iommufd: Add IOMMU_HWPT_INVALIDATE
iommufd: Add data structure for Intel VT-d stage-1 cache invalidation

drivers/iommu/intel/nested.c | 88 ++++++++++
drivers/iommu/iommufd/hw_pagetable.c | 41 +++++
drivers/iommu/iommufd/iommufd_private.h | 10 ++
drivers/iommu/iommufd/iommufd_test.h | 23 +++
drivers/iommu/iommufd/main.c | 3 +
drivers/iommu/iommufd/selftest.c | 76 +++++++++
include/linux/iommu.h | 79 +++++++++
include/uapi/linux/iommufd.h | 79 +++++++++
tools/testing/selftests/iommu/iommufd.c | 152 ++++++++++++++++++
tools/testing/selftests/iommu/iommufd_utils.h | 57 +++++++
10 files changed, 608 insertions(+)

--
2.34.1



2024-01-11 04:11:08

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 2/8] iommufd: Add IOMMU_HWPT_INVALIDATE

In nested translation, the stage-1 page table is user-managed but cached
by the IOMMU hardware, so an update on present page table entries in the
stage-1 page table should be followed with a cache invalidation.

Add an IOMMU_HWPT_INVALIDATE ioctl to support such a cache invalidation.
It takes hwpt_id to specify the iommu_domain, and a multi-entry array to
support multiple invalidation data in one ioctl.

enum iommu_hwpt_invalidate_data_type is defined to tag the data type of
the entries in the multi-entry array.

Reviewed-by: Kevin Tian <[email protected]>
Co-developed-by: Nicolin Chen <[email protected]>
Signed-off-by: Nicolin Chen <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
drivers/iommu/iommufd/hw_pagetable.c | 41 +++++++++++++++++++++++
drivers/iommu/iommufd/iommufd_private.h | 10 ++++++
drivers/iommu/iommufd/main.c | 3 ++
include/uapi/linux/iommufd.h | 43 +++++++++++++++++++++++++
4 files changed, 97 insertions(+)

diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index cbb5df0a6c32..4e8711f19f72 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -371,3 +371,44 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd)
iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
return rc;
}
+
+int iommufd_hwpt_invalidate(struct iommufd_ucmd *ucmd)
+{
+ struct iommu_hwpt_invalidate *cmd = ucmd->cmd;
+ struct iommu_user_data_array data_array = {
+ .type = cmd->data_type,
+ .uptr = u64_to_user_ptr(cmd->data_uptr),
+ .entry_len = cmd->entry_len,
+ .entry_num = cmd->entry_num,
+ };
+ struct iommufd_hw_pagetable *hwpt;
+ u32 done_num = 0;
+ int rc;
+
+ if (cmd->__reserved) {
+ rc = -EOPNOTSUPP;
+ goto out;
+ }
+
+ if (cmd->entry_num && (!cmd->data_uptr || !cmd->entry_len)) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ hwpt = iommufd_get_hwpt_nested(ucmd, cmd->hwpt_id);
+ if (IS_ERR(hwpt)) {
+ rc = PTR_ERR(hwpt);
+ goto out;
+ }
+
+ rc = hwpt->domain->ops->cache_invalidate_user(hwpt->domain,
+ &data_array);
+ done_num = data_array.entry_num;
+
+ iommufd_put_object(ucmd->ictx, &hwpt->obj);
+out:
+ cmd->entry_num = done_num;
+ if (iommufd_ucmd_respond(ucmd, sizeof(*cmd)))
+ return -EFAULT;
+ return rc;
+}
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index abae041e256f..991f864d1f9b 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -328,6 +328,15 @@ iommufd_get_hwpt_paging(struct iommufd_ucmd *ucmd, u32 id)
IOMMUFD_OBJ_HWPT_PAGING),
struct iommufd_hwpt_paging, common.obj);
}
+
+static inline struct iommufd_hw_pagetable *
+iommufd_get_hwpt_nested(struct iommufd_ucmd *ucmd, u32 id)
+{
+ return container_of(iommufd_get_object(ucmd->ictx, id,
+ IOMMUFD_OBJ_HWPT_NESTED),
+ struct iommufd_hw_pagetable, obj);
+}
+
int iommufd_hwpt_set_dirty_tracking(struct iommufd_ucmd *ucmd);
int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd);

@@ -345,6 +354,7 @@ void iommufd_hwpt_paging_abort(struct iommufd_object *obj);
void iommufd_hwpt_nested_destroy(struct iommufd_object *obj);
void iommufd_hwpt_nested_abort(struct iommufd_object *obj);
int iommufd_hwpt_alloc(struct iommufd_ucmd *ucmd);
+int iommufd_hwpt_invalidate(struct iommufd_ucmd *ucmd);

static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
struct iommufd_hw_pagetable *hwpt)
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index c9091e46d208..39b32932c61e 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -322,6 +322,7 @@ union ucmd_buffer {
struct iommu_hw_info info;
struct iommu_hwpt_alloc hwpt;
struct iommu_hwpt_get_dirty_bitmap get_dirty_bitmap;
+ struct iommu_hwpt_invalidate cache;
struct iommu_hwpt_set_dirty_tracking set_dirty_tracking;
struct iommu_ioas_alloc alloc;
struct iommu_ioas_allow_iovas allow_iovas;
@@ -360,6 +361,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = {
__reserved),
IOCTL_OP(IOMMU_HWPT_GET_DIRTY_BITMAP, iommufd_hwpt_get_dirty_bitmap,
struct iommu_hwpt_get_dirty_bitmap, data),
+ IOCTL_OP(IOMMU_HWPT_INVALIDATE, iommufd_hwpt_invalidate,
+ struct iommu_hwpt_invalidate, __reserved),
IOCTL_OP(IOMMU_HWPT_SET_DIRTY_TRACKING, iommufd_hwpt_set_dirty_tracking,
struct iommu_hwpt_set_dirty_tracking, __reserved),
IOCTL_OP(IOMMU_IOAS_ALLOC, iommufd_ioas_alloc_ioctl,
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 0b2bc6252e2c..824560c50ec6 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -49,6 +49,7 @@ enum {
IOMMUFD_CMD_GET_HW_INFO,
IOMMUFD_CMD_HWPT_SET_DIRTY_TRACKING,
IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP,
+ IOMMUFD_CMD_HWPT_INVALIDATE,
};

/**
@@ -613,4 +614,46 @@ struct iommu_hwpt_get_dirty_bitmap {
#define IOMMU_HWPT_GET_DIRTY_BITMAP _IO(IOMMUFD_TYPE, \
IOMMUFD_CMD_HWPT_GET_DIRTY_BITMAP)

+/**
+ * enum iommu_hwpt_invalidate_data_type - IOMMU HWPT Cache Invalidation
+ * Data Type
+ * @IOMMU_HWPT_INVALIDATE_DATA_VTD_S1: Invalidation data for VTD_S1
+ */
+enum iommu_hwpt_invalidate_data_type {
+ IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
+};
+
+/**
+ * struct iommu_hwpt_invalidate - ioctl(IOMMU_HWPT_INVALIDATE)
+ * @size: sizeof(struct iommu_hwpt_invalidate)
+ * @hwpt_id: ID of a nested HWPT for cache invalidation
+ * @data_uptr: User pointer to an array of driver-specific cache invalidation
+ * data.
+ * @data_type: One of enum iommu_hwpt_invalidate_data_type, defining the data
+ * type of all the entries in the invalidation request array. It
+ * should be a type supported by the hwpt pointed by @hwpt_id.
+ * @entry_len: Length (in bytes) of a request entry in the request array
+ * @entry_num: Input the number of cache invalidation requests in the array.
+ * Output the number of requests successfully handled by kernel.
+ * @__reserved: Must be 0.
+ *
+ * Invalidate the iommu cache for user-managed page table. Modifications on a
+ * user-managed page table should be followed by this operation to sync cache.
+ * Each ioctl can support one or more cache invalidation requests in the array
+ * that has a total size of @entry_len * @entry_num.
+ *
+ * An empty invalidation request array by setting @entry_num==0 is allowed, and
+ * @entry_len and @data_uptr would be ignored in this case. This can be used to
+ * check if the given @data_type is supported or not by kernel.
+ */
+struct iommu_hwpt_invalidate {
+ __u32 size;
+ __u32 hwpt_id;
+ __aligned_u64 data_uptr;
+ __u32 data_type;
+ __u32 entry_len;
+ __u32 entry_num;
+ __u32 __reserved;
+};
+#define IOMMU_HWPT_INVALIDATE _IO(IOMMUFD_TYPE, IOMMUFD_CMD_HWPT_INVALIDATE)
#endif
--
2.34.1


2024-01-11 04:11:31

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 3/8] iommu: Add iommu_copy_struct_from_user_array helper

From: Nicolin Chen <[email protected]>

Wrap up the data pointer/num sanity and __iommu_copy_struct_from_user
call for iommu drivers to copy driver specific data at a specific location
in the struct iommu_user_data_array.

And expect it to be used in cache_invalidate_user ops for example.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Nicolin Chen <[email protected]>
Co-developed-by: Yi Liu <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
include/linux/iommu.h | 53 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 93c0d12dd047..580ba74a3d5d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -341,6 +341,59 @@ static inline int __iommu_copy_struct_from_user(
sizeof(*kdst), \
offsetofend(typeof(*kdst), min_last))

+/**
+ * __iommu_copy_struct_from_user_array - Copy iommu driver specific user space
+ * data from an iommu_user_data_array
+ * @dst_data: Pointer to an iommu driver specific user data that is defined in
+ * include/uapi/linux/iommufd.h
+ * @src_array: Pointer to a struct iommu_user_data_array for a user space array
+ * @data_type: The data type of the @dst_data. Must match with @src_array.type
+ * @index: Index to the location in the array to copy user data from
+ * @data_len: Length of current user data structure, i.e. sizeof(struct _dst)
+ * @min_len: Initial length of user data structure for backward compatibility.
+ * This should be offsetofend using the last member in the user data
+ * struct that was initially added to include/uapi/linux/iommufd.h
+ */
+static inline int
+__iommu_copy_struct_from_user_array(void *dst_data,
+ const struct iommu_user_data_array *src_array,
+ unsigned int data_type, unsigned int index,
+ size_t data_len, size_t min_len)
+{
+ struct iommu_user_data src_data;
+
+ if (WARN_ON(!src_array || index >= src_array->entry_num))
+ return -EINVAL;
+ if (!src_array->entry_num)
+ return -EINVAL;
+ src_data.uptr = src_array->uptr + src_array->entry_len * index;
+ src_data.len = src_array->entry_len;
+ src_data.type = src_array->type;
+
+ return __iommu_copy_struct_from_user(dst_data, &src_data, data_type,
+ data_len, min_len);
+}
+
+/**
+ * iommu_copy_struct_from_user_array - Copy iommu driver specific user space
+ * data from an iommu_user_data_array
+ * @kdst: Pointer to an iommu driver specific user data that is defined in
+ * include/uapi/linux/iommufd.h
+ * @user_array: Pointer to a struct iommu_user_data_array for a user space
+ * array
+ * @data_type: The data type of the @kdst. Must match with @user_array->type
+ * @index: Index to the location in the array to copy user data from
+ * @min_last: The last member of the data structure @kdst points in the
+ * initial version.
+ * Return 0 for success, otherwise -error.
+ */
+#define iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
+ index, min_last) \
+ __iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
+ index, sizeof(*kdst), \
+ offsetofend(typeof(*kdst), \
+ min_last))
+
/**
* struct iommu_ops - iommu ops and capabilities
* @capable: check capability
--
2.34.1


2024-01-11 04:11:51

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 4/8] iommufd/selftest: Add mock_domain_cache_invalidate_user support

From: Nicolin Chen <[email protected]>

Add mock_domain_cache_invalidate_user() data structure to support user
space selftest program to cover user cache invalidation pathway.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Nicolin Chen <[email protected]>
Co-developed-by: Yi Liu <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
drivers/iommu/iommufd/iommufd_test.h | 18 ++++++++++
drivers/iommu/iommufd/selftest.c | 50 ++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)

diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h
index 7910fbe1962d..09dfc8aa65c4 100644
--- a/drivers/iommu/iommufd/iommufd_test.h
+++ b/drivers/iommu/iommufd/iommufd_test.h
@@ -148,4 +148,22 @@ struct iommu_hwpt_selftest {
__u32 iotlb;
};

+/* Should not be equal to any defined value in enum iommu_hwpt_invalidate_data_type */
+#define IOMMU_HWPT_INVALIDATE_DATA_SELFTEST 0xdeadbeef
+#define IOMMU_HWPT_INVALIDATE_DATA_SELFTEST_INVALID 0xdadbeef
+
+/**
+ * struct iommu_hwpt_invalidate_selftest - Invalidation data for Mock driver
+ * (IOMMU_HWPT_INVALIDATE_DATA_SELFTEST)
+ * @flags: Invalidate flags
+ * @iotlb_id: Invalidate iotlb entry index
+ *
+ * If IOMMU_TEST_INVALIDATE_ALL is set in @flags, @iotlb_id will be ignored
+ */
+struct iommu_hwpt_invalidate_selftest {
+#define IOMMU_TEST_INVALIDATE_FLAG_ALL (1 << 0)
+ __u32 flags;
+ __u32 iotlb_id;
+};
+
#endif
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 022ef8f55088..23879135d1c3 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -473,9 +473,59 @@ static void mock_domain_free_nested(struct iommu_domain *domain)
kfree(mock_nested);
}

+static int
+mock_domain_cache_invalidate_user(struct iommu_domain *domain,
+ struct iommu_user_data_array *array)
+{
+ struct mock_iommu_domain_nested *mock_nested =
+ container_of(domain, struct mock_iommu_domain_nested, domain);
+ struct iommu_hwpt_invalidate_selftest inv;
+ u32 processed = 0;
+ int i = 0, j;
+ int rc = 0;
+
+ if (array->type != IOMMU_HWPT_INVALIDATE_DATA_SELFTEST) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ for ( ; i < array->entry_num; i++) {
+ rc = iommu_copy_struct_from_user_array(&inv, array,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ i, iotlb_id);
+ if (rc)
+ break;
+
+ if (inv.flags & ~IOMMU_TEST_INVALIDATE_FLAG_ALL) {
+ rc = -EOPNOTSUPP;
+ break;
+ }
+
+ if (inv.iotlb_id > MOCK_NESTED_DOMAIN_IOTLB_ID_MAX) {
+ rc = -EINVAL;
+ break;
+ }
+
+ if (inv.flags & IOMMU_TEST_INVALIDATE_FLAG_ALL) {
+ /* Invalidate all mock iotlb entries and ignore iotlb_id */
+ for (j = 0; j < MOCK_NESTED_DOMAIN_IOTLB_NUM; j++)
+ mock_nested->iotlb[j] = 0;
+ } else {
+ mock_nested->iotlb[inv.iotlb_id] = 0;
+ }
+
+ processed++;
+ }
+
+out:
+ array->entry_num = processed;
+ return rc;
+}
+
static struct iommu_domain_ops domain_nested_ops = {
.free = mock_domain_free_nested,
.attach_dev = mock_domain_nop_attach,
+ .cache_invalidate_user = mock_domain_cache_invalidate_user,
};

static inline struct iommufd_hw_pagetable *
--
2.34.1


2024-01-11 04:12:19

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 5/8] iommufd/selftest: Add IOMMU_TEST_OP_MD_CHECK_IOTLB test op

From: Nicolin Chen <[email protected]>

Allow to test whether IOTLB has been invalidated or not.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Nicolin Chen <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
drivers/iommu/iommufd/iommufd_test.h | 5 ++++
drivers/iommu/iommufd/selftest.c | 26 +++++++++++++++++++
tools/testing/selftests/iommu/iommufd.c | 4 +++
tools/testing/selftests/iommu/iommufd_utils.h | 24 +++++++++++++++++
4 files changed, 59 insertions(+)

diff --git a/drivers/iommu/iommufd/iommufd_test.h b/drivers/iommu/iommufd/iommufd_test.h
index 09dfc8aa65c4..482d4059f5db 100644
--- a/drivers/iommu/iommufd/iommufd_test.h
+++ b/drivers/iommu/iommufd/iommufd_test.h
@@ -21,6 +21,7 @@ enum {
IOMMU_TEST_OP_ACCESS_REPLACE_IOAS,
IOMMU_TEST_OP_MOCK_DOMAIN_FLAGS,
IOMMU_TEST_OP_DIRTY,
+ IOMMU_TEST_OP_MD_CHECK_IOTLB,
};

enum {
@@ -121,6 +122,10 @@ struct iommu_test_cmd {
__aligned_u64 uptr;
__aligned_u64 out_nr_dirty;
} dirty;
+ struct {
+ __u32 id;
+ __u32 iotlb;
+ } check_iotlb;
};
__u32 last;
};
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index 23879135d1c3..db648c81507f 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -843,6 +843,28 @@ static int iommufd_test_md_check_refs(struct iommufd_ucmd *ucmd,
return 0;
}

+static int iommufd_test_md_check_iotlb(struct iommufd_ucmd *ucmd,
+ u32 mockpt_id, unsigned int iotlb_id,
+ u32 iotlb)
+{
+ struct mock_iommu_domain_nested *mock_nested;
+ struct iommufd_hw_pagetable *hwpt;
+ int rc = 0;
+
+ hwpt = get_md_pagetable_nested(ucmd, mockpt_id, &mock_nested);
+ if (IS_ERR(hwpt))
+ return PTR_ERR(hwpt);
+
+ mock_nested = container_of(hwpt->domain,
+ struct mock_iommu_domain_nested, domain);
+
+ if (iotlb_id > MOCK_NESTED_DOMAIN_IOTLB_ID_MAX ||
+ mock_nested->iotlb[iotlb_id] != iotlb)
+ rc = -EINVAL;
+ iommufd_put_object(ucmd->ictx, &hwpt->obj);
+ return rc;
+}
+
struct selftest_access {
struct iommufd_access *access;
struct file *file;
@@ -1324,6 +1346,10 @@ int iommufd_test(struct iommufd_ucmd *ucmd)
return iommufd_test_md_check_refs(
ucmd, u64_to_user_ptr(cmd->check_refs.uptr),
cmd->check_refs.length, cmd->check_refs.refs);
+ case IOMMU_TEST_OP_MD_CHECK_IOTLB:
+ return iommufd_test_md_check_iotlb(ucmd, cmd->id,
+ cmd->check_iotlb.id,
+ cmd->check_iotlb.iotlb);
case IOMMU_TEST_OP_CREATE_ACCESS:
return iommufd_test_create_access(ucmd, cmd->id,
cmd->create_access.flags);
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 6ed328c863c4..c8763b880a16 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -330,6 +330,10 @@ TEST_F(iommufd_ioas, alloc_hwpt_nested)
&nested_hwpt_id[1],
IOMMU_HWPT_DATA_SELFTEST, &data,
sizeof(data));
+ test_cmd_hwpt_check_iotlb_all(nested_hwpt_id[0],
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb_all(nested_hwpt_id[1],
+ IOMMU_TEST_IOTLB_DEFAULT);

/* Negative test: a nested hwpt on top of a nested hwpt */
test_err_hwpt_alloc_nested(EINVAL, self->device_id,
diff --git a/tools/testing/selftests/iommu/iommufd_utils.h b/tools/testing/selftests/iommu/iommufd_utils.h
index ad9202335656..fe0a0f566b67 100644
--- a/tools/testing/selftests/iommu/iommufd_utils.h
+++ b/tools/testing/selftests/iommu/iommufd_utils.h
@@ -195,6 +195,30 @@ static int _test_cmd_hwpt_alloc(int fd, __u32 device_id, __u32 pt_id,
_test_cmd_hwpt_alloc(self->fd, device_id, pt_id, flags, \
hwpt_id, data_type, data, data_len))

+#define test_cmd_hwpt_check_iotlb(hwpt_id, iotlb_id, expected) \
+ ({ \
+ struct iommu_test_cmd test_cmd = { \
+ .size = sizeof(test_cmd), \
+ .op = IOMMU_TEST_OP_MD_CHECK_IOTLB, \
+ .id = hwpt_id, \
+ .check_iotlb = { \
+ .id = iotlb_id, \
+ .iotlb = expected, \
+ }, \
+ }; \
+ ASSERT_EQ(0, \
+ ioctl(self->fd, \
+ _IOMMU_TEST_CMD(IOMMU_TEST_OP_MD_CHECK_IOTLB), \
+ &test_cmd)); \
+ })
+
+#define test_cmd_hwpt_check_iotlb_all(hwpt_id, expected) \
+ ({ \
+ int i; \
+ for (i = 0; i < MOCK_NESTED_DOMAIN_IOTLB_NUM; i++) \
+ test_cmd_hwpt_check_iotlb(hwpt_id, i, expected); \
+ })
+
static int _test_cmd_access_replace_ioas(int fd, __u32 access_id,
unsigned int ioas_id)
{
--
2.34.1


2024-01-11 04:12:51

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 6/8] iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl

From: Nicolin Chen <[email protected]>

Add test cases for the IOMMU_HWPT_INVALIDATE ioctl and verify it by using
the new IOMMU_TEST_OP_MD_CHECK_IOTLB.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Nicolin Chen <[email protected]>
Co-developed-by: Yi Liu <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
tools/testing/selftests/iommu/iommufd.c | 148 ++++++++++++++++++
tools/testing/selftests/iommu/iommufd_utils.h | 33 ++++
2 files changed, 181 insertions(+)

diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index c8763b880a16..1a881e7a21d1 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -116,6 +116,7 @@ TEST_F(iommufd, cmd_length)
TEST_LENGTH(iommu_destroy, IOMMU_DESTROY, id);
TEST_LENGTH(iommu_hw_info, IOMMU_GET_HW_INFO, __reserved);
TEST_LENGTH(iommu_hwpt_alloc, IOMMU_HWPT_ALLOC, __reserved);
+ TEST_LENGTH(iommu_hwpt_invalidate, IOMMU_HWPT_INVALIDATE, __reserved);
TEST_LENGTH(iommu_ioas_alloc, IOMMU_IOAS_ALLOC, out_ioas_id);
TEST_LENGTH(iommu_ioas_iova_ranges, IOMMU_IOAS_IOVA_RANGES,
out_iova_alignment);
@@ -271,7 +272,9 @@ TEST_F(iommufd_ioas, alloc_hwpt_nested)
struct iommu_hwpt_selftest data = {
.iotlb = IOMMU_TEST_IOTLB_DEFAULT,
};
+ struct iommu_hwpt_invalidate_selftest inv_reqs[2] = {};
uint32_t nested_hwpt_id[2] = {};
+ uint32_t num_inv;
uint32_t parent_hwpt_id = 0;
uint32_t parent_hwpt_id_not_work = 0;
uint32_t test_hwpt_id = 0;
@@ -344,6 +347,151 @@ TEST_F(iommufd_ioas, alloc_hwpt_nested)
EXPECT_ERRNO(EBUSY,
_test_ioctl_destroy(self->fd, parent_hwpt_id));

+ /* hwpt_invalidate only supports a user-managed hwpt (nested) */
+ num_inv = 1;
+ test_err_hwpt_invalidate(ENOENT, parent_hwpt_id, inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /* Check data_type by passing zero-length array */
+ num_inv = 0;
+ test_cmd_hwpt_invalidate(nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: Invalid data_type */
+ num_inv = 1;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST_INVALID,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: structure size sanity */
+ num_inv = 1;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs) + 1, &num_inv);
+ assert(!num_inv);
+
+ num_inv = 1;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ 1, &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: invalid flag is passed */
+ num_inv = 1;
+ inv_reqs[0].flags = 0xffffffff;
+ test_err_hwpt_invalidate(EOPNOTSUPP, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: invalid data_uptr when array is not empty */
+ num_inv = 1;
+ inv_reqs[0].flags = 0;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], NULL,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: invalid entry_len when array is not empty */
+ num_inv = 1;
+ inv_reqs[0].flags = 0;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ 0, &num_inv);
+ assert(!num_inv);
+
+ /* Negative test: invalid iotlb_id */
+ num_inv = 1;
+ inv_reqs[0].flags = 0;
+ inv_reqs[0].iotlb_id = MOCK_NESTED_DOMAIN_IOTLB_ID_MAX + 1;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(!num_inv);
+
+ /*
+ * Invalidate the 1st iotlb entry but fail the 2nd request
+ * due to invalid flags configuration in the 2nd request.
+ */
+ num_inv = 2;
+ inv_reqs[0].flags = 0;
+ inv_reqs[0].iotlb_id = 0;
+ inv_reqs[1].flags = 0xffffffff;
+ inv_reqs[1].iotlb_id = 1;
+ test_err_hwpt_invalidate(EOPNOTSUPP, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(num_inv == 1);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 0, 0);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 1,
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 2,
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 3,
+ IOMMU_TEST_IOTLB_DEFAULT);
+
+ /*
+ * Invalidate the 1st iotlb entry but fail the 2nd request
+ * due to invalid iotlb_id configuration in the 2nd request.
+ */
+ num_inv = 2;
+ inv_reqs[0].flags = 0;
+ inv_reqs[0].iotlb_id = 0;
+ inv_reqs[1].flags = 0;
+ inv_reqs[1].iotlb_id = MOCK_NESTED_DOMAIN_IOTLB_ID_MAX + 1;
+ test_err_hwpt_invalidate(EINVAL, nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(num_inv == 1);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 0, 0);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 1,
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 2,
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 3,
+ IOMMU_TEST_IOTLB_DEFAULT);
+
+ /* Invalidate the 2nd iotlb entry and verify */
+ num_inv = 1;
+ inv_reqs[0].flags = 0;
+ inv_reqs[0].iotlb_id = 1;
+ test_cmd_hwpt_invalidate(nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(num_inv == 1);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 0, 0);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 1, 0);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 2,
+ IOMMU_TEST_IOTLB_DEFAULT);
+ test_cmd_hwpt_check_iotlb(nested_hwpt_id[0], 3,
+ IOMMU_TEST_IOTLB_DEFAULT);
+
+ /* Invalidate the 3rd and 4th iotlb entries and verify */
+ num_inv = 2;
+ inv_reqs[0].flags = 0;
+ inv_reqs[0].iotlb_id = 2;
+ inv_reqs[1].flags = 0;
+ inv_reqs[1].iotlb_id = 3;
+ test_cmd_hwpt_invalidate(nested_hwpt_id[0], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(num_inv == 2);
+ test_cmd_hwpt_check_iotlb_all(nested_hwpt_id[0], 0);
+
+ /* Invalidate all iotlb entries for nested_hwpt_id[1] and verify */
+ num_inv = 1;
+ inv_reqs[0].flags = IOMMU_TEST_INVALIDATE_FLAG_ALL;
+ test_cmd_hwpt_invalidate(nested_hwpt_id[1], inv_reqs,
+ IOMMU_HWPT_INVALIDATE_DATA_SELFTEST,
+ sizeof(*inv_reqs), &num_inv);
+ assert(num_inv == 1);
+ test_cmd_hwpt_check_iotlb_all(nested_hwpt_id[1], 0);
+
/* Attach device to nested_hwpt_id[0] that then will be busy */
test_cmd_mock_domain_replace(self->stdev_id, nested_hwpt_id[0]);
EXPECT_ERRNO(EBUSY,
diff --git a/tools/testing/selftests/iommu/iommufd_utils.h b/tools/testing/selftests/iommu/iommufd_utils.h
index fe0a0f566b67..7f41fb796a8a 100644
--- a/tools/testing/selftests/iommu/iommufd_utils.h
+++ b/tools/testing/selftests/iommu/iommufd_utils.h
@@ -219,6 +219,39 @@ static int _test_cmd_hwpt_alloc(int fd, __u32 device_id, __u32 pt_id,
test_cmd_hwpt_check_iotlb(hwpt_id, i, expected); \
})

+static int _test_cmd_hwpt_invalidate(int fd, __u32 hwpt_id, void *reqs,
+ uint32_t data_type, uint32_t lreq,
+ uint32_t *nreqs)
+{
+ struct iommu_hwpt_invalidate cmd = {
+ .size = sizeof(cmd),
+ .hwpt_id = hwpt_id,
+ .data_type = data_type,
+ .data_uptr = (uint64_t)reqs,
+ .entry_len = lreq,
+ .entry_num = *nreqs,
+ };
+ int rc = ioctl(fd, IOMMU_HWPT_INVALIDATE, &cmd);
+ *nreqs = cmd.entry_num;
+ return rc;
+}
+
+#define test_cmd_hwpt_invalidate(hwpt_id, reqs, data_type, lreq, nreqs) \
+ ({ \
+ ASSERT_EQ(0, \
+ _test_cmd_hwpt_invalidate(self->fd, hwpt_id, reqs, \
+ data_type, \
+ lreq, nreqs)); \
+ })
+#define test_err_hwpt_invalidate(_errno, hwpt_id, reqs, data_type, lreq, \
+ nreqs) \
+ ({ \
+ EXPECT_ERRNO(_errno, \
+ _test_cmd_hwpt_invalidate(self->fd, hwpt_id, \
+ reqs, data_type, \
+ lreq, nreqs)); \
+ })
+
static int _test_cmd_access_replace_ioas(int fd, __u32 access_id,
unsigned int ioas_id)
{
--
2.34.1


2024-01-11 04:20:28

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 7/8] iommufd: Add data structure for Intel VT-d stage-1 cache invalidation

This adds the data structure invalidating caches for the nested domain
allocated with IOMMU_HWPT_DATA_VTD_S1 type.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
include/uapi/linux/iommufd.h | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)

diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 824560c50ec6..1dfeaa2e649e 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -623,6 +623,42 @@ enum iommu_hwpt_invalidate_data_type {
IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
};

+/**
+ * enum iommu_hwpt_vtd_s1_invalidate_flags - Flags for Intel VT-d
+ * stage-1 cache invalidation
+ * @IOMMU_VTD_INV_FLAGS_LEAF: Indicates whether the invalidation applies
+ * to all-levels page structure cache or just
+ * the leaf PTE cache.
+ */
+enum iommu_hwpt_vtd_s1_invalidate_flags {
+ IOMMU_VTD_INV_FLAGS_LEAF = 1 << 0,
+};
+
+/**
+ * struct iommu_hwpt_vtd_s1_invalidate - Intel VT-d cache invalidation
+ * (IOMMU_HWPT_INVALIDATE_DATA_VTD_S1)
+ * @addr: The start address of the range to be invalidated. It needs to
+ * be 4KB aligned.
+ * @npages: Number of contiguous 4K pages to be invalidated.
+ * @flags: Combination of enum iommu_hwpt_vtd_s1_invalidate_flags
+ * @__reserved: Must be 0
+ *
+ * The Intel VT-d specific invalidation data for user-managed stage-1 cache
+ * invalidation in nested translation. Userspace uses this structure to
+ * tell the impacted cache scope after modifying the stage-1 page table.
+ *
+ * Invalidating all the caches related to the page table by setting @addr
+ * to be 0 and @npages to be U64_MAX.
+ *
+ * The device TLB will be invalidated automatically if ATS is enabled.
+ */
+struct iommu_hwpt_vtd_s1_invalidate {
+ __aligned_u64 addr;
+ __aligned_u64 npages;
+ __u32 flags;
+ __u32 __reserved;
+};
+
/**
* struct iommu_hwpt_invalidate - ioctl(IOMMU_HWPT_INVALIDATE)
* @size: sizeof(struct iommu_hwpt_invalidate)
--
2.34.1


2024-01-11 04:21:06

by Yi Liu

[permalink] [raw]
Subject: [PATCH v11 8/8] iommu/vt-d: Add iotlb flush for nested domain

From: Lu Baolu <[email protected]>

This implements the .cache_invalidate_user() callback to support iotlb
flush for nested domain.

Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
Co-developed-by: Yi Liu <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
drivers/iommu/intel/nested.c | 88 ++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)

diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index b5a5563ab32c..1e866ed02fef 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -73,9 +73,97 @@ static void intel_nested_domain_free(struct iommu_domain *domain)
kfree(to_dmar_domain(domain));
}

+static void nested_flush_dev_iotlb(struct dmar_domain *domain, u64 addr,
+ unsigned mask)
+{
+ struct device_domain_info *info;
+ unsigned long flags;
+ u16 sid, qdep;
+
+ spin_lock_irqsave(&domain->lock, flags);
+ list_for_each_entry(info, &domain->devices, link) {
+ if (!info->ats_enabled)
+ continue;
+ sid = info->bus << 8 | info->devfn;
+ qdep = info->ats_qdep;
+ qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+ qdep, addr, mask);
+ quirk_extra_dev_tlb_flush(info, addr, mask,
+ IOMMU_NO_PASID, qdep);
+ }
+ spin_unlock_irqrestore(&domain->lock, flags);
+}
+
+static void intel_nested_flush_cache(struct dmar_domain *domain, u64 addr,
+ unsigned long npages, bool ih)
+{
+ struct iommu_domain_info *info;
+ unsigned long i;
+ unsigned mask;
+
+ xa_for_each(&domain->iommu_array, i, info)
+ qi_flush_piotlb(info->iommu,
+ domain_id_iommu(domain, info->iommu),
+ IOMMU_NO_PASID, addr, npages, ih);
+
+ if (!domain->has_iotlb_device)
+ return;
+
+ if (npages == U64_MAX)
+ mask = 64 - VTD_PAGE_SHIFT;
+ else
+ mask = ilog2(__roundup_pow_of_two(npages));
+
+ nested_flush_dev_iotlb(domain, addr, mask);
+}
+
+static int intel_nested_cache_invalidate_user(struct iommu_domain *domain,
+ struct iommu_user_data_array *array)
+{
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct iommu_hwpt_vtd_s1_invalidate inv_entry;
+ u32 index, processed = 0;
+ int ret = 0;
+
+ if (array->type != IOMMU_HWPT_INVALIDATE_DATA_VTD_S1) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ for (index = 0; index < array->entry_num; index++) {
+ ret = iommu_copy_struct_from_user_array(&inv_entry, array,
+ IOMMU_HWPT_INVALIDATE_DATA_VTD_S1,
+ index, __reserved);
+ if (ret)
+ break;
+
+ if ((inv_entry.flags & ~IOMMU_VTD_INV_FLAGS_LEAF) ||
+ inv_entry.__reserved) {
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ if (!IS_ALIGNED(inv_entry.addr, VTD_PAGE_SIZE) ||
+ ((inv_entry.npages == U64_MAX) && inv_entry.addr)) {
+ ret = -EINVAL;
+ break;
+ }
+
+ intel_nested_flush_cache(dmar_domain, inv_entry.addr,
+ inv_entry.npages,
+ inv_entry.flags & IOMMU_VTD_INV_FLAGS_LEAF);
+ processed++;
+ }
+
+out:
+ array->entry_num = processed;
+ return ret;
+}
+
static const struct iommu_domain_ops intel_nested_domain_ops = {
.attach_dev = intel_nested_attach_dev,
.free = intel_nested_domain_free,
+ .cache_invalidate_user = intel_nested_cache_invalidate_user,
};

struct iommu_domain *intel_nested_domain_alloc(struct iommu_domain *parent,
--
2.34.1


2024-01-11 05:51:39

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH v11 0/8] Add iommufd nesting (part 2/2)

> From: Yi Liu <[email protected]>
> Sent: Thursday, January 11, 2024 12:10 PM
>
> v11:
> - Drop hw_error field in vtd cache invalidation uapi. devTLB invalidation
> error is a serious security emergency requiring the host kernel to handle.
> No need to expose it to userspace (especially given existing VMs doesn't
> issue devTLB invalidation at all).
> - The vtd qi_submit_sync() and related callers are reverted back to the
> original state due to above drop.
> - Align with the vtd path, drop the hw_error reporting in mock driver and
> selftest as well since selftest is a demo of the real driver.
> - Drop iommu_respond_struct_to_user_array() since no more driver want to
> respond single entry in the user_array.
> - Two typos from Wubinbin
>

this looks good to me. so my r-b still holds.

2024-01-11 16:55:45

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v11 6/8] iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl

On Thu, Jan 11, 2024 at 08:50:45AM -0800, Nicolin Chen wrote:
> On Wed, Jan 10, 2024 at 08:10:13PM -0800, Yi Liu wrote:
> > +#define test_cmd_hwpt_invalidate(hwpt_id, reqs, data_type, lreq, nreqs) \
> > + ({ \
> > + ASSERT_EQ(0, \
> > + _test_cmd_hwpt_invalidate(self->fd, hwpt_id, reqs, \
> > + data_type, \
> > + lreq, nreqs)); \
> > + })
> > +#define test_err_hwpt_invalidate(_errno, hwpt_id, reqs, data_type, lreq, \
> > + nreqs) \
> > + ({ \
> > + EXPECT_ERRNO(_errno, \
> > + _test_cmd_hwpt_invalidate(self->fd, hwpt_id, \
> > + reqs, data_type, \
> > + lreq, nreqs)); \
> > + })
>
> Nit: spaces at the end of the four lines above that have string
> "data_type" are all misaligned, probably because of the previous
> replacement of "req_type".

I will fix it

Jason

2024-01-11 16:57:05

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH v11 3/8] iommu: Add iommu_copy_struct_from_user_array helper

On Wed, Jan 10, 2024 at 08:10:10PM -0800, Yi Liu wrote:

> +#define iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
> + index, min_last) \
> + __iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
> + index, sizeof(*kdst), \
> + offsetofend(typeof(*kdst), \
> + min_last))

There is an extra space before '\' at the 2nd line, making it:

390 #define iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
391 index, min_last) \
392 __iommu_copy_struct_from_user_array(kdst, user_array, data_type, \
393 index, sizeof(*kdst), \
394 offsetofend(typeof(*kdst), \
395 min_last))

2024-01-11 17:31:00

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH v11 6/8] iommufd/selftest: Add coverage for IOMMU_HWPT_INVALIDATE ioctl

On Wed, Jan 10, 2024 at 08:10:13PM -0800, Yi Liu wrote:
> +#define test_cmd_hwpt_invalidate(hwpt_id, reqs, data_type, lreq, nreqs) \
> + ({ \
> + ASSERT_EQ(0, \
> + _test_cmd_hwpt_invalidate(self->fd, hwpt_id, reqs, \
> + data_type, \
> + lreq, nreqs)); \
> + })
> +#define test_err_hwpt_invalidate(_errno, hwpt_id, reqs, data_type, lreq, \
> + nreqs) \
> + ({ \
> + EXPECT_ERRNO(_errno, \
> + _test_cmd_hwpt_invalidate(self->fd, hwpt_id, \
> + reqs, data_type, \
> + lreq, nreqs)); \
> + })

Nit: spaces at the end of the four lines above that have string
"data_type" are all misaligned, probably because of the previous
replacement of "req_type".

2024-01-11 19:57:05

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH v11 0/8] Add iommufd nesting (part 2/2)

On Wed, Jan 10, 2024 at 08:10:07PM -0800, Yi Liu wrote:
> v11:
> - Drop hw_error field in vtd cache invalidation uapi. devTLB invalidation
> error is a serious security emergency requiring the host kernel to handle.
> No need to expose it to userspace (especially given existing VMs doesn't
> issue devTLB invalidation at all).
> - The vtd qi_submit_sync() and related callers are reverted back to the
> original state due to above drop.
> - Align with the vtd path, drop the hw_error reporting in mock driver and
> selftest as well since selftest is a demo of the real driver.
> - Drop iommu_respond_struct_to_user_array() since no more driver want to
> respond single entry in the user_array.
> - Two typos from Wubinbin

Okay, this is very last minute but I updated the series.

Jason

2024-01-16 03:04:13

by Zhenzhong Duan

[permalink] [raw]
Subject: RE: [PATCH v11 0/8] Add iommufd nesting (part 2/2)



>-----Original Message-----
>From: Liu, Yi L <[email protected]>
>Subject: [PATCH v11 0/8] Add iommufd nesting (part 2/2)
>
>Nested translation is a hardware feature that is supported by many modern
>IOMMU hardwares. It has two stages (stage-1, stage-2) address translation
>to get access to the physical address. stage-1 translation table is owned
>by userspace (e.g. by a guest OS), while stage-2 is owned by kernel. Changes
>to stage-1 translation table should be followed by an IOTLB invalidation.
>
>Take Intel VT-d as an example, the stage-1 translation table is I/O page
>table. As the below diagram shows, guest I/O page table pointer in GPA
>(guest physical address) is passed to host and be used to perform the stage-
>1
>address translation. Along with it, modifications to present mappings in the
>guest I/O page table should be followed with an IOTLB invalidation.
>
> .-------------. .---------------------------.
> | vIOMMU | | Guest I/O page table |
> | | '---------------------------'
> .----------------/
> | PASID Entry |--- PASID cache flush --+
> '-------------' |
> | | V
> | | I/O page table pointer in GPA
> '-------------'
>Guest
>------| Shadow |---------------------------|--------
> v v v
>Host
> .-------------. .------------------------.
> | pIOMMU | | FS for GIOVA->GPA |
> | | '------------------------'
> .----------------/ |
> | PASID Entry | V (Nested xlate)
> '----------------\.----------------------------------.
> | | | SS for GPA->HPA, unmanaged domain|
> | | '----------------------------------'
> '-------------'
>Where:
> - FS = First stage page tables
> - SS = Second stage page tables
><Intel VT-d Nested translation>
>
>This series is based on the first part which was merged [1], this series is to
>add the cache invalidation interface or the userspace to invalidate cache
>after
>modifying the stage-1 page table. This includes both the iommufd changes
>and the
>VT-d driver changes.
>
>Complete code can be found in [2], QEMU could can be found in [3].
>
>At last, this is a team work together with Nicolin Chen, Lu Baolu. Thanks
>them for the help. ^_^. Look forward to your feedbacks.
>
>[1] https://lore.kernel.org/linux-iommu/20231026044216.64964-1-
>[email protected]/ - merged
>[2] https://github.com/yiliu1765/iommufd/tree/iommufd_nesting
>[3]
>https://github.com/yiliu1765/qemu/tree/zhenzhong/wip/iommufd_nesting
>_rfcv1

This series is tested with a real implementation of QEMU at
https://lists.gnu.org/archive/html/qemu-devel/2024-01/msg02740.html

May be late, but still

Tested-by: Zhenzhong Duan <[email protected]>

Thanks
Zhenzhong