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

["Tian, Kevin" <kevin.tian@intel.com>]

> From: Lu Baolu
> Sent: Thursday, April 21, 2022 7:36 PM
> 
> The latest VT-d specification states that the PGSNP field in the pasid
> table entry should be treated as Reserved(0) for implementations not
> supporting Snoop Control (SC=0 in the Extended Capability Register).
> This adds a check before setting the field.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/pasid.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index f8d215d85695..5cb2daa2b8cb 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -625,8 +625,14 @@ int intel_pasid_setup_first_level(struct intel_iommu
> *iommu,
>  		}
>  	}
> 
> -	if (flags & PASID_FLAG_PAGE_SNOOP)
> -		pasid_set_pgsnp(pte);
> +	if (flags & PASID_FLAG_PAGE_SNOOP) {
> +		if (ecap_sc_support(iommu->ecap)) {
> +			pasid_set_pgsnp(pte);
> +		} else {
> +			pasid_clear_entry(pte);
> +			return -EINVAL;
> +		}
> +	}
> 
>  	pasid_set_domain_id(pte, did);
>  	pasid_set_address_width(pte, iommu->agaw);
> @@ -710,7 +716,8 @@ int intel_pasid_setup_second_level(struct
> intel_iommu *iommu,
>  	pasid_set_fault_enable(pte);
>  	pasid_set_page_snoop(pte, !!ecap_smpwc(iommu->ecap));
> 
> -	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
> +	if (ecap_sc_support(iommu->ecap) &&
> +	    domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
>  		pasid_set_pgsnp(pte);
> 

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().

Thanks
Kevin