[PATCH v2 4/4] iommu/arm-smmu-v3: Enable HTTU for stage1 with io-pgtable mapping

[Ryan Roberts]

On 23/04/2024 18:32, Jason Gunthorpe wrote:
> On Tue, Apr 23, 2024 at 05:45:16PM +0100, Ryan Roberts wrote:
>> On 22/02/2024 09:49, Shameer Kolothum wrote:
>>> From: Kunkun Jiang <jiangkunkun at huawei.com>
>>>
>>> If io-pgtable quirk flag indicates support for hardware update of
>>> dirty state, enable HA/HD bits in the SMMU CD and also set the DBM
>>> bit in the page descriptor.
>>>
>>> And now report the dirty page tracking capability of SMMUv3.
>>>
>>> Co-developed-by: Keqian Zhu <zhukeqian1 at huawei.com>
>>> Signed-off-by: Keqian Zhu <zhukeqian1 at huawei.com>
>>> Signed-off-by: Kunkun Jiang <jiangkunkun at huawei.com>
>>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi at huawei.com>
>>
>> Section 3.13 of the spec states: "Where translation tables are shared between
>> CDs that contain the same ASID (within a translation regime), the CD HA and HD
>> fields must be identical."
>>
>> I don't think the way that smmu domains work, its possible to end up with a
>> single pgtable shared between multiple CDs? 
> 
> It is possible. iommufd can link a single hwpt -> iommu_domain ->
> smmu_domain to many different RIDs and to different PASIDs each with
> their own CD.

Ahh ok.

> 
>> So the driver should be able to guarantee this constraint is met?
> 
> It is expected to be done by this:
> 
>>> @@ -1271,6 +1271,12 @@ void arm_smmu_make_s1_cd(struct arm_smmu_cd *target,
>>>  		CTXDESC_CD_0_ASET |
>>>  		FIELD_PREP(CTXDESC_CD_0_ASID, smmu_domain->asid)
>>>  		);
>>> +
>>> +	/* To enable dirty flag update, set both Access flag and dirty state update */
>>> +	if (pgtbl_cfg->quirks & IO_PGTABLE_QUIRK_ARM_HD)
>>> +		target->data[0] |= cpu_to_le64(CTXDESC_CD_0_TCR_HA |
>>> +					       CTXDESC_CD_0_TCR_HD);
>>> +
> 
> This function is the only place that programs the ASID into a CD entry
> for the domain, and it always derives the HA/HD bits in the same way
> from some immutable value stored in the iommu_domain structure.

But then I don't understand how this code can make the guarrantee? Whether or
not dirty tracking is enabled seems to be a property of the domain, not the page
table. (See arm_smmu_domain_finalise()). So if multiple domains can share a page
table surely some of those domains could end up with dirty tracking on and
others with it off. So there would be multiple CDs, all pointing to the same
pgtable but with different HA/HD values?

Or perhaps I've misunderstood your comment and there is actually always a 1:1
relationship between domain and pgtable? But a 1:* relationship between domain
and CD?

> 
> Jason