Re: [RFC PATCH v4 08/10] iommu/riscv: Pre-enable GADE for second-stage domains

[fangyu . yu]

>Hi Fangyu,
>
>On 9/15/2026 11:28 AM, fangyu.yu@xxxxxxxxxxxxxxxxx wrote:
>> From: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>>
>> Pre-enable RISCV_IOMMU_DC_TC_GADE in the device context when
>> attaching a second-stage domain, if the IOMMU supports AMO_HWAD.
>>
>> Software pre-populates second-stage page tables with D set, so
>> enabling GADE by default does not change normal behavior. When
>> dirty tracking is enabled, iommufd clears the pre-set D bits and
>> GADE becomes necessary for hardware to update the dirty bit on
>> write access.
>>
>> This avoids toggling GADE dynamically and keeps device context
>> setup consistent with second-stage domain attachment.
>>
>> Signed-off-by: Fangyu Yu <fangyu.yu@xxxxxxxxxxxxxxxxx>
>> ---
>>   drivers/iommu/riscv/iommu.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index 16779877351b..f01fd4e2f840 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -1330,6 +1330,8 @@ static int riscv_iommu_attach_paging_domain(struct iommu_domain *iommu_domain,
>>   			FIELD_PREP(RISCV_IOMMU_DC_IOHGATP_MODE, pt_info.iohgatp_mode) |
>>   			FIELD_PREP(RISCV_IOMMU_DC_IOHGATP_GSCID, domain->gscid) |
>>   			FIELD_PREP(RISCV_IOMMU_DC_IOHGATP_PPN, pt_info.ppn);
>> +		if (iommu->caps & RISCV_IOMMU_CAPABILITIES_AMO_HWAD)
>> +			dc.tc |= RISCV_IOMMU_DC_TC_GADE;

Hi Shuai:

>
>I might be missing something, but I don't see how the GADE bit set
>here reaches the hardware IOMMU DC. In riscv_iommu_iodir_update()
>(in patch 04/10), it looks like only the V bit is taken:
>
>	tc = READ_ONCE(dc->tc);
>	tc |= new_dc->ta & RISCV_IOMMU_DC_TC_V;
>	...
>	WRITE_ONCE(dc->tc, tc);
>

You’re right — this is a real issue.

GADE was being set in the software-side dc, but it was not
propagated into the hardware DC in riscv_iommu_iodir_update().

I have already fixed this in my local tree, and I’ll include the
change in the next version of the patchset:
    tc &= ~RISCV_IOMMU_DC_TC_GADE;
    tc |= new_dc->tc & RISCV_IOMMU_DC_TC_GADE;
    tc |= new_dc->ta & RISCV_IOMMU_DC_TC_V;

>I saw that you mentioned you tested it on QEMU. As far as I know,
>mainline QEMU has not yet declared the AMO_HWAD capability. Could
>you clarify whether you used a modified QEMU for this test?
>

I used mainline QEMU, not a modified one.

In QEMU, I only tested the second-stage path with a
VM using device passthrough.

I did not test the dirty-log related part there,
so that path cannot be covered there.

That is why I noted in the TODO that I will build a
dedicated test case for dirty log handling later.

Thanks,
Fangyu

>Thanks,
>Shuai
>
>>   	} else {
>>   		if (!riscv_iommu_fsc_supported(iommu, pt_info.fsc_iosatp_mode))
>>   			return -ENODEV;