RE: [PATCH v5 04/13] iommufd/hw_pagetable: Enforce invalidation op on vIOMMU-based hwpt_nested

2024-10-29 Thread Tian, Kevin
> From: Jason Gunthorpe 
> Sent: Wednesday, October 30, 2024 12:05 AM
> 
> On Tue, Oct 29, 2024 at 08:22:43AM +, Tian, Kevin wrote:
> > > From: Nicolin Chen 
> > > Sent: Saturday, October 26, 2024 7:51 AM
> > >
> > > @@ -302,7 +302,9 @@ iommufd_viommu_alloc_hwpt_nested(struct
> > > iommufd_viommu *viommu, u32 flags,
> > >   }
> > >   hwpt->domain->owner = viommu->iommu_dev->ops;
> > >
> > > - if (WARN_ON_ONCE(hwpt->domain->type !=
> > > IOMMU_DOMAIN_NESTED)) {
> > > + if (WARN_ON_ONCE(hwpt->domain->type !=
> > > IOMMU_DOMAIN_NESTED ||
> > > +  (!viommu->ops->cache_invalidate &&
> > > +   !hwpt->domain->ops->cache_invalidate_user))) {
> > >   rc = -EINVAL;
> > >   goto out_abort;
> > >   }
> >
> > According to patch5, cache invalidate in viommu only uses
> > viommu->ops->cache_invalidate. Is here intended to allow
> > nested hwpt created via viommu to still support the old
> > method?
> 
> I think that is reasonable?
> 

Yes, just want to confirm.

Reviewed-by: Kevin Tian 



Re: [PATCH v5 04/13] iommufd/hw_pagetable: Enforce invalidation op on vIOMMU-based hwpt_nested

2024-10-29 Thread Jason Gunthorpe
On Tue, Oct 29, 2024 at 08:22:43AM +, Tian, Kevin wrote:
> > From: Nicolin Chen 
> > Sent: Saturday, October 26, 2024 7:51 AM
> > 
> > @@ -302,7 +302,9 @@ iommufd_viommu_alloc_hwpt_nested(struct
> > iommufd_viommu *viommu, u32 flags,
> > }
> > hwpt->domain->owner = viommu->iommu_dev->ops;
> > 
> > -   if (WARN_ON_ONCE(hwpt->domain->type !=
> > IOMMU_DOMAIN_NESTED)) {
> > +   if (WARN_ON_ONCE(hwpt->domain->type !=
> > IOMMU_DOMAIN_NESTED ||
> > +(!viommu->ops->cache_invalidate &&
> > + !hwpt->domain->ops->cache_invalidate_user))) {
> > rc = -EINVAL;
> > goto out_abort;
> > }
> 
> According to patch5, cache invalidate in viommu only uses
> viommu->ops->cache_invalidate. Is here intended to allow
> nested hwpt created via viommu to still support the old
> method?

I think that is reasonable?

Jason



Re: [PATCH v5 04/13] iommufd/hw_pagetable: Enforce invalidation op on vIOMMU-based hwpt_nested

2024-10-29 Thread Jason Gunthorpe
On Fri, Oct 25, 2024 at 04:50:33PM -0700, Nicolin Chen wrote:
> A vIOMMU-based hwpt_nested requires a cache invalidation op too, either
> using the one in iommu_domain_ops or the one in viommu_ops. Enforce that
> upon the allocated hwpt_nested.
> 
> Signed-off-by: Nicolin Chen 
> ---
>  drivers/iommu/iommufd/hw_pagetable.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe 

Jason



RE: [PATCH v5 04/13] iommufd/hw_pagetable: Enforce invalidation op on vIOMMU-based hwpt_nested

2024-10-29 Thread Tian, Kevin
> From: Nicolin Chen 
> Sent: Saturday, October 26, 2024 7:51 AM
> 
> @@ -302,7 +302,9 @@ iommufd_viommu_alloc_hwpt_nested(struct
> iommufd_viommu *viommu, u32 flags,
>   }
>   hwpt->domain->owner = viommu->iommu_dev->ops;
> 
> - if (WARN_ON_ONCE(hwpt->domain->type !=
> IOMMU_DOMAIN_NESTED)) {
> + if (WARN_ON_ONCE(hwpt->domain->type !=
> IOMMU_DOMAIN_NESTED ||
> +  (!viommu->ops->cache_invalidate &&
> +   !hwpt->domain->ops->cache_invalidate_user))) {
>   rc = -EINVAL;
>   goto out_abort;
>   }

According to patch5, cache invalidate in viommu only uses
viommu->ops->cache_invalidate. Is here intended to allow
nested hwpt created via viommu to still support the old
method?