On Thu, 2026-02-12 at 08:33 +0000, Duan, Zhenzhong wrote:
>
>
>
> > -----Original Message-----
> > From: Clement Mathieu--Drif
> > <[[email protected]](mailto:[email protected])>
> > Subject: Re: [RFC PATCH 04/14] intel_iommu: Create the nested hwpt with
> > IOMMU_HWPT_ALLOC_PASID flag
> >
> >
> > On Wed, 2026-02-04 at 22:11 -0500, Zhenzhong Duan wrote:
> >
> > > When pasid is enabled, any hwpt attached to non-PASID or PASID should be
> > > IOMMU_HWPT_ALLOC_PASID flagged, or else attachment fails.
> > >
> > > Signed-off-by: Zhenzhong Duan
> >
> > <[[[email protected]](mailto:[email protected])](mailto:[[email protected]](mailto:[email protected]))>
> >
> > > ---
> > > hw/i386/intel_iommu_accel.c | 10 ++++++----
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/hw/i386/intel_iommu_accel.c b/hw/i386/intel_iommu_accel.c
> > > index 67d54849f2..d61cfec1e6 100644
> > > --- a/hw/i386/intel_iommu_accel.c
> > > +++ b/hw/i386/intel_iommu_accel.c
> > > @@ -69,11 +69,13 @@ VTDHostIOMMUDevice
> >
> > *vtd_find_hiod_iommufd(VTDAddressSpace *as)
> >
> > > return NULL;
> > > }
> > >
> > > -static bool vtd_create_fs_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> > > +static bool vtd_create_fs_hwpt(VTDHostIOMMUDevice *vtd_hiod,
> >
> >
> > Does this change break naming consistency?
>
>
> Yes, because we need to check vtd_hiod->iommu_state->pasid in
> vtd_create_fs_hwpt() but not in vtd_destroy_old_fs_hwpt().
>
> We can change vtd_destroy_old_fs_hwpt() to also pass vtd_hiod or add a new
> parameter 'iommu_state *s' to vtd_create_fs_hwpt().
First solution lgtm
>
>
>
> > Other functions use use idev for HostIOMMUDeviceIOMMUFD structures
>
>
> Do you see others except vtd_destroy_old_fs_hwpt()?
I don't think so
Thanks !
>
> Thanks
> Zhenzhong
>
>
>
> >
> > > VTDPASIDEntry *pe, uint32_t *fs_hwpt_id,
> > > Error **errp)
> > > {
> > > + HostIOMMUDeviceIOMMUFD *idev =
> >
> > HOST_IOMMU_DEVICE_IOMMUFD(vtd_hiod->hiod);
> >
> > > struct iommu_hwpt_vtd_s1 vtd = {};
> > > + uint32_t flags = vtd_hiod->iommu_state->pasid ?
> >
> > IOMMU_HWPT_ALLOC_PASID : 0;
> >
> > >
> > > vtd.flags = (VTD_SM_PASID_ENTRY_SRE(pe) ? IOMMU_VTD_S1_SRE : 0) |
> > > (VTD_SM_PASID_ENTRY_WPE(pe) ? IOMMU_VTD_S1_WPE : 0) |
> > > @@ -82,8 +84,8 @@ static bool
> >
> > vtd_create_fs_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> >
> > > vtd.pgtbl_addr = (uint64_t)vtd_pe_get_fspt_base(pe);
> > >
> > > return iommufd_backend_alloc_hwpt(idev->iommufd, idev->devid, idev-
> > > hwpt_id,
> > > - 0, IOMMU_HWPT_DATA_VTD_S1,
> > > sizeof(vtd),
> > > - &vtd, fs_hwpt_id, errp);
> > > + flags, IOMMU_HWPT_DATA_VTD_S1,
> > > + sizeof(vtd), &vtd, fs_hwpt_id,
> > > errp);
> > > }
> > >
> > > static void vtd_destroy_old_fs_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> > > @@ -116,7 +118,7 @@ static bool
> >
> > vtd_device_attach_iommufd(VTDHostIOMMUDevice *vtd_hiod,
> >
> > > }
> > >
> > > if (vtd_pe_pgtt_is_fst(pe)) {
> > > - if (!vtd_create_fs_hwpt(idev, pe, &hwpt_id, errp)) {
> > > + if (!vtd_create_fs_hwpt(vtd_hiod, pe, &hwpt_id, errp)) {
> > > return false;
> > > }
> > > }
> >
>