Re: [PATCH v2 1/4] iommu/vt-d: Check before setting PGSNP bit in pasid table entry

[Lu Baolu]

On 2022/4/24 11:37, Tian, Kevin wrote:
> > > This should be rebased on top of Jason's enforce coherency series
> > > instead of blindly setting it. No matter whether it's legacy mode
> > > where we set SNP in PTE or scalable mode where we set PGSNP
> > > in PASID entry for entire page table, the trigger point should be
> > > same i.e. when someone calls enforce_cache_coherency().
> > With Jason's enforce coherency series merged, we even don't need to set
> > PGSNP bit of a pasid entry for second level translation. 2nd level
> > always supports SNP in PTEs, so set PGSNP in pasid table entry is
> > unnecessary.
> Yes, this sounds correct for 2nd-level.
>
> but setting PGSNP of 1st level translation is also relevant to that
> change when talking about enforcing coherency in the guest. In
> this case PASID_FLAG_PAGE_SNOOP should be set also after
> enforce_cache_coherency() is called.

Yes. Agreed.

> Currently it's always set for unmanaged domain in
> domain_setup_first_level():
>
> 	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
> 		flags |= PASID_FLAG_PAGE_SNOOP;
>
> Suppose we need a separate interface to update PGSNP after pasid
> entry is set up.

Currently enforcing coherency is only used in VFIO. In the VFIO use
case, it's safe to always set PGSNP when an UNMANAGED domain is attached
on the first level pasid translation. We could add support of updating
PGSNP after pasid entry setup once there's a real need.

Best regards,
baolu