[PATCH v3 9/9] iommu/arm-smmu-v3: Use S2FWB for NESTED domains

[Tian, Kevin]

> From: Jason Gunthorpe <jgg at nvidia.com>
> Sent: Friday, October 25, 2024 10:01 PM
> 
> On Thu, Oct 24, 2024 at 07:54:10AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg at nvidia.com>
> > > Sent: Thursday, October 10, 2024 12:23 AM
> > >
> > > Force Write Back (FWB) changes how the S2 IOPTE's MemAttr field
> > > works. When S2FWB is supported and enabled the IOPTE will force
> cachable
> > > access to IOMMU_CACHE memory when nesting with a S1 and deny
> cachable
> > > access otherwise.
> >
> > didn't get the last part "deny cacheable access otherwise"
> 
>  Force Write Back (FWB) changes how the S2 IOPTE's MemAttr field
>  works. When S2FWB is supported and enabled the IOPTE will force cachable
>  access to IOMMU_CACHE memory when nesting with a S1 and deny
> cachable
>  access when !IOMMU_CACHE.
> 
> ?

Good. I read the original words as if cacheable access is denied
when S2FWB is not supported or disabled. 

> 
> > > @@ -169,7 +169,8 @@ arm_smmu_domain_alloc_nesting(struct device
> *dev,
> > > u32 flags,
> > >  	 * Must support some way to prevent the VM from bypassing the
> > > cache
> > >  	 * because VFIO currently does not do any cache maintenance.
> > >  	 */
> > > -	if (!arm_smmu_master_canwbs(master))
> > > +	if (!arm_smmu_master_canwbs(master) &&
> > > +	    !(master->smmu->features & ARM_SMMU_FEAT_S2FWB))
> > >  		return ERR_PTR(-EOPNOTSUPP);
> >
> > Probably can clarify the difference between CANWBS and S2FWB here
> > by copying some words from the previous commit message. especially
> > about the part of PCI nosnoop.
> 
> 	/*
> 	 * Must support some way to prevent the VM from bypassing the
> cache
> 	 * because VFIO currently does not do any cache maintenance.
> canwbs
> 	 * indicates the device is fully coherent and no cache maintenance is
> 	 * ever required, even for PCI No-Snoop. S2FWB means the S1 can't
> make
> 	 * things non-coherent using the memattr, but No-Snoop behavior is
> not
> 	 * effected.
> 	 */
> 	if (!arm_smmu_master_canwbs(master) &&
> 	    !(master->smmu->features & ARM_SMMU_FEAT_S2FWB))
> 		return ERR_PTR(-EOPNOTSUPP);
> 
> ?
> 

Looks good!