iommufd: Prepare for IOMMUFD_OBJ_HW_QUEUE
Subsystems (3)
v2 cover iommufd: Prepare for IOMMUFD_OBJ_HW_QUEUE
The new HW Queue object will require more interactions with IOMMU drivers, with a few more for-driver APIs. This will complicate the driver-allocated structure design like the viommu_alloc op: since the core structure is not initialized during the driver allocation stage, a new for-driver API can't reference any member in the core vIOMMU structure.
[before]
core: viommu = ops->viommu_alloc();
driver: // my_viommu is successfully allocated
driver: my_viommu = iommufd_viommu_alloc(...);
driver: // This may crash if it reads viommu->ictx
driver: new = iommufd_new_viommu_helper(my_viommu->core ...);
core: viommu->ictx = ucmd->ictx;
core: ...
Make a preparatory series doing:
1 Replace viommu_alloc design with get_viommu_size + viommu_init 2 Add a new iommufd_object_alloc_ucmd
3 Cosmetic fixes and clean ups
[after 1]
core: viommu = ops->get_viommu_size();
driver: return VIOMMU_STRUCT_SIZE();
core: viommu->ictx = ucmd->ictx; // and others
core: rc = ops->viommu_init();
driver: // This is safe now as viommu->ictx is inited
driver: new = iommufd_new_viommu_helper(my_viommu->core ...);
core: ...
Some of the patches are included from:
[PATCH v5 00/29] iommufd: Add vIOMMU infrastructure (Part-4 HW QUEUE)
https://lore.kernel.org/all/[email protected]/
This is on Github:
https://github.com/nicolinc/iommufd/commits/iommufd_hw_queue-prep-v2
Changelog
v2
* Add Reviewed-by from Kevin and Jason
* Drop unused mock_nested->parent
* Revise commit messages and kdocs
* Add WARN_ON if new_obj is already set
* Re-organize the patches replacing viommu_alloc
* Use EOPNOTSUPP for failures due to driver bugs
* Return size_t for get_viommu_size op (0 means EOPNOTSUPP)
v1
https://lore.kernel.org/all/[email protected]/
Thanks
Nicolin
Nicolin Chen (14):
iommufd: Apply obvious cosmetic fixes
iommufd: Drop unused ictx in struct iommufd_vdevice
iommufd: Use enum iommu_viommu_type for type in struct iommufd_viommu
iommufd: Use enum iommu_veventq_type for type in struct
iommufd_veventq
iommufd: Return EOPNOTSUPP for failures due to driver bugs
iommu: Introduce get_viommu_size and viommu_init ops iommufd/viommu: Support get_viommu_size and viommu_init ops iommufd/selftest: Drop parent domain from mock_iommu_domain_nested iommufd/selftest: Replace mock_viommu_alloc with mock_viommu_init iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
iommu: Deprecate viommu_alloc op
iommufd: Move _iommufd_object_alloc out of driver.c
iommufd: Introduce iommufd_object_alloc_ucmd helper
iommufd: Apply the new iommufd_object_alloc_ucmd helper
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 11 ++-- drivers/iommu/iommufd/io_pagetable.h | 2 +- drivers/iommu/iommufd/iommufd_private.h | 42 ++++++++++--- include/linux/iommu.h | 26 ++++---- include/linux/iommufd.h | 39 +++--------- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 46 +++++++------- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +- drivers/iommu/iommufd/device.c | 5 +- drivers/iommu/iommufd/driver.c | 33 ---------- drivers/iommu/iommufd/eventq.c | 14 ++--- drivers/iommu/iommufd/hw_pagetable.c | 10 ++- drivers/iommu/iommufd/io_pagetable.c | 3 +- drivers/iommu/iommufd/iova_bitmap.c | 1 - drivers/iommu/iommufd/main.c | 63 +++++++++++++++++-- drivers/iommu/iommufd/pages.c | 9 ++- drivers/iommu/iommufd/selftest.c | 56 ++++++++--------- drivers/iommu/iommufd/viommu.c | 48 +++++++++----- 17 files changed, 222 insertions(+), 189 deletions(-)
Signature · 1 line
2.43.0
PATCH v2 1/14 iommufd: Apply obvious cosmetic fixes
Run clang-format but exclude those not so obvious ones, which leaves us:
- Align indentations
- Add missing spaces
- Remove unnecessary spaces
- Remove unnecessary line wrappings
--- drivers/iommu/iommufd/io_pagetable.h | 2 +- drivers/iommu/iommufd/iommufd_private.h | 6 ++---- include/linux/iommufd.h | 5 +++-- drivers/iommu/iommufd/device.c | 3 +-- drivers/iommu/iommufd/hw_pagetable.c | 6 ++---- drivers/iommu/iommufd/io_pagetable.c | 3 +-- drivers/iommu/iommufd/iova_bitmap.c | 1 - drivers/iommu/iommufd/main.c | 6 ++---- drivers/iommu/iommufd/pages.c | 9 ++++----- drivers/iommu/iommufd/selftest.c | 24 +++++++++++------------- 10 files changed, 27 insertions(+), 38 deletions(-)
drivers/iommu/iommufd/io_pagetable.h +1 -1
@@ -240,7 +240,7 @@ int iopt_area_add_access(struct iopt_area *area, unsigned long start, unsigned long last, struct page **out_pages, unsigned int flags);void iopt_area_remove_access (struct iopt_area *area,unsigned long start, -unsigned long last); +unsigned long last);int iopt_pages_rw_access (struct iopt_pages *pages,unsigned long start_byte,void *data,unsigned long length,unsigned int flags);
drivers/iommu/iommufd/iommufd_private.h +2 -4
@@ -266,8 +266,7 @@ struct iommufd_ioas {static inline struct iommufd_ioas *iommufd_get_ioas (struct iommufd_ctx *ictx,u32 id) { -return container_of (iommufd_get_object (ictx, id, -IOMMUFD_OBJ_IOAS ), +return container_of (iommufd_get_object (ictx, id,IOMMUFD_OBJ_IOAS ), struct iommufd_ioas, obj); }
@@ -452,8 +451,7 @@ struct iommufd_access {int iopt_add_access (struct io_pagetable *iopt,struct iommufd_access *access);void iopt_remove_access (struct io_pagetable *iopt, -struct iommufd_access *access, -u32 iopt_access_list_id); +struct iommufd_access *access,u32 iopt_access_list_id);void iommufd_access_destroy_object (struct iommufd_object *obj);struct iommufd_eventq {
include/linux/iommufd.h +3 -2
@@ -171,8 +171,9 @@ static inline void iommufd_access_unpin_pages(struct iommufd_access *access, { } -static inline int iommufd_access_rw (struct iommufd_access *access,unsigned long iova, -void *data,size_t len,unsigned int flags) +static inline int iommufd_access_rw (struct iommufd_access *access, +unsigned long iova,void *data,size_t len, +unsigned int flags) {return -EOPNOTSUPP ; }
drivers/iommu/iommufd/device.c +1 -2
@@ -485,8 +485,7 @@ iommufd_device_get_attach_handle(struct iommufd_device *idev, ioasid_t pasid)lockdep_assert_held (&idev->igroup->lock); - handle = -iommu_attach_handle_get (idev->igroup->group, pasid,0 ); + handle =iommu_attach_handle_get (idev->igroup->group, pasid,0 );if (IS_ERR (handle))return NULL ;return to_iommufd_handle (handle);
drivers/iommu/iommufd/hw_pagetable.c +2 -4
@@ -309,10 +309,8 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags,refcount_inc (&viommu->obj.users); hwpt_nested->parent = viommu->hwpt; - hwpt->domain = - viommu->ops->alloc_domain_nested (viommu, - flags & ~IOMMU_HWPT_FAULT_ID_VALID , - user_data); + hwpt->domain = viommu->ops->alloc_domain_nested ( + viommu, flags & ~IOMMU_HWPT_FAULT_ID_VALID , user_data);if (IS_ERR (hwpt->domain)) { rc =PTR_ERR (hwpt->domain); hwpt->domain =NULL ;
drivers/iommu/iommufd/io_pagetable.c +1 -2
@@ -1410,8 +1410,7 @@ int iopt_add_access(struct io_pagetable *iopt, struct iommufd_access *access) }void iopt_remove_access (struct io_pagetable *iopt, -struct iommufd_access *access, -u32 iopt_access_list_id) +struct iommufd_access *access,u32 iopt_access_list_id) {down_write (&iopt->domains_rwsem);down_write (&iopt->iova_rwsem);
drivers/iommu/iommufd/iova_bitmap.c +0 -1
@@ -407,7 +407,6 @@ void iova_bitmap_set(struct iova_bitmap *bitmap, update_indexes:if (unlikely (!iova_bitmap_mapped_range (mapped, iova, length))) { -/* * The attempt to advance the base index to @iova * may fail if it's out of bounds, or pinning the pages
drivers/iommu/iommufd/main.c +2 -4
@@ -102,9 +102,8 @@ static int iommufd_object_dec_wait_shortterm(struct iommufd_ctx *ictx,return 0 ;if (wait_event_timeout (ictx->destroy_wait, -refcount_read (&to_destroy->shortterm_users) == -0 , -msecs_to_jiffies (60000 ))) +refcount_read (&to_destroy->shortterm_users) ==0 , +msecs_to_jiffies (60000 )))return 0 ;pr_crit ("Time out waiting for iommufd object to become free\n" );
@@ -539,7 +538,6 @@ static struct miscdevice iommu_misc_dev = { .mode = 0660, }; -static struct miscdevice vfio_misc_dev = { .minor =VFIO_MINOR , .name ="vfio" ,
drivers/iommu/iommufd/pages.c +4 -5
@@ -1287,8 +1287,7 @@ static int pfn_reader_first(struct pfn_reader *pfns, struct iopt_pages *pages, }static struct iopt_pages *iopt_alloc_pages (unsigned long start_byte, -unsigned long length, -bool writable) +unsigned long length,bool writable) {struct iopt_pages *pages;
@@ -1328,7 +1327,7 @@ struct iopt_pages *iopt_alloc_user_pages(void __user *uptr,struct iopt_pages *pages;unsigned long end;void __user *uptr_down = - (void __user *)ALIGN_DOWN ((uintptr_t )uptr,PAGE_SIZE ); + (void __user *)ALIGN_DOWN ((uintptr_t )uptr,PAGE_SIZE );if (check_add_overflow ((unsigned long )uptr, length, &end))return ERR_PTR (-EOVERFLOW );
@@ -2111,8 +2110,8 @@ iopt_pages_get_exact_access(struct iopt_pages *pages, unsigned long index,* This should be undone through a matching call to iopt_area_remove_access() */ int iopt_area_add_access (struct iopt_area *area,unsigned long start_index, -unsigned long last_index,struct page **out_pages, -unsigned int flags) +unsigned long last_index,struct page **out_pages, +unsigned int flags) {struct iopt_pages *pages = area->pages;struct iopt_pages_access *access;
drivers/iommu/iommufd/selftest.c +11 -13
@@ -1216,9 +1216,8 @@ 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) +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;
@@ -1491,7 +1490,7 @@ static int iommufd_test_access_pages(struct iommufd_ucmd *ucmd,int rc;/* Prevent syzkaller from triggering a WARN_ON in kvzalloc() */ -if (length >16 *1024 *1024 ) +if (length >16 *1024 *1024 )return -ENOMEM ;if (flags & ~(MOCK_FLAGS_ACCESS_WRITE |MOCK_FLAGS_ACCESS_SYZ ))
@@ -1508,7 +1507,7 @@ static int iommufd_test_access_pages(struct iommufd_ucmd *ucmd,if (flags &MOCK_FLAGS_ACCESS_SYZ ) iova =iommufd_test_syz_conv_iova (staccess->access, - &cmd->access_pages.iova); + &cmd->access_pages.iova); npages = (ALIGN (iova + length,PAGE_SIZE ) - ALIGN_DOWN(iova, PAGE_SIZE)) /
@@ -1584,7 +1583,7 @@ static int iommufd_test_access_rw(struct iommufd_ucmd *ucmd,int rc;/* Prevent syzkaller from triggering a WARN_ON in kvzalloc() */ -if (length >16 *1024 *1024 ) +if (length >16 *1024 *1024 )return -ENOMEM ; if (flags & ~(MOCK_ACCESS_RW_WRITE | MOCK_ACCESS_RW_SLOW_PATH |
@@ -1610,7 +1609,7 @@ static int iommufd_test_access_rw(struct iommufd_ucmd *ucmd,if (flags &MOCK_FLAGS_ACCESS_SYZ ) iova =iommufd_test_syz_conv_iova (staccess->access, - &cmd->access_rw.iova); + &cmd->access_rw.iova); rc =iommufd_access_rw (staccess->access, iova, tmp, length, flags);if (rc)
@@ -1665,7 +1664,7 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id, goto out_put; } -if (copy_from_user (tmp, uptr,DIV_ROUND_UP (max,BITS_PER_BYTE ))) { +if (copy_from_user (tmp, uptr,DIV_ROUND_UP (max,BITS_PER_BYTE ))) { rc = -EFAULT ; goto out_free; }
@@ -1701,7 +1700,7 @@ static int iommufd_test_dirty(struct iommufd_ucmd *ucmd, unsigned int mockpt_id,static int iommufd_test_trigger_iopf (struct iommufd_ucmd *ucmd,struct iommu_test_cmd *cmd) { -struct iopf_fault event = { }; +struct iopf_fault event = {};struct iommufd_device *idev; idev =iommufd_get_device (ucmd, cmd->trigger_iopf.dev_id);
@@ -1832,8 +1831,7 @@ static int iommufd_test_pasid_attach(struct iommufd_ucmd *ucmd, rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd));if (rc) -iommufd_device_detach (sobj->idev.idev, - cmd->pasid_attach.pasid); +iommufd_device_detach (sobj->idev.idev, cmd->pasid_attach.pasid); out_sobj:iommufd_put_object (ucmd->ictx, &sobj->obj);
@@ -2004,8 +2002,8 @@ int __init iommufd_test_init(void) goto err_bus; rc =iommu_device_register_bus (&mock_iommu.iommu_dev, &mock_ops, - &iommufd_mock_bus_type.bus, - &iommufd_mock_bus_type.nb); + &iommufd_mock_bus_type.bus, + &iommufd_mock_bus_type.nb);if (rc) goto err_sysfs;
Signature · 1 line
2.43.0
Re: [PATCH v2 01/14] iommufd: Apply obvious cosmetic fixes
Quoted 9 lines · Nicolin Chen
Run clang-format but exclude those not so obvious ones, which leaves us:
- Align indentations
- Add missing spaces
- Remove unnecessary spaces
- Remove unnecessary line wrappingsReviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
PATCH v2 2/14 iommufd: Drop unused ictx in struct iommufd_vdevice
In reply to Nicolin ChenThe core code can always get the ictx pointer via vdev->viommu->ictx, thus drop this unused one.
--- drivers/iommu/iommufd/iommufd_private.h | 1 - 1 file changed, 1 deletion(-)
drivers/iommu/iommufd/iommufd_private.h +0 -1
@@ -601,7 +601,6 @@ void iommufd_vdevice_destroy(struct iommufd_object *obj);struct iommufd_vdevice {struct iommufd_object obj; -struct iommufd_ctx *ictx;struct iommufd_viommu *viommu;struct device *dev;u64 id;/* per-vIOMMU virtual ID */
Signature · 1 line
2.43.0
Re: [PATCH v2 02/14] iommufd: Drop unused ictx in struct iommufd_vdevice
Quoted 6 lines · Nicolin Chen
The core code can always get the ictx pointer via
vdev->viommu->ictx, thus drop this unused one.Reviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
PATCH v2 3/14 iommufd: Use enum iommu_viommu_type for type in struct iommufd_viommu
In reply to Nicolin ChenReplace unsigned init, to make it clear. No functional changes.
The viommu_alloc iommu op will be deprecated, so don't change that.
--- include/linux/iommufd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/iommufd.h +1 -1
@@ -101,7 +101,7 @@ struct iommufd_viommu {struct list_head veventqs;struct rw_semaphore veventqs_rwsem; -unsigned int type; +enum iommu_viommu_type type; };/**
Signature · 1 line
2.43.0
Re: [PATCH v2 03/14] iommufd: Use enum iommu_viommu_type for type in struct iommufd_viommu
Quoted 7 lines · Nicolin Chen
Replace unsigned init, to make it clear. No functional changes.
The
viommu_allociommu op will be deprecated, so don't change that.Reviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 03/14] iommufd: Use enum iommu_viommu_type for type in struct iommufd_viommu
In reply to Nicolin ChenQuoted 1 line · Nicolin Chen
Replace unsigned init, to make it clear. No functional changes.
Super nit: Typo: s/init/int
Quoted 15 lines
The
viommu_allociommu op will be deprecated, so don't change that.Reviewed-by Kevin Tian <[email protected]>Reviewed-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- include/linux/iommufd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)include/linux/iommufd.h +0
-0
include/linux/iommufd.h struct iommufd_viommu { · line 105
Quoted 6 lines
include/linux/iommufd.h +1
-1@@ -101,7 +101,7 @@ struct iommufd_viommu {struct list_head veventqs;struct rw_semaphore veventqs_rwsem; -unsigned int type; +enum iommu_viommu_type type; };
Quoted 5 lines
include/linux/iommufd.h +0
-0/** Signature · 1 line
2.43.0
PATCH v2 4/14 iommufd: Use enum iommu_veventq_type for type in struct iommufd_veventq
In reply to Nicolin ChenReplace unsigned init, to make it clear. No functional changes.
--- drivers/iommu/iommufd/iommufd_private.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +3 -2
@@ -526,7 +526,7 @@ struct iommufd_veventq {struct list_head node;/* for iommufd_viommu::veventqs */ struct iommufd_vevent lost_events_header; -unsigned int type; +enum iommu_veventq_type type;unsigned int depth;/* Use common.lock for protection */
@@ -581,7 +581,8 @@ iommufd_get_viommu(struct iommufd_ucmd *ucmd, u32 id) }static inline struct iommufd_veventq * -iommufd_viommu_find_veventq (struct iommufd_viommu *viommu,u32 type) +iommufd_viommu_find_veventq (struct iommufd_viommu *viommu, +enum iommu_veventq_type type) {struct iommufd_veventq *veventq, *next;
Signature · 1 line
2.43.0
Re: [PATCH v2 04/14] iommufd: Use enum iommu_veventq_type for type in struct iommufd_veventq
Quoted 5 lines · Nicolin Chen
Replace unsigned init, to make it clear. No functional changes.
Reviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 04/14] iommufd: Use enum iommu_veventq_type for type in struct iommufd_veventq
In reply to Nicolin ChenQuoted 1 line · Nicolin Chen
Replace unsigned init, to make it clear. No functional changes.
Nit: s/init/int
Quoted 1 line
Quoted 33 lines
Reviewed-by Kevin Tian <[email protected]>Reviewed-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/iommufd_private.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)drivers/iommu/iommufd/iommufd_private.h +3
-2@@ -526,7 +526,7 @@ struct iommufd_veventq {struct list_head node;/* for iommufd_viommu::veventqs */ struct iommufd_vevent lost_events_header; -unsigned int type; +enum iommu_veventq_type type;unsigned int depth;/* Use common.lock for protection */ @@ -581,7 +581,8 @@ iommufd_get_viommu(struct iommufd_ucmd *ucmd, u32 id) }static inline struct iommufd_veventq * -iommufd_viommu_find_veventq (struct iommufd_viommu *viommu,u32 type) +iommufd_viommu_find_veventq (struct iommufd_viommu *viommu, +enum iommu_veventq_type type) {struct iommufd_veventq *veventq, *next;Signature · 1 line
2.43.0
PATCH v2 5/14 iommufd: Return EOPNOTSUPP for failures due to driver bugs
In reply to Nicolin ChenIt's more accurate to report EOPNOTSUPP when an ioctl failed due to driver bug, since there is nothing wrong with the user space side.
--- drivers/iommu/iommufd/device.c | 2 +- drivers/iommu/iommufd/hw_pagetable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/iommu/iommufd/device.c +1 -1
@@ -1483,7 +1483,7 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)*/ if (WARN_ON_ONCE (cmd->out_data_type ==IOMMU_HW_INFO_TYPE_NONE )) { - rc = -ENODEV ; + rc = -EOPNOTSUPP ; goto out_free; } }else {
drivers/iommu/iommufd/hw_pagetable.c +2 -2
@@ -264,7 +264,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx, hwpt->domain->cookie_type =IOMMU_COOKIE_IOMMUFD ;if (WARN_ON_ONCE (hwpt->domain->type !=IOMMU_DOMAIN_NESTED )) { - rc = -EINVAL ; + rc = -EOPNOTSUPP ; goto out_abort; }return hwpt_nested;
@@ -321,7 +321,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags, hwpt->domain->cookie_type =IOMMU_COOKIE_IOMMUFD ;if (WARN_ON_ONCE (hwpt->domain->type !=IOMMU_DOMAIN_NESTED )) { - rc = -EINVAL ; + rc = -EOPNOTSUPP ; goto out_abort; }return hwpt_nested;
Signature · 1 line
2.43.0
Re: [PATCH v2 05/14] iommufd: Return EOPNOTSUPP for failures due to driver bugs
Quoted 4 lines · Nicolin Chen
It's more accurate to report EOPNOTSUPP when an ioctl failed due to driver bug, since there is nothing wrong with the user space side.
Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 05/14] iommufd: Return EOPNOTSUPP for failures due to driver bugs
In reply to Nicolin ChenQuoted 8 lines · Nicolin Chen
It's more accurate to report EOPNOTSUPP when an ioctl failed due to driver bug, since there is nothing wrong with the user space side.
Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/device.c | 2 +- drivers/iommu/iommufd/hw_pagetable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
Jason
Re: [PATCH v2 05/14] iommufd: Return EOPNOTSUPP for failures due to driver bugs
In reply to Nicolin ChenQuoted 36 lines · Nicolin Chen
It's more accurate to report EOPNOTSUPP when an ioctl failed due to driver bug, since there is nothing wrong with the user space side.
Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/device.c | 2 +- drivers/iommu/iommufd/hw_pagetable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)drivers/iommu/iommufd/device.c +1
-1@@ -1483,7 +1483,7 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)*/ if (WARN_ON_ONCE (cmd->out_data_type ==IOMMU_HW_INFO_TYPE_NONE )) { - rc = -ENODEV ; + rc = -EOPNOTSUPP ; goto out_free; } }else {drivers/iommu/iommufd/hw_pagetable.c +1
-1@@ -264,7 +264,7 @@ iommufd_hwpt_nested_alloc(struct iommufd_ctx *ictx, hwpt->domain->cookie_type =IOMMU_COOKIE_IOMMUFD ;if (WARN_ON_ONCE (hwpt->domain->type !=IOMMU_DOMAIN_NESTED )) { - rc = -EINVAL ; + rc = -EOPNOTSUPP ; goto out_abort; }return hwpt_nested;
drivers/iommu/iommufd/hw_pagetable.c iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags, · line 327
Quoted 8 lines
drivers/iommu/iommufd/hw_pagetable.c +1
-1@@ -321,7 +321,7 @@ iommufd_viommu_alloc_hwpt_nested(struct iommufd_viommu *viommu, u32 flags, hwpt->domain->cookie_type =IOMMU_COOKIE_IOMMUFD ;if (WARN_ON_ONCE (hwpt->domain->type !=IOMMU_DOMAIN_NESTED )) { - rc = -EINVAL ; + rc = -EOPNOTSUPP ; goto out_abort; }return hwpt_nested;
Much clearer retvals.
Quoted 3 lines
Signature · 1 line
2.43.0
RE: [PATCH v2 05/14] iommufd: Return EOPNOTSUPP for failures due to driver bugs
In reply to Nicolin ChenQuoted 7 lines
From: Nicolin Chen <[email protected]>
Sent: Saturday, June 14, 2025 2:35 PMIt's more accurate to report EOPNOTSUPP when an ioctl failed due to driver bug, since there is nothing wrong with the user space side.
Signed-off-by Nicolin Chen <[email protected]>
PATCH v2 6/14 iommu: Introduce get_viommu_size and viommu_init ops
In reply to Nicolin ChenSo far, a vIOMMU object has been allocated by IOMMU driver and initialized with the driver-level structure, before it returns to the iommufd core for core-level structure initialization. It has been requiring iommufd core to expose some core structure/helpers in its driver.c file, which result in a size increase of this driver module.
Meanwhile, IOMMU drivers are now requiring more vIOMMU-base structures for some advanced feature, such as the existing vDEVICE and a future HW_QUEUE. Initializing a core-structure later than driver-structure gives for-driver helpers some trouble, when they are used by IOMMU driver assuming that the new structure (including core) are fully initialized, for example:
core: viommu = ops->viommu_alloc();
driver: // my_viommu is successfully allocated
driver: my_viommu = iommufd_viommu_alloc(...);
driver: // This may crash if it reads viommu->ictx
driver: new = iommufd_new_viommu_helper(my_viommu->core ...);
core: viommu->ictx = ucmd->ictx;
core: ...
To ease such a condition, allow the IOMMU driver to report the size of its vIOMMU structure, let the core allocate a vIOMMU object and initialize the core-level structure first, and then hand it over the driver to initialize its driver-level structure.
Thus, this requires two new iommu ops, get_viommu_size and viommu_init, so iommufd core can communicate with drivers to replace the viommu_alloc op:
core: viommu = ops->get_viommu_size();
driver: return VIOMMU_STRUCT_SIZE();
core: viommu->ictx = ucmd->ictx; // and others
core: rc = ops->viommu_init();
driver: // This is safe now as viommu->ictx is inited
driver: new = iommufd_new_viommu_helper(my_viommu->core ...);
core: ...
This also adds a VIOMMU_STRUCT_SIZE macro, for drivers to use, which would statically sanitize the driver structure.
--- include/linux/iommu.h | 15 +++++++++++++++ include/linux/iommufd.h | 6 ++++++ 2 files changed, 21 insertions(+)
include/linux/iommu.h +15 -0
@@ -14,6 +14,7 @@#include <linux/err.h> #include <linux/of.h> #include <linux/iova_bitmap.h> +#include <uapi/linux/iommufd.h> #define IOMMU_READ (1 << 0)#define IOMMU_WRITE (1 << 1)
@@ -596,6 +597,16 @@ iommu_copy_struct_from_full_user_array(void *kdst, size_t kdst_entry_size,* - IOMMU_DOMAIN_DMA: must use a dma domain * - 0: use the default setting * @default_domain_ops: the default ops for domains +* @get_viommu_size: Get the size of a driver-level vIOMMU structure for a given +* @dev corresponding to @viommu_type. Driver should return 0 +* if vIOMMU isn't supported accordingly. It is required for +* driver to use the VIOMMU_STRUCT_SIZE macro to sanitize the +* driver-level vIOMMU structure related to the core one +* @viommu_init: Init the driver-level struct of an iommufd_viommu on a physical +* IOMMU instance @viommu->iommu_dev, as the set of virtualization +* resources shared/passed to user space IOMMU instance. Associate +* it with a nesting @parent_domain. It is required for driver to +* set @viommu->ops pointing to its own viommu_ops * @viommu_alloc: Allocate an iommufd_viommu on a physical IOMMU instance behind * the @dev, as the set of virtualization resources shared/passed * to user space IOMMU instance. And associate it with a nesting
@@ -654,6 +665,10 @@ struct iommu_ops {int (*def_domain_type)(struct device *dev); +size_t (*get_viommu_size)(struct device *dev, +enum iommu_viommu_type viommu_type); +int (*viommu_init)(struct iommufd_viommu *viommu, +struct iommu_domain *parent_domain);struct iommufd_viommu *(*viommu_alloc)(struct device *dev,struct iommu_domain *parent_domain,struct iommufd_ctx *ictx,unsigned int viommu_type);
include/linux/iommufd.h +6 -0
@@ -229,6 +229,12 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, }#endif /* CONFIG_IOMMUFD_DRIVER_CORE */ +#define VIOMMU_STRUCT_SIZE (drv_struct, member) \ + (sizeof(drv_struct) + \ + BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ + BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_viommu, \ + ((drv_struct *)NULL)->member))) +/* * Helpers for IOMMU driver to allocate driver structures that will be freed by * the iommufd core. The free op will be called prior to freeing the memory.
Signature · 1 line
2.43.0
Re: [PATCH v2 06/14] iommu: Introduce get_viommu_size and viommu_init ops
Quoted 36 lines · Nicolin Chen
So far, a vIOMMU object has been allocated by IOMMU driver and initialized with the driver-level structure, before it returns to the iommufd core for core-level structure initialization. It has been requiring iommufd core to expose some core structure/helpers in its
driver.cfile, which result in a size increase of this driver module.Meanwhile, IOMMU drivers are now requiring more vIOMMU-base structures for some advanced feature, such as the existing vDEVICE and a future
HW_QUEUE. Initializing a core-structure later than driver-structure gives for-driver helpers some trouble, when they are used by IOMMU driver assuming that the new structure (including core) are fully initialized, for example:core: viommu =
ops->viommu_alloc();
driver: //my_viommuis successfully allocated
driver:my_viommu=iommufd_viommu_alloc(...);
driver: // This may crash if it readsviommu->ictx
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core:viommu->ictx=ucmd->ictx;
core: ...To ease such a condition, allow the IOMMU driver to report the size of its vIOMMU structure, let the core allocate a vIOMMU object and initialize the core-level structure first, and then hand it over the driver to initialize its driver-level structure.
Thus, this requires two new iommu ops,
get_viommu_sizeandviommu_init, so iommufd core can communicate with drivers to replace theviommu_allocop:core: viommu =
ops->get_viommu_size();
driver: returnVIOMMU_STRUCT_SIZE();
core:viommu->ictx=ucmd->ictx; // and others
core: rc =ops->viommu_init();
driver: // This is safe now asviommu->ictxis inited
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core: ...
Quoted 5 lines
This also adds a
VIOMMU_STRUCT_SIZEmacro, for drivers to use, which would statically sanitize the driver structure.Suggested-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 06/14] iommu: Introduce get_viommu_size and viommu_init ops
In reply to Nicolin ChenQuoted 39 lines · Nicolin Chen
So far, a vIOMMU object has been allocated by IOMMU driver and initialized with the driver-level structure, before it returns to the iommufd core for core-level structure initialization. It has been requiring iommufd core to expose some core structure/helpers in its
driver.cfile, which result in a size increase of this driver module.Meanwhile, IOMMU drivers are now requiring more vIOMMU-base structures for some advanced feature, such as the existing vDEVICE and a future
HW_QUEUE. Initializing a core-structure later than driver-structure gives for-driver helpers some trouble, when they are used by IOMMU driver assuming that the new structure (including core) are fully initialized, for example:core: viommu =
ops->viommu_alloc();
driver: //my_viommuis successfully allocated
driver:my_viommu=iommufd_viommu_alloc(...);
driver: // This may crash if it readsviommu->ictx
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core:viommu->ictx=ucmd->ictx;
core: ...To ease such a condition, allow the IOMMU driver to report the size of its vIOMMU structure, let the core allocate a vIOMMU object and initialize the core-level structure first, and then hand it over the driver to initialize its driver-level structure.
Thus, this requires two new iommu ops,
get_viommu_sizeandviommu_init, so iommufd core can communicate with drivers to replace theviommu_allocop:core: viommu =
ops->get_viommu_size();
driver: returnVIOMMU_STRUCT_SIZE();
core:viommu->ictx=ucmd->ictx; // and others
core: rc =ops->viommu_init();
driver: // This is safe now asviommu->ictxis inited
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core: ...This also adds a
VIOMMU_STRUCT_SIZEmacro, for drivers to use, which would statically sanitize the driver structure.
Quoted 6 lines
Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- include/linux/iommu.h | 15 +++++++++++++++ include/linux/iommufd.h | 6 ++++++ 2 files changed, 21 insertions(+)
Jason
Re: [PATCH v2 06/14] iommu: Introduce get_viommu_size and viommu_init ops
In reply to Nicolin ChenQuoted 39 lines · Nicolin Chen
So far, a vIOMMU object has been allocated by IOMMU driver and initialized with the driver-level structure, before it returns to the iommufd core for core-level structure initialization. It has been requiring iommufd core to expose some core structure/helpers in its
driver.cfile, which result in a size increase of this driver module.Meanwhile, IOMMU drivers are now requiring more vIOMMU-base structures for some advanced feature, such as the existing vDEVICE and a future
HW_QUEUE. Initializing a core-structure later than driver-structure gives for-driver helpers some trouble, when they are used by IOMMU driver assuming that the new structure (including core) are fully initialized, for example:core: viommu =
ops->viommu_alloc();
driver: //my_viommuis successfully allocated
driver:my_viommu=iommufd_viommu_alloc(...);
driver: // This may crash if it readsviommu->ictx
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core:viommu->ictx=ucmd->ictx;
core: ...To ease such a condition, allow the IOMMU driver to report the size of its vIOMMU structure, let the core allocate a vIOMMU object and initialize the core-level structure first, and then hand it over the driver to initialize its driver-level structure.
Thus, this requires two new iommu ops,
get_viommu_sizeandviommu_init, so iommufd core can communicate with drivers to replace theviommu_allocop:core: viommu =
ops->get_viommu_size();
driver: returnVIOMMU_STRUCT_SIZE();
core:viommu->ictx=ucmd->ictx; // and others
core: rc =ops->viommu_init();
driver: // This is safe now asviommu->ictxis inited
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core: ...This also adds a
VIOMMU_STRUCT_SIZEmacro, for drivers to use, which would statically sanitize the driver structure.
Quoted 7 lines
Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- include/linux/iommu.h | 15 +++++++++++++++ include/linux/iommufd.h | 6 ++++++ 2 files changed, 21 insertions(+)
RE: [PATCH v2 06/14] iommu: Introduce get_viommu_size and viommu_init ops
In reply to Nicolin ChenQuoted 43 lines
From: Nicolin Chen <[email protected]>
Sent: Saturday, June 14, 2025 2:35 PMSo far, a vIOMMU object has been allocated by IOMMU driver and initialized with the driver-level structure, before it returns to the iommufd core for core-level structure initialization. It has been requiring iommufd core to expose some core structure/helpers in its
driver.cfile, which result in a size increase of this driver module.Meanwhile, IOMMU drivers are now requiring more vIOMMU-base structures for
some advanced feature, such as the existing vDEVICE and a futureHW_QUEUE.
Initializing a core-structure later than driver-structure gives for-driver helpers some trouble, when they are used by IOMMU driver assuming that the
new structure (including core) are fully initialized, for example:core: viommu =
ops->viommu_alloc();
driver: //my_viommuis successfully allocated
driver:my_viommu=iommufd_viommu_alloc(...);
driver: // This may crash if it readsviommu->ictx
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core:viommu->ictx=ucmd->ictx;
core: ...To ease such a condition, allow the IOMMU driver to report the size of its vIOMMU structure, let the core allocate a vIOMMU object and initialize the core-level structure first, and then hand it over the driver to initialize its driver-level structure.
Thus, this requires two new iommu ops,
get_viommu_sizeandviommu_init, so
iommufd core can communicate with drivers to replace theviommu_allocop:core: viommu =
ops->get_viommu_size();
driver: returnVIOMMU_STRUCT_SIZE();
core:viommu->ictx=ucmd->ictx; // and others
core: rc =ops->viommu_init();
driver: // This is safe now asviommu->ictxis inited
driver: new =iommufd_new_viommu_helper(my_viommu->core ...);
core: ...
Quoted 6 lines
This also adds a
VIOMMU_STRUCT_SIZEmacro, for drivers to use, which would
statically sanitize the driver structure.Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>
PATCH v2 7/14 iommufd/viommu: Support get_viommu_size and viommu_init ops
In reply to Nicolin ChenTo ease the for-driver iommufd APIs, get_viommu_size and viommu_init ops are introduced to replace the viommu_init op.
Let the new viommu_init pathway coexist with the old viommu_alloc one.
Since the viommu_alloc op and its pathway will be soon deprecated, try to minimize the code difference between them by adding a tentative jump tag.
Note that this fails a !viommu->ops case from now on with a WARN_ON_ONCE since a vIOMMU is expected to support an alloc_domain_nested op for now, or some sort of a viommu op in the foreseeable future. This WARN_ON_ONCE can be lifted, if some day there is a use case wanting !viommu->ops.
--- drivers/iommu/iommufd/viommu.c | 42 +++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-)
drivers/iommu/iommufd/viommu.c +39 -3
@@ -21,6 +21,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)struct iommufd_viommu *viommu;struct iommufd_device *idev;const struct iommu_ops *ops; +size_t viommu_size;int rc;if (cmd->flags || cmd->type ==IOMMU_VIOMMU_TYPE_DEFAULT )
@@ -31,11 +32,29 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)return PTR_ERR (idev); ops =dev_iommu_ops (idev->dev); -if (!ops->viommu_alloc) { +if (!ops->get_viommu_size || !ops->viommu_init) { +if (ops->viommu_alloc) + goto get_hwpt_paging; + rc = -EOPNOTSUPP ; + goto out_put_idev; + } + + viommu_size = ops->get_viommu_size (idev->dev, cmd->type); +if (!viommu_size) { + rc = -EOPNOTSUPP ; + goto out_put_idev; + } + +/* +* It is a driver bug for providing a viommu_size smaller than the core +* vIOMMU structure size +*/ +if (WARN_ON_ONCE (viommu_size <sizeof (*viommu))) { rc = -EOPNOTSUPP ; goto out_put_idev; } +get_hwpt_paging: hwpt_paging =iommufd_get_hwpt_paging (ucmd, cmd->hwpt_id);if (IS_ERR (hwpt_paging)) { rc =PTR_ERR (hwpt_paging);
@@ -47,8 +66,13 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_hwpt; } - viommu = ops->viommu_alloc (idev->dev, hwpt_paging->common.domain, - ucmd->ictx, cmd->type); +if (ops->viommu_alloc) + viommu = ops->viommu_alloc (idev->dev, + hwpt_paging->common.domain, + ucmd->ictx, cmd->type); +else + viommu = (struct iommufd_viommu *)_iommufd_object_alloc ( + ucmd->ictx, viommu_size,IOMMUFD_OBJ_VIOMMU );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
@@ -68,6 +92,18 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)*/ viommu->iommu_dev =__iommu_get_iommu_dev (idev->dev); +if (!ops->viommu_alloc) { + rc = ops->viommu_init (viommu, hwpt_paging->common.domain); +if (rc) + goto out_abort; + } + +/* It is a driver bug that viommu->ops isn't filled */ +if (WARN_ON_ONCE (!viommu->ops)) { + rc = -EOPNOTSUPP ; + goto out_abort; + } + cmd->out_viommu_id = viommu->obj.id; rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd));if (rc)
Signature · 1 line
2.43.0
Re: [PATCH v2 07/14] iommufd/viommu: Support get_viommu_size and viommu_init ops
Quoted 8 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced to replace theviommu_initop.Let the new
viommu_initpathway coexist with the oldviommu_allocone.Since the
viommu_allocop and its pathway will be soon deprecated, try to minimize the code difference between them by adding a tentative jump tag.
Quoted 7 lines
Note that this fails a !viommu->ops case from now on with a
WARN_ON_ONCEsince a vIOMMU is expected to support analloc_domain_nestedop for now, or some sort of a viommu op in the foreseeable future. ThisWARN_ON_ONCEcan be lifted, if some day there is a use case wanting !viommu->ops.Suggested-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 07/14] iommufd/viommu: Support get_viommu_size and viommu_init ops
In reply to Nicolin ChenQuoted 13 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced to replace theviommu_initop.Let the new
viommu_initpathway coexist with the oldviommu_allocone.Since the
viommu_allocop and its pathway will be soon deprecated, try to minimize the code difference between them by adding a tentative jump tag.Note that this fails a !viommu->ops case from now on with a
WARN_ON_ONCEsince a vIOMMU is expected to support analloc_domain_nestedop for now, or some sort of a viommu op in the foreseeable future. ThisWARN_ON_ONCEcan be lifted, if some day there is a use case wanting !viommu->ops.
Quoted 5 lines
Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/viommu.c | 42 +++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-)
Jason
RE: [PATCH v2 07/14] iommufd/viommu: Support get_viommu_size and viommu_init ops
In reply to Nicolin ChenQuoted 18 lines
From: Nicolin Chen <[email protected]>
Sent: Saturday, June 14, 2025 2:35 PMTo ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced to replace theviommu_initop.Let the new
viommu_initpathway coexist with the oldviommu_allocone.Since the
viommu_allocop and its pathway will be soon deprecated, try to minimize the code difference between them by adding a tentative jump tag.Note that this fails a !viommu->ops case from now on with a
WARN_ON_ONCE
since a vIOMMU is expected to support analloc_domain_nestedop for now, or some sort of a viommu op in the foreseeable future. ThisWARN_ON_ONCE
can be lifted, if some day there is a use case wanting !viommu->ops.
Quoted 2 lines
Suggested-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>
PATCH v2 8/14 iommufd/selftest: Drop parent domain from mock_iommu_domain_nested
In reply to Nicolin ChenThere is no use of this parent domain. Delete the dead code.
Note that the s2_parent in struct mock_viommu will be a deadcode too. Yet, keep it because it will be soon used by HW queue objects, i.e. no point in adding it back and forth in such a short window. Besides, keeping it could cover the majority of vIOMMU use cases where a driver-level structure will be larger in size than the core structure.
--- drivers/iommu/iommufd/selftest.c | 3 --- 1 file changed, 3 deletions(-)
drivers/iommu/iommufd/selftest.c +0 -3
@@ -138,7 +138,6 @@ to_mock_domain(struct iommu_domain *domain)struct mock_iommu_domain_nested {struct iommu_domain domain;struct mock_viommu *mock_viommu; -struct mock_iommu_domain *parent;u32 iotlb[MOCK_NESTED_DOMAIN_IOTLB_NUM ]; };
@@ -434,7 +433,6 @@ mock_domain_alloc_nested(struct device *dev, struct iommu_domain *parent, mock_nested =__mock_domain_alloc_nested (user_data);if (IS_ERR (mock_nested))return ERR_CAST (mock_nested); - mock_nested->parent = mock_parent;return &mock_nested->domain; }
@@ -692,7 +690,6 @@ mock_viommu_alloc_domain_nested(struct iommufd_viommu *viommu, u32 flags,if (IS_ERR (mock_nested))return ERR_CAST (mock_nested); mock_nested->mock_viommu = mock_viommu; - mock_nested->parent = mock_viommu->s2_parent;return &mock_nested->domain; }
Signature · 1 line
2.43.0
Re: [PATCH v2 08/14] iommufd/selftest: Drop parent domain from mock_iommu_domain_nested
Quoted 8 lines · Nicolin Chen
There is no use of this parent domain. Delete the dead code.
Note that the
s2_parentin structmock_viommuwill be a deadcode too. Yet, keep it because it will be soon used by HW queue objects, i.e. no point in adding it back and forth in such a short window. Besides, keeping it could cover the majority of vIOMMU use cases where a driver-level structure will be larger in size than the core structure.
Quoted 4 lines
Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/selftest.c | 3 --- 1 file changed, 3 deletions(-)
Jason
RE: [PATCH v2 08/14] iommufd/selftest: Drop parent domain from mock_iommu_domain_nested
In reply to Nicolin ChenQuoted 5 lines
From: Nicolin Chen <[email protected]>
Sent: Saturday, June 14, 2025 2:35 PMThere is no use of this parent domain. Delete the dead code.
Quoted 7 lines
Note that the
s2_parentin structmock_viommuwill be a deadcode too. Yet, keep it because it will be soon used by HW queue objects, i.e. no point in adding it back and forth in such a short window. Besides, keeping it could cover the majority of vIOMMU use cases where a driver-level structure will be larger in size than the core structure.Signed-off-by Nicolin Chen <[email protected]>
PATCH v2 9/14 iommufd/selftest: Replace mock_viommu_alloc with mock_viommu_init
In reply to Nicolin ChenTo ease the for-driver iommufd APIs, get_viommu_size and viommu_init ops are introduced.
Sanitize the inputs and report the size of struct mock_viommu on success, in mock_get_viommu_size().
The core will ensure the viommu_type is set to the core vIOMMU object, so simply init the driver part in mock_viommu_init().
Remove the mock_viommu_alloc, completing the replacement.
--- drivers/iommu/iommufd/selftest.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
drivers/iommu/iommufd/selftest.c +14 -15
@@ -769,25 +769,23 @@ static struct iommufd_viommu_ops mock_viommu_ops = { .cache_invalidate = mock_viommu_cache_invalidate, }; -static struct iommufd_viommu *mock_viommu_alloc (struct device *dev, -struct iommu_domain *domain, -struct iommufd_ctx *ictx, -unsigned int viommu_type) +static size_t mock_get_viommu_size (struct device *dev, +enum iommu_viommu_type viommu_type) { -struct mock_iommu_device *mock_iommu = - iommu_get_iommu_dev(dev, struct mock_iommu_device, iommu_dev); -struct mock_viommu *mock_viommu; -if (viommu_type !=IOMMU_VIOMMU_TYPE_SELFTEST ) -return ERR_PTR (-EOPNOTSUPP ); +return 0 ; + return VIOMMU_STRUCT_SIZE(struct mock_viommu, core); +} - mock_viommu = iommufd_viommu_alloc(ictx, struct mock_viommu, core, - &mock_viommu_ops); -if (IS_ERR (mock_viommu)) -return ERR_CAST (mock_viommu); +static int mock_viommu_init (struct iommufd_viommu *viommu, +struct iommu_domain *parent_domain) +{ +struct mock_iommu_device *mock_iommu =container_of ( + viommu->iommu_dev, struct mock_iommu_device, iommu_dev);refcount_inc (&mock_iommu->users); -return &mock_viommu->core; + viommu->ops = &mock_viommu_ops; +return 0 ; }static const struct iommu_ops mock_ops = {
@@ -807,7 +805,8 @@ static const struct iommu_ops mock_ops = { .probe_device = mock_probe_device, .page_response = mock_domain_page_response, .user_pasid_table = true, - .viommu_alloc = mock_viommu_alloc, + .get_viommu_size = mock_get_viommu_size, + .viommu_init = mock_viommu_init, .default_domain_ops = &(struct iommu_domain_ops ){ .free = mock_domain_free,
Signature · 1 line
2.43.0
Re: [PATCH v2 09/14] iommufd/selftest: Replace mock_viommu_alloc with mock_viommu_init
Quoted 9 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
mock_viommuon success, inmock_get_viommu_size().The core will ensure the
viommu_typeis set to the core vIOMMU object, so simply init the driver part inmock_viommu_init().
Quoted 7 lines
Remove the
mock_viommu_alloc, completing the replacement.Reviewed-by Kevin Tian <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/selftest.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
Jason
PATCH v2 10/14 iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
In reply to Nicolin ChenTo ease the for-driver iommufd APIs, get_viommu_size and viommu_init ops are introduced.
Sanitize the inputs and report the size of struct arm_vsmmu on success, in arm_smmu_get_viommu_size().
Place the type sanity at the last, becase there will be soon an impl level get_viommu_size op, which will require the same sanity tests prior. It can simply insert a piece of code in front of the IOMMU_VIOMMU_TYPE_ARM_SMMUV3 sanity.
The core will ensure the viommu_type is set to the core vIOMMU object, and pass in the same dev pointer, so arm_vsmmu_init() won't need to repeat the same sanity tests but to simply init the arm_vsmmu struct.
Remove the arm_vsmmu_alloc, completing the replacement.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 11 +++-- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 46 ++++++++++--------- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +- 3 files changed, 32 insertions(+), 28 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +6 -5
@@ -1034,18 +1034,19 @@ struct arm_vsmmu {#if IS_ENABLED (CONFIG_ARM_SMMU_V3_IOMMUFD )void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type); -struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev, -struct iommu_domain *parent, -struct iommufd_ctx *ictx, -unsigned int viommu_type); +size_t arm_smmu_get_viommu_size (struct device *dev, +enum iommu_viommu_type viommu_type); +int arm_vsmmu_init (struct iommufd_viommu *viommu, +struct iommu_domain *parent_domain);int arm_smmu_attach_prepare_vmaster (struct arm_smmu_attach_state *state,struct arm_smmu_nested_domain *nested_domain);void arm_smmu_attach_commit_vmaster (struct arm_smmu_attach_state *state);void arm_smmu_master_clear_vmaster (struct arm_smmu_master *master);int arm_vmaster_report_event (struct arm_smmu_vmaster *vmaster,u64 *evt);#else +#define arm_smmu_get_viommu_size NULL#define arm_smmu_hw_info NULL -#define arm_vsmmu_alloc NULL +#define arm_vsmmu_init NULL static inline int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state,
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +24 -22
75 lines not on this page · raw message
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2 -1
@@ -3688,7 +3688,8 @@ static struct iommu_ops arm_smmu_ops = { .get_resv_regions = arm_smmu_get_resv_regions, .page_response = arm_smmu_page_response, .def_domain_type = arm_smmu_def_domain_type, - .viommu_alloc = arm_vsmmu_alloc, + .get_viommu_size = arm_smmu_get_viommu_size, + .viommu_init = arm_vsmmu_init, .user_pasid_table = 1, .pgsize_bitmap = -1UL, /* Restricted during device attach */ .owner = THIS_MODULE,
Signature · 1 line
2.43.0
Re: [PATCH v2 10/14] iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
Quoted 15 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
arm_vsmmuon success, inarm_smmu_get_viommu_size().Place the type sanity at the last, becase there will be soon an impl level
get_viommu_sizeop, which will require the same sanity tests prior. It can simply insert a piece of code in front of theIOMMU_VIOMMU_TYPE_ARM_SMMUV3sanity.The core will ensure the
viommu_typeis set to the core vIOMMU object, and pass in the same dev pointer, soarm_vsmmu_init()won't need to repeat the same sanity tests but to simply init thearm_vsmmustruct.
Quoted 8 lines
Remove the
arm_vsmmu_alloc, completing the replacement.Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 11 +++-- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 46 ++++++++++--------- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +- 3 files changed, 32 insertions(+), 28 deletions(-)
Will
Re: [PATCH v2 10/14] iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
In reply to Nicolin ChenQuoted 15 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
arm_vsmmuon success, inarm_smmu_get_viommu_size().Place the type sanity at the last, becase there will be soon an impl level
get_viommu_sizeop, which will require the same sanity tests prior. It can simply insert a piece of code in front of theIOMMU_VIOMMU_TYPE_ARM_SMMUV3sanity.The core will ensure the
viommu_typeis set to the core vIOMMU object, and pass in the same dev pointer, soarm_vsmmu_init()won't need to repeat the same sanity tests but to simply init thearm_vsmmustruct.
Quoted 8 lines
Remove the
arm_vsmmu_alloc, completing the replacement.Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 11 +++-- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 46 ++++++++++--------- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +- 3 files changed, 32 insertions(+), 28 deletions(-)
Jason
Re: [PATCH v2 10/14] iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
In reply to Nicolin ChenQuoted 11 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
arm_vsmmuon success, inarm_smmu_get_viommu_size().Place the type sanity at the last, becase there will be soon an impl level
get_viommu_sizeop, which will require the same sanity tests prior. It can simply insert a piece of code in front of theIOMMU_VIOMMU_TYPE_ARM_SMMUV3sanity.
That's what I was wondering, so we plan to replace the impl->vsmmu_alloc op as well?
Quoted 112 lines
The core will ensure the
viommu_typeis set to the core vIOMMU object, and pass in the same dev pointer, soarm_vsmmu_init()won't need to repeat the same sanity tests but to simply init thearm_vsmmustruct.Remove the
arm_vsmmu_alloc, completing the replacement.Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 11 +++-- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 46 ++++++++++--------- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 +- 3 files changed, 32 insertions(+), 28 deletions(-)drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +6
-5@@ -1034,18 +1034,19 @@ struct arm_vsmmu {#if IS_ENABLED (CONFIG_ARM_SMMU_V3_IOMMUFD )void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type); -struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev, -struct iommu_domain *parent, -struct iommufd_ctx *ictx, -unsigned int viommu_type); +size_t arm_smmu_get_viommu_size (struct device *dev, +enum iommu_viommu_type viommu_type); +int arm_vsmmu_init (struct iommufd_viommu *viommu, +struct iommu_domain *parent_domain);int arm_smmu_attach_prepare_vmaster (struct arm_smmu_attach_state *state,struct arm_smmu_nested_domain *nested_domain);void arm_smmu_attach_commit_vmaster (struct arm_smmu_attach_state *state);void arm_smmu_master_clear_vmaster (struct arm_smmu_master *master);int arm_vmaster_report_event (struct arm_smmu_vmaster *vmaster,u64 *evt);#else +#define arm_smmu_get_viommu_size NULL#define arm_smmu_hw_info NULL -#define arm_vsmmu_alloc NULL +#define arm_vsmmu_init NULL static inline int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state,drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +22
-21@@ -382,25 +382,14 @@ static const struct iommufd_viommu_ops arm_vsmmu_ops = { .cache_invalidate = arm_vsmmu_cache_invalidate, }; -struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev, -struct iommu_domain *parent, -struct iommufd_ctx *ictx, -unsigned int viommu_type) +size_t arm_smmu_get_viommu_size (struct device *dev, +enum iommu_viommu_type viommu_type) { -struct arm_smmu_device *smmu = - iommu_get_iommu_dev(dev, struct arm_smmu_device, iommu);struct arm_smmu_master *master =dev_iommu_priv_get (dev); -struct arm_smmu_domain *s2_parent =to_smmu_domain (parent); -struct arm_vsmmu *vsmmu; - -if (viommu_type !=IOMMU_VIOMMU_TYPE_ARM_SMMUV3 ) -return ERR_PTR (-EOPNOTSUPP ); +struct arm_smmu_device *smmu = master->smmu;if (!(smmu->features &ARM_SMMU_FEAT_NESTING )) -return ERR_PTR (-EOPNOTSUPP ); - -if (s2_parent->smmu != master->smmu) -return ERR_PTR (-EINVAL ); +return 0 ;/* * FORCE_SYNC is not set with FEAT_NESTING. Some study of the exact HW @@ -408,7 +397,7 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev,* any change to remove this. */ if (WARN_ON (smmu->options &ARM_SMMU_OPT_CMDQ_FORCE_SYNC )) -return ERR_PTR (-EOPNOTSUPP ); +return 0 ;/* * Must support some way to prevent the VM from bypassing the cache @@ -420,19 +409,32 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev,*/ if (!arm_smmu_master_canwbs (master) && !(smmu->features &ARM_SMMU_FEAT_S2FWB )) -return ERR_PTR (-EOPNOTSUPP ); +return 0 ; - vsmmu = iommufd_viommu_alloc(ictx, struct arm_vsmmu, core, - &arm_vsmmu_ops); -if (IS_ERR (vsmmu)) -return ERR_CAST (vsmmu); +if (viommu_type !=IOMMU_VIOMMU_TYPE_ARM_SMMUV3 ) +return 0 ; + + return VIOMMU_STRUCT_SIZE(struct arm_vsmmu, core); +} + +int arm_vsmmu_init (struct iommufd_viommu *viommu, +struct iommu_domain *parent_domain) +{ + struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core); +struct arm_smmu_device *smmu = + container_of(viommu->iommu_dev, struct arm_smmu_device, iommu); +struct arm_smmu_domain *s2_parent =to_smmu_domain (parent_domain); + +if (s2_parent->smmu != smmu) +return -EINVAL ;
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, · line 438
Quoted 10 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +2
-1vsmmu->smmu = smmu; vsmmu->s2_parent = s2_parent;/* FIXME Move VMID allocation from the S2 domain allocation to here */ vsmmu->vmid = s2_parent->s2_cfg.vmid; -return &vsmmu->core; + viommu->ops = &arm_vsmmu_ops; +return 0 ; }
Seems much better now that the driver doesn't need to callback to the core for allocating viommu. One quick question though I see we've removed the following too:
if (master->smmu->impl_ops &&master->smmu->impl_ops->vsmmu_alloc) vsmmu = master->smmu->impl_ops->vsmmu_alloc( master->smmu, s2_parent, ictx, viommu_type, user_data);
Not sure why don't I see that in the diffs.. do we plan to split this into an impl-specific size and init too?
Quoted 19 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +0
-0int arm_vmaster_report_event (struct arm_smmu_vmaster *vmaster,u64 *evt)drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2
-1@@ -3688,7 +3688,8 @@ static struct iommu_ops arm_smmu_ops = { .get_resv_regions = arm_smmu_get_resv_regions, .page_response = arm_smmu_page_response, .def_domain_type = arm_smmu_def_domain_type, - .viommu_alloc = arm_vsmmu_alloc, + .get_viommu_size = arm_smmu_get_viommu_size, + .viommu_init = arm_vsmmu_init, .user_pasid_table = 1, .pgsize_bitmap = -1UL, /* Restricted during device attach */ .owner = THIS_MODULE,Signature · 1 line
2.43.0
With that,
Re: [PATCH v2 10/14] iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
Quoted 7 lines · Pranjal Shrivastava
Quoted 6 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
arm_vsmmuon success, inarm_smmu_get_viommu_size().
Quoted 8 lines
Quoted 5 lines
Place the type sanity at the last, becase there will be soon an impl level
get_viommu_sizeop, which will require the same sanity tests prior. It can simply insert a piece of code in front of theIOMMU_VIOMMU_TYPE_ARM_SMMUV3sanity.That's what I was wondering, so we plan to replace the
impl->vsmmu_allocop as well?
There is no such op in v6.16-rc1.
Quoted 9 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, · line 438
Quoted 4 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +2
-1-return &vsmmu->core; + viommu->ops = &arm_vsmmu_ops; +return 0 ; }Seems much better now that the driver doesn't need to callback to the core for allocating viommu. One quick question though I see we've removed the following too:
Quoted 8 lines
if (master->smmu->impl_ops &&master->smmu->impl_ops->vsmmu_alloc) vsmmu = master->smmu->impl_ops->vsmmu_alloc( master->smmu, s2_parent, ictx, viommu_type, user_data);Not sure why don't I see that in the diffs.. do we plan to split this into an impl-specific size and init too?
Because there is no vsmmu_alloc in v6.16-rc1. I guess you are referring to older versions of HW queue (vCMDQ) series that was not merged?
Thanks
Nicolin
Pranjal Shrivastava
Reviewed-byRe: [PATCH v2 10/14] iommu/arm-smmu-v3: Replace arm_vsmmu_alloc with arm_vsmmu_init
Quoted 34 lines · Nicolin Chen
Quoted 7 lines · Pranjal Shrivastava
Quoted 6 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced.Sanitize the inputs and report the size of struct
arm_vsmmuon success, inarm_smmu_get_viommu_size().Quoted 8 lines
Quoted 5 lines
Place the type sanity at the last, becase there will be soon an impl level
get_viommu_sizeop, which will require the same sanity tests prior. It can simply insert a piece of code in front of theIOMMU_VIOMMU_TYPE_ARM_SMMUV3sanity.That's what I was wondering, so we plan to replace the
impl->vsmmu_allocop as well?There is no such op in v6.16-rc1.
Quoted 15 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, · line 438
Quoted 4 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +2
-1-return &vsmmu->core; + viommu->ops = &arm_vsmmu_ops; +return 0 ; }Seems much better now that the driver doesn't need to callback to the core for allocating viommu. One quick question though I see we've removed the following too:
if (master->smmu->impl_ops &&master->smmu->impl_ops->vsmmu_alloc) vsmmu = master->smmu->impl_ops->vsmmu_alloc( master->smmu, s2_parent, ictx, viommu_type, user_data);
Quoted 6 lines
Quoted 2 lines
Not sure why don't I see that in the diffs.. do we plan to split this into an impl-specific size and init too?
Because there is no
vsmmu_allocin v6.16-rc1. I guess you are referring to older versions of HW queue (vCMDQ) series that was not merged?
That's right, my bad. I rebased the older series while pulling, apologies for the confusion!
Quoted 3 lines
Thanks
Nicolin
Thanks!
PATCH v2 11/14 iommu: Deprecate viommu_alloc op
In reply to Nicolin ChenTo ease the for-driver iommufd APIs, get_viommu_size and viommu_init ops are introduced. Now, those existing vIOMMU supported drivers implemented these two ops, replacing the viommu_alloc one. So, there is no use of it.
Remove it from the headers and the viommu core.
--- include/linux/iommu.h | 11 ----------- include/linux/iommufd.h | 18 ------------------ drivers/iommu/iommufd/viommu.c | 20 +++++--------------- 3 files changed, 5 insertions(+), 44 deletions(-)
include/linux/iommu.h +0 -11
@@ -607,14 +607,6 @@ iommu_copy_struct_from_full_user_array(void *kdst, size_t kdst_entry_size,* resources shared/passed to user space IOMMU instance. Associate * it with a nesting @parent_domain. It is required for driver to * set @viommu->ops pointing to its own viommu_ops -* @viommu_alloc: Allocate an iommufd_viommu on a physical IOMMU instance behind -* the @dev, as the set of virtualization resources shared/passed -* to user space IOMMU instance. And associate it with a nesting -* @parent_domain. The @viommu_type must be defined in the header -* include/uapi/linux/iommufd.h -* It is required to call iommufd_viommu_alloc() helper for -* a bundled allocation of the core and the driver structures, -* using the given @ictx pointer. * @pgsize_bitmap: bitmap of all possible supported page sizes * @owner: Driver module providing these ops * @identity_domain: An always available, always attachable identity
@@ -669,9 +661,6 @@ struct iommu_ops { enum iommu_viommu_type viommu_type);int (*viommu_init)(struct iommufd_viommu *viommu,struct iommu_domain *parent_domain); -struct iommufd_viommu *(*viommu_alloc)( -struct device *dev,struct iommu_domain *parent_domain, -struct iommufd_ctx *ictx,unsigned int viommu_type);const struct iommu_domain_ops *default_domain_ops;unsigned long pgsize_bitmap;
include/linux/iommufd.h +0 -18
@@ -234,22 +234,4 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_viommu, \ ((drv_struct *)NULL)->member))) - -/* -* Helpers for IOMMU driver to allocate driver structures that will be freed by -* the iommufd core. The free op will be called prior to freeing the memory. -*/ -#define iommufd_viommu_alloc (ictx, drv_struct, member, viommu_ops) \ - ({ \ - drv_struct *ret; \ - \ - static_assert(__same_type(struct iommufd_viommu, \ - ((drv_struct *)NULL)->member)); \ - static_assert(offsetof(drv_struct, member.obj) == 0); \ - ret = (drv_struct *)_iommufd_object_alloc( \ - ictx, sizeof(drv_struct), IOMMUFD_OBJ_VIOMMU); \ - if (!IS_ERR(ret)) \ - ret->member.ops = viommu_ops; \ - ret; \ - })#endif
drivers/iommu/iommufd/viommu.c +5 -15
@@ -33,8 +33,6 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) ops =dev_iommu_ops (idev->dev);if (!ops->get_viommu_size || !ops->viommu_init) { -if (ops->viommu_alloc) - goto get_hwpt_paging; rc = -EOPNOTSUPP ; goto out_put_idev; }
@@ -54,7 +52,6 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } -get_hwpt_paging: hwpt_paging =iommufd_get_hwpt_paging (ucmd, cmd->hwpt_id);if (IS_ERR (hwpt_paging)) { rc =PTR_ERR (hwpt_paging);
@@ -66,13 +63,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_hwpt; } -if (ops->viommu_alloc) - viommu = ops->viommu_alloc (idev->dev, - hwpt_paging->common.domain, - ucmd->ictx, cmd->type); -else - viommu = (struct iommufd_viommu *)_iommufd_object_alloc ( - ucmd->ictx, viommu_size,IOMMUFD_OBJ_VIOMMU ); + viommu = (struct iommufd_viommu *)_iommufd_object_alloc ( + ucmd->ictx, viommu_size,IOMMUFD_OBJ_VIOMMU );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
@@ -92,11 +84,9 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)*/ viommu->iommu_dev =__iommu_get_iommu_dev (idev->dev); -if (!ops->viommu_alloc) { - rc = ops->viommu_init (viommu, hwpt_paging->common.domain); -if (rc) - goto out_abort; - } + rc = ops->viommu_init (viommu, hwpt_paging->common.domain); +if (rc) + goto out_abort;/* It is a driver bug that viommu->ops isn't filled */ if (WARN_ON_ONCE (!viommu->ops)) {
Signature · 1 line
2.43.0
Re: [PATCH v2 11/14] iommu: Deprecate viommu_alloc op
Quoted 9 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced. Now, those existing vIOMMU supported drivers implemented these two ops, replacing theviommu_allocone. So, there is no use of it.Remove it from the headers and the viommu core.
Suggested-by Jason Gunthorpe<[email protected]>Reviewed-by Kevin Tian<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 11/14] iommu: Deprecate viommu_alloc op
In reply to Nicolin ChenQuoted 6 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced. Now, those existing vIOMMU supported drivers implemented these two ops, replacing theviommu_allocone. So, there is no use of it.Remove it from the headers and the viommu core.
Quoted 8 lines
Suggested-by Jason Gunthorpe <[email protected]>Reviewed-by Kevin Tian <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- include/linux/iommu.h | 11 ----------- include/linux/iommufd.h | 18 ------------------ drivers/iommu/iommufd/viommu.c | 20 +++++--------------- 3 files changed, 5 insertions(+), 44 deletions(-)
Jason
Re: [PATCH v2 11/14] iommu: Deprecate viommu_alloc op
In reply to Nicolin ChenQuoted 6 lines · Nicolin Chen
To ease the for-driver iommufd APIs,
get_viommu_sizeandviommu_initops are introduced. Now, those existing vIOMMU supported drivers implemented these two ops, replacing theviommu_allocone. So, there is no use of it.Remove it from the headers and the viommu core.
Quoted 9 lines
Suggested-by Jason Gunthorpe <[email protected]>Reviewed-by Kevin Tian <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- include/linux/iommu.h | 11 ----------- include/linux/iommufd.h | 18 ------------------ drivers/iommu/iommufd/viommu.c | 20 +++++--------------- 3 files changed, 5 insertions(+), 44 deletions(-)
Much cleaner now IMO.
PATCH v2 12/14 iommufd: Move _iommufd_object_alloc out of driver.c
In reply to Nicolin ChenNow, all driver structures will be allocated by the core, i.e. no longer a need of driver calling _iommufd_object_alloc. Thus, move it back.
Before:
text data bss dec hex filename
3024 180 0 3204 c84 drivers/iommu/iommufd/driver.o 9074 610 64 9748 2614 drivers/iommu/iommufd/main.o
After:
text data bss dec hex filename
2665 164 0 2829 b0d drivers/iommu/iommufd/driver.o
9410 618 64 10092 276c drivers/iommu/iommufd/main.o
--- drivers/iommu/iommufd/iommufd_private.h | 4 +++ include/linux/iommufd.h | 10 -------- drivers/iommu/iommufd/driver.c | 33 ------------------------- drivers/iommu/iommufd/main.c | 32 ++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 43 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +4 -0
@@ -230,6 +230,10 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx,iommufd_object_remove (ictx, obj, obj->id,0 ); } +struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx, +size_t size, +enum iommufd_object_type type); +#define __iommufd_object_alloc (ictx, ptr, type, obj) \ container_of(_iommufd_object_alloc( \ ictx, \
include/linux/iommufd.h +0 -10
@@ -190,9 +190,6 @@ static inline int iommufd_vfio_compat_set_no_iommu(struct iommufd_ctx *ictx)#endif /* CONFIG_IOMMUFD */ #if IS_ENABLED (CONFIG_IOMMUFD_DRIVER_CORE ) -struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx, -size_t size, -enum iommufd_object_type type);struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id); int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
@@ -201,13 +198,6 @@ int iommufd_viommu_report_event(struct iommufd_viommu *viommu, enum iommu_veventq_type type, void *event_data, size_t data_len);#else /* !CONFIG_IOMMUFD_DRIVER_CORE */ -static inline struct iommufd_object * -_iommufd_object_alloc (struct iommufd_ctx *ictx,size_t size, -enum iommufd_object_type type) -{ -return ERR_PTR (-EOPNOTSUPP ); -} -static inline struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id) {
drivers/iommu/iommufd/driver.c +0 -33
@@ -3,39 +3,6 @@*/ #include "iommufd_private.h" -struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx, -size_t size, -enum iommufd_object_type type) -{ -struct iommufd_object *obj; -int rc; - - obj =kzalloc (size,GFP_KERNEL_ACCOUNT ); -if (!obj) -return ERR_PTR (-ENOMEM ); - obj->type = type; -/* Starts out bias'd by 1 until it is removed from the xarray */ -refcount_set (&obj->shortterm_users,1 ); -refcount_set (&obj->users,1 ); - -/* -* Reserve an ID in the xarray but do not publish the pointer yet since -* the caller hasn't initialized it yet. Once the pointer is published -* in the xarray and visible to other threads we can't reliably destroy -* it anymore, so the caller must complete all errorable operations -* before calling iommufd_object_finalize(). -*/ - rc =xa_alloc (&ictx->objects, &obj->id,XA_ZERO_ENTRY , xa_limit_31b, -GFP_KERNEL_ACCOUNT ); -if (rc) - goto out_free; -return obj; -out_free: -kfree (obj); -return ERR_PTR (rc); -} -EXPORT_SYMBOL_NS_GPL (_iommufd_object_alloc,"IOMMUFD" ); -/* Caller should xa_lock(&viommu->vdevs) to protect the return value */ struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id)
drivers/iommu/iommufd/main.c +32 -0
@@ -29,6 +29,38 @@ struct iommufd_object_ops {static const struct iommufd_object_ops iommufd_object_ops[];static struct miscdevice vfio_misc_dev; +struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx, +size_t size, +enum iommufd_object_type type) +{ +struct iommufd_object *obj; +int rc; + + obj =kzalloc (size,GFP_KERNEL_ACCOUNT ); +if (!obj) +return ERR_PTR (-ENOMEM ); + obj->type = type; +/* Starts out bias'd by 1 until it is removed from the xarray */ +refcount_set (&obj->shortterm_users,1 ); +refcount_set (&obj->users,1 ); + +/* +* Reserve an ID in the xarray but do not publish the pointer yet since +* the caller hasn't initialized it yet. Once the pointer is published +* in the xarray and visible to other threads we can't reliably destroy +* it anymore, so the caller must complete all errorable operations +* before calling iommufd_object_finalize(). +*/ + rc =xa_alloc (&ictx->objects, &obj->id,XA_ZERO_ENTRY , xa_limit_31b, +GFP_KERNEL_ACCOUNT ); +if (rc) + goto out_free; +return obj; +out_free: +kfree (obj); +return ERR_PTR (rc); +} +/* * Allow concurrent access to the object. *
Signature · 1 line
2.43.0
Re: [PATCH v2 12/14] iommufd: Move _iommufd_object_alloc out of driver.c
Quoted 12 lines · Nicolin Chen
Now, all driver structures will be allocated by the core, i.e. no longer a need of driver calling
_iommufd_object_alloc. Thus, move it back.Before:
text data bss dec hex filename 3024 180 0 3204 c84 drivers/iommu/iommufd/driver.o 9074 610 64 9748 2614 drivers/iommu/iommufd/main.o After: text data bss dec hex filename 2665 164 0 2829 b0d drivers/iommu/iommufd/driver.o 9410 618 64 10092 276c drivers/iommu/iommufd/main.o
Quoted 3 lines
Reviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
PATCH v2 13/14 iommufd: Introduce iommufd_object_alloc_ucmd helper
In reply to Nicolin ChenAn object allocator needs to call either iommufd_object_finalize() upon a success or iommufd_object_abort_and_destroy() upon an error code.
To reduce duplication, store a new_obj in the struct iommufd_ucmd and call iommufd_object_finalize/iommufd_object_abort_and_destroy() accordingly in the main function.
Similar to iommufd_object_alloc() and __iommufd_object_alloc(), add a pair of helpers: __iommufd_object_alloc_ucmd() and iommufd_object_alloc_ucmd().
--- drivers/iommu/iommufd/iommufd_private.h | 26 +++++++++++++++++++++++++ drivers/iommu/iommufd/main.c | 25 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+)
drivers/iommu/iommufd/iommufd_private.h +26 -0
@@ -135,6 +135,7 @@ struct iommufd_ucmd {void __user *ubuffer;u32 user_size;void *cmd; +struct iommufd_object *new_obj; }; int iommufd_vfio_ioctl(struct iommufd_ctx *ictx, unsigned int cmd,
@@ -230,6 +231,11 @@ iommufd_object_put_and_try_destroy(struct iommufd_ctx *ictx,iommufd_object_remove (ictx, obj, obj->id,0 ); } +/* +* Callers of these normal object allocators must call iommufd_object_finalize() +* to finalize the object, or call iommufd_object_abort_and_destroy() to revert +* the allocation. +*/ struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx,size_t size,enum iommufd_object_type type);
@@ -246,6 +252,26 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,#define iommufd_object_alloc (ictx, ptr, type) \ __iommufd_object_alloc(ictx, ptr, type, obj) +/* +* Callers of these _ucmd allocators should not call iommufd_object_finalize() +* or iommufd_object_abort_and_destroy(), as the core automatically does that. +*/ +struct iommufd_object * +_iommufd_object_alloc_ucmd (struct iommufd_ucmd *ucmd,size_t size, +enum iommufd_object_type type); + +#define __iommufd_object_alloc_ucmd (ucmd, ptr, type, obj) \ + container_of(_iommufd_object_alloc_ucmd( \ + ucmd, \ + sizeof(*(ptr)) + BUILD_BUG_ON_ZERO( \ + offsetof(typeof(*(ptr)), \ + obj) != 0), \ + type), \ + typeof(*(ptr)), obj) + +#define iommufd_object_alloc_ucmd (ucmd, ptr, type) \ + __iommufd_object_alloc_ucmd(ucmd, ptr, type, obj) +/* * The IO Address Space (IOAS) pagetable is a virtual page table backed by the * io_pagetable object. It is a user controlled mapping of IOVA -> PFNs. The
drivers/iommu/iommufd/main.c +25 -0
@@ -61,6 +61,24 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,return ERR_PTR (rc); } +struct iommufd_object *_iommufd_object_alloc_ucmd (struct iommufd_ucmd *ucmd, +size_t size, +enum iommufd_object_type type) +{ +struct iommufd_object *new_obj; + +/* Something is coded wrong if this is hit */ +if (WARN_ON (ucmd->new_obj)) +return ERR_PTR (-EBUSY ); + + new_obj =_iommufd_object_alloc (ucmd->ictx, size, type); +if (IS_ERR (new_obj)) +return new_obj; + + ucmd->new_obj = new_obj; +return new_obj; +} +/* * Allow concurrent access to the object. *
@@ -448,6 +466,13 @@ static long iommufd_fops_ioctl(struct file *filp, unsigned int cmd,if (ret)return ret; ret = op->execute (&ucmd); + +if (ucmd.new_obj) { +if (ret) +iommufd_object_abort_and_destroy (ictx, ucmd.new_obj); +else +iommufd_object_finalize (ictx, ucmd.new_obj); + }return ret; }
Signature · 1 line
2.43.0
Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd helper
Quoted 7 lines · Nicolin Chen
An object allocator needs to call either
iommufd_object_finalize()upon a success oriommufd_object_abort_and_destroy()upon an error code.To reduce duplication, store a
new_objin the structiommufd_ucmdand call iommufd_object_finalize/iommufd_object_abort_and_destroy() accordingly in the main function.
Quoted 7 lines
Similar to
iommufd_object_alloc()and__iommufd_object_alloc(), add a pair of helpers:__iommufd_object_alloc_ucmd()andiommufd_object_alloc_ucmd().Suggested-by Jason Gunthorpe<[email protected]>Reviewed-by Kevin Tian<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd helpery
In reply to Nicolin ChenQuoted 10 lines · Nicolin Chen
An object allocator needs to call either
iommufd_object_finalize()upon a success oriommufd_object_abort_and_destroy()upon an error code.To reduce duplication, store a
new_objin the structiommufd_ucmdand call iommufd_object_finalize/iommufd_object_abort_and_destroy() accordingly in the main function.Similar to
iommufd_object_alloc()and__iommufd_object_alloc(), add a pair of helpers:__iommufd_object_alloc_ucmd()andiommufd_object_alloc_ucmd().
Quoted 10 lines
Suggested-by Jason Gunthorpe <[email protected]>Reviewed-by Kevin Tian <[email protected]>Reviewed-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/iommufd_private.h | 26 +++++++++++++++++++++++++ drivers/iommu/iommufd/main.c | 25 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+)
Quoted 2 lines
2.43.0
Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd helper
In reply to Nicolin ChenQuoted 26 lines
drivers/iommu/iommufd/main.c +18
-0@@ -61,6 +61,24 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,return ERR_PTR (rc); } +struct iommufd_object *_iommufd_object_alloc_ucmd (struct iommufd_ucmd *ucmd, +size_t size, +enum iommufd_object_type type) +{ +struct iommufd_object *new_obj; + +/* Something is coded wrong if this is hit */ +if (WARN_ON (ucmd->new_obj)) +return ERR_PTR (-EBUSY ); + + new_obj =_iommufd_object_alloc (ucmd->ictx, size, type); +if (IS_ERR (new_obj)) +return new_obj; + + ucmd->new_obj = new_obj; +return new_obj; +} +/* * Allow concurrent access to the object. *
drivers/iommu/iommufd/main.c static long iommufd_fops_ioctl(struct file *filp, unsigned int cmd, · line 472
Quoted 7 lines
drivers/iommu/iommufd/main.c +4
-0@@ -448,6 +466,13 @@ static long iommufd_fops_ioctl(struct file *filp, unsigned int cmd,if (ret)return ret; ret = op->execute (&ucmd); + +if (ucmd.new_obj) { +if (ret) +iommufd_object_abort_and_destroy (ictx, ucmd.new_obj);
Sorry I didn't follow this thread before and maybe missed something.
According to 70eadc7fc7ef, abort op is for the object that can assume the caller is holding the lock. But here is for no locking, so calling iommufd_object_abort_and_destroy() is quite confusing.
Is it better we change to:
if (ret) {
iommufd_object_ops[obj->type].destroy(obj);
iommufd_object_abort(ictx, obj);
}Also explicitely assert iommufd_object_alloc_ucmd() and abort can't be used at the same time.
in _iommufd_object_alloc_ucmd():
if (WARN_ON(iommufd_object_ops[type].abort)) return ERR_PTR(-EFAULT);
Thanks,
Yilun
Quoted 10 lines
drivers/iommu/iommufd/main.c +3
-0+else +iommufd_object_finalize (ictx, ucmd.new_obj); + }return ret; }Signature · 1 line
2.43.0
Tian, Kevin
maintainerRE: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd helper
Quoted 48 lines
From: Xu Yilun <[email protected]>
Sent: Wednesday, July 9, 2025 1:32 PMReview struct iommufd_object
Quoted 1 line
(quoted diff) +0
-0@@ -61,6 +61,24 @@ struct iommufd_object*_iommufd_object_alloc(struct
iommufd_ctx*ictx,drivers/iommu/iommufd/main.c struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
Quoted 4 lines
drivers/iommu/iommufd/main.c +1
-0return ERR_PTR (rc); } +struct iommufd_object *_iommufd_object_alloc_ucmd(struct
iommufd_ucmd*ucmd,Review struct iommufd_object · line 66
Quoted 2 lines
(quoted diff) +2
-0+ size_t size, + enum iommufd_object_typetype)
drivers/iommu/iommufd/main.c static long iommufd_fops_ioctl(struct file *filp,
Quoted 19 lines
drivers/iommu/iommufd/main.c +15
-0+{ +struct iommufd_object *new_obj; + +/* Something is coded wrong if this is hit */ +if (WARN_ON (ucmd->new_obj)) +return ERR_PTR (-EBUSY ); + + new_obj =_iommufd_object_alloc (ucmd->ictx, size, type); +if (IS_ERR (new_obj)) +return new_obj; + + ucmd->new_obj = new_obj; +return new_obj; +} +/* * Allow concurrent access to the object. * @@ -448,6 +466,13 @@ static long iommufd_fops_ioctl(struct file *filp,unsigned int cmd,
drivers/iommu/iommufd/main.c static long iommufd_fops_ioctl(struct file *filp, unsigned int cmd,
Quoted 7 lines
drivers/iommu/iommufd/main.c +4
-0if (ret)return ret; ret = op->execute (&ucmd); + +if (ucmd.new_obj) { +if (ret) + iommufd_object_abort_and_destroy(ictx,ucmd.new_obj);
Sorry I didn't follow this thread before and maybe missed something.
According to
70eadc7fc7ef, abort op is for the object that can assume the caller is holding the lock. But here is for no locking, so callingiommufd_object_abort_and_destroy()is quite confusing.
Quoted 6 lines
Is it better we change to:
if (ret) { iommufd_object_ops[obj->type].destroy(obj); iommufd_object_abort(ictx, obj); }
I'd keep the original way. The function name describes what to do, not what to be called exactly inside. Lacking of the abort method doesn't change the meaning of the function which is about abort and destroy (just like how it's called before introducing @abort).
Quoted 9 lines
Also explicitely assert
iommufd_object_alloc_ucmd()and abort can't be used at the same time.in
_iommufd_object_alloc_ucmd():if (WARN_ON(iommufd_object_ops[type].abort)) return ERR_PTR(-EFAULT);
but this check sounds necessary.
Re: [PATCH v2 13/14] iommufd: Introduce iommufd_object_alloc_ucmd helper
Quoted 11 lines · Kevin
Quoted 9 lines
From: Xu Yilun <[email protected]>
Also explicitely assertiommufd_object_alloc_ucmd()and abort can't be used at the same time.in
_iommufd_object_alloc_ucmd():if (WARN_ON(iommufd_object_ops[type].abort)) return ERR_PTR(-EFAULT);but this check sounds necessary.
Let me send a patch for this.
Thanks
Nicolin
PATCH v2 14/14 iommufd: Apply the new iommufd_object_alloc_ucmd helper
In reply to Nicolin ChenNow the new ucmd-based object allocator eases the finalize/abort routine, apply this to all existing allocators that aren't protected by any lock.
Upgrade the for-driver vIOMMU alloctor too, and pass down to all existing viommu_alloc op accordingly.
Note that __iommufd_object_alloc_ucmd() builds in some static tests that cover both static_asserts in the iommufd_viommu_alloc(). Thus drop them.
--- drivers/iommu/iommufd/eventq.c | 14 ++++---------- drivers/iommu/iommufd/viommu.c | 24 ++++++------------------ 2 files changed, 10 insertions(+), 28 deletions(-)
drivers/iommu/iommufd/eventq.c +4 -10
@@ -427,8 +427,8 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd)if (cmd->flags)return -EOPNOTSUPP ; - fault =__iommufd_object_alloc (ucmd->ictx, fault,IOMMUFD_OBJ_FAULT , - common.obj); + fault =__iommufd_object_alloc_ucmd (ucmd, fault,IOMMUFD_OBJ_FAULT , + common.obj);if (IS_ERR (fault))return PTR_ERR (fault);
@@ -437,10 +437,8 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) fdno =iommufd_eventq_init (&fault->common,"[iommufd-pgfault]" , ucmd->ictx, &iommufd_fault_fops); -if (fdno <0 ) { - rc = fdno; - goto out_abort; - } +if (fdno <0 ) +return fdno; cmd->out_fault_id = fault->common.obj.id; cmd->out_fault_fd = fdno;
@@ -448,7 +446,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd));if (rc) goto out_put_fdno; -iommufd_object_finalize (ucmd->ictx, &fault->common.obj);fd_install (fdno, fault->common.filep);
@@ -456,9 +453,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd) out_put_fdno:put_unused_fd (fdno);fput (fault->common.filep); -out_abort: -iommufd_object_abort_and_destroy (ucmd->ictx, &fault->common.obj); -return rc; }
drivers/iommu/iommufd/viommu.c +6 -18
@@ -63,8 +63,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_hwpt; } - viommu = (struct iommufd_viommu *)_iommufd_object_alloc ( - ucmd->ictx, viommu_size,IOMMUFD_OBJ_VIOMMU ); + viommu = (struct iommufd_viommu *)_iommufd_object_alloc_ucmd ( + ucmd, viommu_size,IOMMUFD_OBJ_VIOMMU );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
@@ -86,23 +86,17 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) rc = ops->viommu_init (viommu, hwpt_paging->common.domain);if (rc) - goto out_abort; + goto out_put_hwpt;/* It is a driver bug that viommu->ops isn't filled */ if (WARN_ON_ONCE (!viommu->ops)) { rc = -EOPNOTSUPP ; - goto out_abort; + goto out_put_hwpt; } cmd->out_viommu_id = viommu->obj.id; rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd)); -if (rc) - goto out_abort; -iommufd_object_finalize (ucmd->ictx, &viommu->obj); - goto out_put_hwpt; -out_abort: -iommufd_object_abort_and_destroy (ucmd->ictx, &viommu->obj); out_put_hwpt:iommufd_put_object (ucmd->ictx, &hwpt_paging->common.obj); out_put_idev:
@@ -150,7 +144,7 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } - vdev =iommufd_object_alloc (ucmd->ictx, vdev,IOMMUFD_OBJ_VDEVICE ); + vdev =iommufd_object_alloc_ucmd (ucmd, vdev,IOMMUFD_OBJ_VDEVICE );if (IS_ERR (vdev)) { rc =PTR_ERR (vdev); goto out_put_idev;
@@ -165,18 +159,12 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) curr =xa_cmpxchg (&viommu->vdevs, virt_id,NULL , vdev,GFP_KERNEL );if (curr) { rc =xa_err (curr) ?: -EEXIST ; - goto out_abort; + goto out_put_idev; } cmd->out_vdevice_id = vdev->obj.id; rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd)); -if (rc) - goto out_abort; -iommufd_object_finalize (ucmd->ictx, &vdev->obj); - goto out_put_idev; -out_abort: -iommufd_object_abort_and_destroy (ucmd->ictx, &vdev->obj); out_put_idev:iommufd_put_object (ucmd->ictx, &idev->obj); out_put_viommu:
Signature · 1 line
2.43.0
Re: [PATCH v2 14/14] iommufd: Apply the new iommufd_object_alloc_ucmd helper
Quoted 6 lines · Nicolin Chen
Now the new ucmd-based object allocator eases the finalize/abort routine, apply this to all existing allocators that aren't protected by any lock.
Upgrade the for-driver vIOMMU alloctor too, and pass down to all existing
viommu_allocop accordingly.
Quoted 6 lines
Note that
__iommufd_object_alloc_ucmd()builds in some static tests that cover bothstatic_assertsin theiommufd_viommu_alloc(). Thus drop them.Suggested-by Jason Gunthorpe<[email protected]>Reviewed-by Jason Gunthorpe<[email protected]>Signed-off-by Nicolin Chen<[email protected]>
RE: [PATCH v2 14/14] iommufd: Apply the new iommufd_object_alloc_ucmd helper
In reply to Nicolin ChenQuoted 9 lines
From: Nicolin Chen <[email protected]>
Sent: Saturday, June 14, 2025 2:35 PMNow the new ucmd-based object allocator eases the finalize/abort routine, apply this to all existing allocators that aren't protected by any lock.
Upgrade the for-driver vIOMMU alloctor too, and pass down to all existing
viommu_allocop accordingly.
Quoted 6 lines
Note that
__iommufd_object_alloc_ucmd()builds in some static tests that cover bothstatic_assertsin theiommufd_viommu_alloc(). Thus drop them.Suggested-by Jason Gunthorpe <[email protected]>Reviewed-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>
Quoted 8 lines · Nicolin Chen
Nicolin Chen (14):
iommufd: Apply obvious cosmetic fixes
iommufd: Drop unused ictx in structiommufd_vdevice
iommufd: Use enumiommu_viommu_typefor type in structiommufd_viommu
iommufd: Use enumiommu_veventq_typefor type in struct
iommufd_veventq
iommufd: Return EOPNOTSUPP for failures due to driver bugs
iommu: Introduceget_viommu_sizeandviommu_initops
Quoted 8 lines
iommufd/viommu: Support
get_viommu_sizeandviommu_initops iommufd/selftest: Drop parent domain frommock_iommu_domain_nestediommufd/selftest: Replacemock_viommu_allocwithmock_viommu_initiommu/arm-smmu-v3: Replacearm_vsmmu_allocwitharm_vsmmu_init
iommu: Deprecateviommu_allocop
iommufd: Move_iommufd_object_allocout ofdriver.c
iommufd: Introduceiommufd_object_alloc_ucmdhelper
iommufd: Apply the newiommufd_object_alloc_ucmdhelper
Applied to iommfd for-next, thanks
Jason