From: Yi Liu <yi.l.liu@intel.com>
To: Nicolin Chen <nicolinc@nvidia.com>, <will@kernel.org>,
<robin.murphy@arm.com>, <jgg@nvidia.com>, <kevin.tian@intel.com>,
<tglx@linutronix.de>, <maz@kernel.org>,
<alex.williamson@redhat.com>
Cc: <joro@8bytes.org>, <shuah@kernel.org>,
<reinette.chatre@intel.com>, <eric.auger@redhat.com>,
<yebin10@huawei.com>, <apatel@ventanamicro.com>,
<shivamurthy.shastri@linutronix.de>, <bhelgaas@google.com>,
<anna-maria@linutronix.de>, <yury.norov@gmail.com>,
<nipun.gupta@amd.com>, <iommu@lists.linux.dev>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>, <kvm@vger.kernel.org>,
<linux-kselftest@vger.kernel.org>, <patches@lists.linux.dev>,
<jean-philippe@linaro.org>, <mdf@kernel.org>,
<mshavit@google.com>, <shameerali.kolothum.thodi@huawei.com>,
<smostafa@google.com>, <ddutile@redhat.com>
Subject: Re: [PATCH RFCv2 06/13] iommufd: Make attach_handle generic
Date: Sat, 18 Jan 2025 16:23:22 +0800 [thread overview]
Message-ID: <62ccc75d-3f30-4167-b9e1-21dd95a6631d@intel.com> (raw)
In-Reply-To: <c708aedc678c63e2466b43ab9d4f8ac876e49aa1.1736550979.git.nicolinc@nvidia.com>
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@nvidia.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
next prev parent reply other threads:[~2025-01-18 8:18 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-11 3:32 [PATCH RFCv2 00/13] iommu: Add MSI mapping support with nested SMMU Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 01/13] genirq/msi: Store the IOMMU IOVA directly in msi_desc instead of iommu_cookie Nicolin Chen
2025-01-23 17:10 ` Eric Auger
2025-01-23 18:48 ` Jason Gunthorpe
2025-01-29 12:11 ` Eric Auger
2025-01-11 3:32 ` [PATCH RFCv2 02/13] genirq/msi: Rename iommu_dma_compose_msi_msg() to msi_msg_set_msi_addr() Nicolin Chen
2025-01-23 17:10 ` Eric Auger
2025-01-23 18:50 ` Jason Gunthorpe
2025-01-29 10:44 ` Eric Auger
2025-01-11 3:32 ` [PATCH RFCv2 03/13] iommu: Make iommu_dma_prepare_msi() into a generic operation Nicolin Chen
2025-01-23 17:10 ` Eric Auger
2025-01-23 18:16 ` Jason Gunthorpe
2025-01-29 12:29 ` Eric Auger
2025-01-11 3:32 ` [PATCH RFCv2 04/13] irqchip: Have CONFIG_IRQ_MSI_IOMMU be selected by the irqchips that need it Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 05/13] iommu: Turn fault_data to iommufd private pointer Nicolin Chen
2025-01-23 9:54 ` Tian, Kevin
2025-01-23 13:25 ` Jason Gunthorpe
2025-01-29 12:40 ` Eric Auger
2025-02-03 17:48 ` Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 06/13] iommufd: Make attach_handle generic Nicolin Chen
2025-01-18 8:23 ` Yi Liu [this message]
2025-01-18 20:32 ` Nicolin Chen
2025-01-19 10:40 ` Yi Liu
2025-01-20 5:54 ` Nicolin Chen
2025-01-24 13:31 ` Yi Liu
2025-01-20 14:20 ` Jason Gunthorpe
2025-01-29 13:14 ` Eric Auger
2025-02-03 18:08 ` Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 07/13] iommufd: Implement sw_msi support natively Nicolin Chen
2025-01-15 4:21 ` Yury Norov
2025-01-16 20:21 ` Jason Gunthorpe
2025-01-23 19:30 ` Jason Gunthorpe
2025-01-11 3:32 ` [PATCH RFCv2 08/13] iommu: Turn iova_cookie to dma-iommu private pointer Nicolin Chen
2025-01-13 16:40 ` Jason Gunthorpe
2025-01-11 3:32 ` [PATCH RFCv2 09/13] iommufd: Add IOMMU_OPTION_SW_MSI_START/SIZE ioctls Nicolin Chen
2025-01-23 10:07 ` Tian, Kevin
2025-02-03 18:36 ` Nicolin Chen
2025-01-29 13:44 ` Eric Auger
2025-01-29 14:58 ` Jason Gunthorpe
2025-01-29 17:23 ` Eric Auger
2025-01-29 17:39 ` Jason Gunthorpe
2025-01-29 17:49 ` Eric Auger
2025-01-29 20:15 ` Jason Gunthorpe
2025-02-07 4:26 ` Nicolin Chen
2025-02-07 14:30 ` Jason Gunthorpe
2025-02-07 15:28 ` Jason Gunthorpe
2025-02-07 18:59 ` Nicolin Chen
2025-02-09 18:09 ` Jason Gunthorpe
2025-01-11 3:32 ` [PATCH RFCv2 10/13] iommufd/selftes: Add coverage for IOMMU_OPTION_SW_MSI_START/SIZE Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 11/13] iommufd/device: Allow setting IOVAs for MSI(x) vectors Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 12/13] vfio-iommufd: Provide another layer of msi_iova helpers Nicolin Chen
2025-01-11 3:32 ` [PATCH RFCv2 13/13] vfio/pci: Allow preset MSI IOVAs via VFIO_IRQ_SET_ACTION_PREPARE Nicolin Chen
2025-01-23 9:06 ` [PATCH RFCv2 00/13] iommu: Add MSI mapping support with nested SMMU Shameerali Kolothum Thodi
2025-01-23 13:24 ` Jason Gunthorpe
2025-01-29 14:54 ` Eric Auger
2025-01-29 15:04 ` Jason Gunthorpe
2025-01-29 17:46 ` Eric Auger
2025-01-29 20:13 ` Jason Gunthorpe
2025-02-04 12:55 ` Eric Auger
2025-02-04 13:02 ` Jason Gunthorpe
2025-02-05 22:49 ` Jacob Pan
2025-02-05 22:56 ` Nicolin Chen
2025-02-07 14:34 ` Jason Gunthorpe
2025-02-07 14:42 ` Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=62ccc75d-3f30-4167-b9e1-21dd95a6631d@intel.com \
--to=yi.l.liu@intel.com \
--cc=alex.williamson@redhat.com \
--cc=anna-maria@linutronix.de \
--cc=apatel@ventanamicro.com \
--cc=bhelgaas@google.com \
--cc=ddutile@redhat.com \
--cc=eric.auger@redhat.com \
--cc=iommu@lists.linux.dev \
--cc=jean-philippe@linaro.org \
--cc=jgg@nvidia.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=maz@kernel.org \
--cc=mdf@kernel.org \
--cc=mshavit@google.com \
--cc=nicolinc@nvidia.com \
--cc=nipun.gupta@amd.com \
--cc=patches@lists.linux.dev \
--cc=reinette.chatre@intel.com \
--cc=robin.murphy@arm.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=shivamurthy.shastri@linutronix.de \
--cc=shuah@kernel.org \
--cc=smostafa@google.com \
--cc=tglx@linutronix.de \
--cc=will@kernel.org \
--cc=yebin10@huawei.com \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox