[PATCH v3 3/4] iommu/arm-smmu-v3: Add support for dirty tracking in domain alloc

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Tian, Kevin <kevin.tian at intel.com>
> Sent: Wednesday, May 22, 2024 8:16 AM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi at huawei.com>;
> iommu at lists.linux.dev; linux-arm-kernel at lists.infradead.org
> Cc: robin.murphy at arm.com; will at kernel.org; joro at 8bytes.org;
> jgg at nvidia.com; ryan.roberts at arm.com; nicolinc at nvidia.com;
> mshavit at google.com; eric.auger at redhat.com; joao.m.martins at oracle.com;
> jiangkunkun <jiangkunkun at huawei.com>; zhukeqian
> <zhukeqian1 at huawei.com>; Linuxarm <linuxarm at huawei.com>
> Subject: RE: [PATCH v3 3/4] iommu/arm-smmu-v3: Add support for dirty tracking
> in domain alloc
> 
> > From: Shameer Kolothum <shameerali.kolothum.thodi at huawei.com>
> > Sent: Tuesday, April 30, 2024 9:43 PM
> >
> > @@ -2422,31 +2425,32 @@ static int arm_smmu_domain_finalise(struct
> > arm_smmu_domain *smmu_domain,
> >  	if (!(smmu->features & ARM_SMMU_FEAT_TRANS_S2))
> >  		smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
> >
> > +	pgtbl_cfg = (struct io_pgtable_cfg) {
> > +		.pgsize_bitmap	= smmu->pgsize_bitmap,
> > +		.coherent_walk	= smmu->features &
> > ARM_SMMU_FEAT_COHERENCY,
> > +		.tlb		= &arm_smmu_flush_ops,
> > +		.iommu_dev	= smmu->dev,
> > +	};
> > +
> >  	switch (smmu_domain->stage) {
> >  	case ARM_SMMU_DOMAIN_S1:
> > -		ias = (smmu->features & ARM_SMMU_FEAT_VAX) ? 52 : 48;
> > -		ias = min_t(unsigned long, ias, VA_BITS);
> > -		oas = smmu->ias;
> > +		unsigned long ias = (smmu->features &
> > +				     ARM_SMMU_FEAT_VAX) ? 52 : 48;
> > +		pgtbl_cfg.ias = min_t(unsigned long, ias, VA_BITS);
> > +		pgtbl_cfg.oas = smmu->ias;
> > +		if (enable_dirty)
> > +			pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_ARM_HD;
> 
> why is dirty tracking considered as a quirk?

Yes, that is a bit unconventional. But this was discussed earlier and in SMMUv3 
driver the word "quirk" is considered in the broadest sense and there are
precedent for this in the driver already.

>From Robin:

"Indeed these features aren't decorative grooves on a piece of furniture, 
but in the case of io-pgtable we're merely using "quirk" in its broadest 
sense to imply something that differs from the baseline default 
behaviour - ARM_MTK_EXT, ARM_TTBR1 and ARM_OUTER_WBWA (or whatever it's 
called this week) are all just indicating extra hardware features 
entirely comparable to HTTU;..."

https://lore.kernel.org/linux-iommu/5ada4a8b-8852-f83c-040a-9ef5dac51de2@arm.com/

> 
> >  		fmt = ARM_64_LPAE_S1;
> >  		break;
> >  	case ARM_SMMU_DOMAIN_S2:
> > -		ias = smmu->ias;
> > -		oas = smmu->oas;
> > +		pgtbl_cfg.ias = smmu->ias;
> > +		pgtbl_cfg.oas = smmu->oas;
> >  		fmt = ARM_64_LPAE_S2;
> >  		break;
> 
> so dirty-tracking is not supported by s2? what about nesting?

It will be added later when we have the nested support.
 
> if this is desired then attempting to set dirty_tracking on a s2 domain
> should be rejected with an error.

Ok. I will add that.

Thanks,
Shameer