| 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: <Zqqq5JYFswS49z2L@Asurada-Nvidia>
Date: Wed, 31 Jul 2024 14:21:40 -0700
From: Nicolin Chen <nicolinc@...dia.com>
To: "jgg@...dia.com" <jgg@...dia.com>, "Tian, Kevin" <kevin.tian@...el.com>
CC: "Liu, Yi L" <yi.l.liu@...el.com>, "iommu@...ts.linux.dev"
<iommu@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] iommufd: Enforce IOMMU_RESV_SW_MSI upon hwpt_paging
allocation
On Wed, Jul 31, 2024 at 11:13:11AM -0700, Nicolin Chen wrote:
> On Wed, Jul 31, 2024 at 07:45:46AM +0000, Tian, Kevin wrote:
> > > From: Nicolin Chen <nicolinc@...dia.com>
> > > Sent: Monday, July 29, 2024 7:51 AM
> > > @@ -364,7 +305,8 @@ int iommufd_hw_pagetable_attach(struct
> > > iommufd_hw_pagetable *hwpt,
> > > }
> > >
> > > if (hwpt_is_paging(hwpt)) {
> > > - rc = iommufd_hwpt_paging_attach(to_hwpt_paging(hwpt),
> > > idev);
> > > + rc = iopt_table_enforce_dev_resv_regions(
> > > + &to_hwpt_paging(hwpt)->ioas->iopt, idev-
> > > >dev);
> >
> > Is it simpler to extend the original operation to the parent S2 when
> > it's hwpt_nested?
>
> Likely. I recall that was what one of our WIP versions did.
>
> > The name iommufd_hwpt_paging_attach() is a bit misleading. The
> > actual work there is all about reservations. It doesn't change any
> > tracking structure about attachment between device and hwpt.
>
> How about iommufd_hwpt_enforce/remove_rr() taking hwpt v.s.
> hwpt_paging.
> > With that I think continuing this per-device reservation scheme is
> > easier than adding specific reservation for SW_MSI at hwpt creation
> > time and then further requiring check at attach time to verify
> > the attached device is allocated with the same address as the one
> > during allocation.
>
> Jason, do you agree?
I came up with something plus a bit of naming alignment:
iommufd_device_attach_reserved_iova()
iommufd_group_remove_reserved_iova()
iommufd_group_do_replace_reserved_iova()
If it looks good to both of you, I will send a formal patch.
Thanks
Nic
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 76b7297d22b0f..f03218dc4861e 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -338,8 +338,9 @@ static int iommufd_group_setup_msi(struct iommufd_group *igroup,
return 0;
}
-static int iommufd_hwpt_paging_attach(struct iommufd_hwpt_paging *hwpt_paging,
- struct iommufd_device *idev)
+static int
+iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
+ struct iommufd_hwpt_paging *hwpt_paging)
{
int rc;
@@ -374,11 +375,9 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
goto err_unlock;
}
- if (hwpt_is_paging(hwpt)) {
- rc = iommufd_hwpt_paging_attach(to_hwpt_paging(hwpt), idev);
- if (rc)
- goto err_unlock;
- }
+ rc = iommufd_device_attach_reserved_iova(idev, to_hwpt_paging(hwpt));
+ if (rc)
+ goto err_unlock;
/*
* Only attach to the group once for the first device that is in the
@@ -398,9 +397,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
mutex_unlock(&idev->igroup->lock);
return 0;
err_unresv:
- if (hwpt_is_paging(hwpt))
- iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt,
- idev->dev);
+ iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt, idev->dev);
err_unlock:
mutex_unlock(&idev->igroup->lock);
return rc;
@@ -417,9 +414,7 @@ iommufd_hw_pagetable_detach(struct iommufd_device *idev)
iommufd_hwpt_detach_device(hwpt, idev);
idev->igroup->hwpt = NULL;
}
- if (hwpt_is_paging(hwpt))
- iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt,
- idev->dev);
+ iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt, idev->dev);
mutex_unlock(&idev->igroup->lock);
/* Caller must destroy hwpt */
@@ -451,8 +446,8 @@ iommufd_group_remove_reserved_iova(struct iommufd_group *igroup,
}
static int
-iommufd_group_do_replace_paging(struct iommufd_group *igroup,
- struct iommufd_hwpt_paging *hwpt_paging)
+iommufd_group_do_replace_reserved_iova(struct iommufd_group *igroup,
+ struct iommufd_hwpt_paging *hwpt_paging)
{
struct iommufd_hw_pagetable *old_hwpt = igroup->hwpt;
struct iommufd_device *cur;
@@ -460,8 +455,7 @@ iommufd_group_do_replace_paging(struct iommufd_group *igroup,
lockdep_assert_held(&igroup->lock);
- if (!hwpt_is_paging(old_hwpt) ||
- hwpt_paging->ioas != to_hwpt_paging(old_hwpt)->ioas) {
+ if (hwpt_paging->ioas != to_hwpt_paging(old_hwpt)->ioas) {
list_for_each_entry(cur, &igroup->device_list, group_item) {
rc = iopt_table_enforce_dev_resv_regions(
&hwpt_paging->ioas->iopt, cur->dev, NULL);
@@ -502,20 +496,15 @@ iommufd_device_do_replace(struct iommufd_device *idev,
}
old_hwpt = igroup->hwpt;
- if (hwpt_is_paging(hwpt)) {
- rc = iommufd_group_do_replace_paging(igroup,
- to_hwpt_paging(hwpt));
- if (rc)
- goto err_unlock;
- }
+ rc = iommufd_group_do_replace_reserved_iova(igroup, to_hwpt_paging(hwpt));
+ if (rc)
+ goto err_unlock;
rc = iommufd_hwpt_replace_device(idev, hwpt, old_hwpt);
if (rc)
goto err_unresv;
- if (hwpt_is_paging(old_hwpt) &&
- (!hwpt_is_paging(hwpt) ||
- to_hwpt_paging(hwpt)->ioas != to_hwpt_paging(old_hwpt)->ioas))
+ if (to_hwpt_paging(hwpt)->ioas != to_hwpt_paging(old_hwpt)->ioas)
iommufd_group_remove_reserved_iova(igroup,
to_hwpt_paging(old_hwpt));
@@ -535,9 +524,7 @@ iommufd_device_do_replace(struct iommufd_device *idev,
/* Caller must destroy old_hwpt */
return old_hwpt;
err_unresv:
- if (hwpt_is_paging(hwpt))
- iommufd_group_remove_reserved_iova(igroup,
- to_hwpt_paging(hwpt));
+ iommufd_group_remove_reserved_iova(igroup, to_hwpt_paging(hwpt));
err_unlock:
mutex_unlock(&idev->igroup->lock);
return ERR_PTR(rc);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 04109572a53ca..618524e1ce9c5 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -316,7 +316,9 @@ static inline bool hwpt_is_paging(struct iommufd_hw_pagetable *hwpt)
static inline struct iommufd_hwpt_paging *
to_hwpt_paging(struct iommufd_hw_pagetable *hwpt)
{
- return container_of(hwpt, struct iommufd_hwpt_paging, common);
+ if (hwpt_is_paging(hwpt))
+ return container_of(hwpt, struct iommufd_hwpt_paging, common);
+ return container_of(hwpt, struct iommufd_hwpt_nested, common)->parent;
}
static inline struct iommufd_hwpt_paging *
Powered by blists - more mailing lists