| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <62ccc75d-3f30-4167-b9e1-21dd95a6631d@intel.com>
Date: Sat, 18 Jan 2025 16:23:22 +0800
From: Yi Liu <yi.l.liu@...el.com>
To: Nicolin Chen <nicolinc@...dia.com>, <will@...nel.org>,
<robin.murphy@....com>, <jgg@...dia.com>, <kevin.tian@...el.com>,
<tglx@...utronix.de>, <maz@...nel.org>, <alex.williamson@...hat.com>
CC: <joro@...tes.org>, <shuah@...nel.org>, <reinette.chatre@...el.com>,
<eric.auger@...hat.com>, <yebin10@...wei.com>, <apatel@...tanamicro.com>,
<shivamurthy.shastri@...utronix.de>, <bhelgaas@...gle.com>,
<anna-maria@...utronix.de>, <yury.norov@...il.com>, <nipun.gupta@....com>,
<iommu@...ts.linux.dev>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <kvm@...r.kernel.org>,
<linux-kselftest@...r.kernel.org>, <patches@...ts.linux.dev>,
<jean-philippe@...aro.org>, <mdf@...nel.org>, <mshavit@...gle.com>,
<shameerali.kolothum.thodi@...wei.com>, <smostafa@...gle.com>,
<ddutile@...hat.com>
Subject: Re: [PATCH RFCv2 06/13] iommufd: Make attach_handle generic
On 2025/1/11 11:32, Nicolin Chen wrote:
> "attach_handle" was added exclusively for the iommufd_fault_iopf_handler()
> used by IOPF/PRI use cases, along with the "fault_data". Now, the iommufd
> version of sw_msi function will resue the attach_handle and fault_data for
> a non-fault case.
>
> Move the attach_handle part out of the fault.c file to make it generic for
> all cases. Simplify the remaining fault specific routine to attach/detach.
I guess you can send it separately since both of our series need it. :)
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/iommu/iommufd/iommufd_private.h | 40 +-------
> drivers/iommu/iommufd/device.c | 105 +++++++++++++++++++++
> drivers/iommu/iommufd/fault.c | 120 +++---------------------
> 3 files changed, 122 insertions(+), 143 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index b6d706cf2c66..063c0a42f54f 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -472,42 +472,12 @@ void iommufd_fault_destroy(struct iommufd_object *obj);
> int iommufd_fault_iopf_handler(struct iopf_group *group);
>
> int iommufd_fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev);
> + struct iommufd_device *idev,
> + bool enable_iopf);
> void iommufd_fault_domain_detach_dev(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev);
> -int iommufd_fault_domain_replace_dev(struct iommufd_device *idev,
> - struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_hw_pagetable *old);
> -
> -static inline int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev)
> -{
> - if (hwpt->fault)
> - return iommufd_fault_domain_attach_dev(hwpt, idev);
> -
> - return iommu_attach_group(hwpt->domain, idev->igroup->group);
> -}
> -
> -static inline void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev)
> -{
> - if (hwpt->fault) {
> - iommufd_fault_domain_detach_dev(hwpt, idev);
> - return;
> - }
> -
> - iommu_detach_group(hwpt->domain, idev->igroup->group);
> -}
> -
> -static inline int iommufd_hwpt_replace_device(struct iommufd_device *idev,
> - struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_hw_pagetable *old)
> -{
> - if (old->fault || hwpt->fault)
> - return iommufd_fault_domain_replace_dev(idev, hwpt, old);
> -
> - return iommu_group_replace_domain(idev->igroup->group, hwpt->domain);
> -}
> + struct iommufd_device *idev,
> + struct iommufd_attach_handle *handle,
> + bool disable_iopf);
>
> static inline struct iommufd_viommu *
> iommufd_get_viommu(struct iommufd_ucmd *ucmd, u32 id)
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index dfd0898fb6c1..38b31b652147 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -352,6 +352,111 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
> return 0;
> }
>
> +/* The device attach/detach/replace helpers for attach_handle */
> +
> +static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
> + struct iommufd_device *idev)
> +{
> + struct iommufd_attach_handle *handle;
> + int rc;
> +
> + if (hwpt->fault) {
> + rc = iommufd_fault_domain_attach_dev(hwpt, idev, true);
> + if (rc)
> + return rc;
> + }
> +
> + handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> + if (!handle) {
> + rc = -ENOMEM;
> + goto out_fault_detach;
> + }
> +
> + handle->idev = idev;
> + rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
> + &handle->handle);
> + if (rc)
> + goto out_free_handle;
> +
> + return 0;
> +
> +out_free_handle:
> + kfree(handle);
> + handle = NULL;
> +out_fault_detach:
> + if (hwpt->fault)
> + iommufd_fault_domain_detach_dev(hwpt, idev, handle, true);
> + return rc;
> +}
> +
> +static struct iommufd_attach_handle *
> +iommufd_device_get_attach_handle(struct iommufd_device *idev)
> +{
> + struct iommu_attach_handle *handle;
> +
> + handle =
> + iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
> + if (IS_ERR(handle))
> + return NULL;
> + return to_iommufd_handle(handle);
> +}
> +
> +static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
> + struct iommufd_device *idev)
> +{
> + struct iommufd_attach_handle *handle;
> +
> + handle = iommufd_device_get_attach_handle(idev);
> + iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
> + if (hwpt->fault)
> + iommufd_fault_domain_detach_dev(hwpt, idev, handle, true);
> + kfree(handle);
> +}
> +
> +static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
> + struct iommufd_hw_pagetable *hwpt,
> + struct iommufd_hw_pagetable *old)
> +{
> + struct iommufd_attach_handle *old_handle =
> + iommufd_device_get_attach_handle(idev);
> + struct iommufd_attach_handle *handle;
> + int rc;
> +
> + if (hwpt->fault) {
> + rc = iommufd_fault_domain_attach_dev(hwpt, idev, !old->fault);
> + if (rc)
> + return rc;
> + }
> +
> + handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> + if (!handle) {
> + rc = -ENOMEM;
> + goto out_fault_detach;
> + }
> +
> + handle->idev = idev;
> + rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,
> + &handle->handle);
> + if (rc)
> + goto out_free_handle;
> +
> + if (old->fault)
> + iommufd_fault_domain_detach_dev(old, idev, old_handle,
> + !hwpt->fault);
> + kfree(old_handle);
> +
> + return 0;
> +
> +out_free_handle:
> + kfree(handle);
> + handle = NULL;
> +out_fault_detach:
> + if (hwpt->fault)
> + iommufd_fault_domain_detach_dev(hwpt, idev, handle,
> + !old->fault);
> + return rc;
> +}
> +
> int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
> struct iommufd_device *idev)
> {
> diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
> index 06aa83a75e94..1d9bd3024b57 100644
> --- a/drivers/iommu/iommufd/fault.c
> +++ b/drivers/iommu/iommufd/fault.c
> @@ -60,42 +60,17 @@ static void iommufd_fault_iopf_disable(struct iommufd_device *idev)
> mutex_unlock(&idev->iopf_lock);
> }
>
> -static int __fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev)
> -{
> - struct iommufd_attach_handle *handle;
> - int ret;
> -
> - handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> - if (!handle)
> - return -ENOMEM;
> -
> - handle->idev = idev;
> - ret = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
> - &handle->handle);
> - if (ret)
> - kfree(handle);
> -
> - return ret;
> -}
> -
> int iommufd_fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev)
> + struct iommufd_device *idev,
> + bool enable_iopf)
> {
> - int ret;
> + int rc = 0;
>
> if (!hwpt->fault)
> return -EINVAL;
> -
> - ret = iommufd_fault_iopf_enable(idev);
> - if (ret)
> - return ret;
> -
> - ret = __fault_domain_attach_dev(hwpt, idev);
> - if (ret)
> - iommufd_fault_iopf_disable(idev);
> -
> - return ret;
> + if (enable_iopf)
> + rc = iommufd_fault_iopf_enable(idev);
> + return rc;
> }
>
> static void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt,
> @@ -127,86 +102,15 @@ static void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt,
> mutex_unlock(&fault->mutex);
> }
>
> -static struct iommufd_attach_handle *
> -iommufd_device_get_attach_handle(struct iommufd_device *idev)
> -{
> - struct iommu_attach_handle *handle;
> -
> - handle = iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
> - if (IS_ERR(handle))
> - return NULL;
> -
> - return to_iommufd_handle(handle);
> -}
> -
> void iommufd_fault_domain_detach_dev(struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_device *idev)
> + struct iommufd_device *idev,
> + struct iommufd_attach_handle *handle,
> + bool disable_iopf)
> {
> - struct iommufd_attach_handle *handle;
> -
> - handle = iommufd_device_get_attach_handle(idev);
> - iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
> - iommufd_auto_response_faults(hwpt, handle);
> - iommufd_fault_iopf_disable(idev);
> - kfree(handle);
> -}
> -
> -static int __fault_domain_replace_dev(struct iommufd_device *idev,
> - struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_hw_pagetable *old)
> -{
> - struct iommufd_attach_handle *handle, *curr = NULL;
> - int ret;
> -
> - if (old->fault)
> - curr = iommufd_device_get_attach_handle(idev);
> -
> - if (hwpt->fault) {
> - handle = kzalloc(sizeof(*handle), GFP_KERNEL);
> - if (!handle)
> - return -ENOMEM;
> -
> - handle->idev = idev;
> - ret = iommu_replace_group_handle(idev->igroup->group,
> - hwpt->domain, &handle->handle);
> - } else {
> - ret = iommu_replace_group_handle(idev->igroup->group,
> - hwpt->domain, NULL);
> - }
> -
> - if (!ret && curr) {
> - iommufd_auto_response_faults(old, curr);
> - kfree(curr);
> - }
> -
> - return ret;
> -}
> -
> -int iommufd_fault_domain_replace_dev(struct iommufd_device *idev,
> - struct iommufd_hw_pagetable *hwpt,
> - struct iommufd_hw_pagetable *old)
> -{
> - bool iopf_off = !hwpt->fault && old->fault;
> - bool iopf_on = hwpt->fault && !old->fault;
> - int ret;
> -
> - if (iopf_on) {
> - ret = iommufd_fault_iopf_enable(idev);
> - if (ret)
> - return ret;
> - }
> -
> - ret = __fault_domain_replace_dev(idev, hwpt, old);
> - if (ret) {
> - if (iopf_on)
> - iommufd_fault_iopf_disable(idev);
> - return ret;
> - }
> -
> - if (iopf_off)
> + if (handle)
> + iommufd_auto_response_faults(hwpt, handle);
no need to check handle. After this patch, both the non-fault and the fault
path will allocate handle. It cannot be used to isolate fault and non-fault
path. Also, the callers of iommufd_fault_domain_detach_dev() will check
hwpt->fault before calling it. So just call iommufd_auto_response_faults().
> + if (disable_iopf)
> iommufd_fault_iopf_disable(idev);
> -
> - return 0;
> }
>
> void iommufd_fault_destroy(struct iommufd_object *obj)
--
Regards,
Yi Liu
Powered by blists - more mailing lists