Re: [PATCH v6 09/10] iommu/riscv: Add dirty tracking support for second-stage domains

[fangyu.yu@linux.alibaba.com]

>> >@@ -1439,6 +1454,9 @@ static int riscv_iommu_init_domain_s2(struct device *dev,
>> > 	if (domain->gscid < 0)
>> > 		return -ENOMEM;
>> >
>> >+	if (iommu->caps & RISCV_IOMMU_CAPABILITIES_AMO_HWAD)
>> >+		domain->domain.dirty_ops = &riscv_iommu_dirty_ops;
>>
>> This sets dirty_ops based on whether the allocating device's IOMMU
>> supports AMO_HWAD. As I understand it, the domain/HWPT is not tied
>> to that IOMMU instance. A later attach may target a device behind
>> another instance without AMO_HWAD, and the attach can still succeed
>> with GADE disabled, potentially causing guest page faults.
>>
>> Shouldn't we therefore revalidate AMO_HWAD cap for domains with
>> dirty_ops and reject this combination?
>>
>> FYI, both AMD and Intel have attach-time dirty-tracking checks. See
>> drivers/iommu/intel/iommu.c:3195 and drivers/iommu/amd/iommu.c:3221.
>
>Right, need to check on every domain attach that the domain is
>compatible with the iommu instance.
>

Hi Shuai, Jason:

Thanks for pointing that out.

The domain is not tied to the allocating IOMMU instance, and
riscv_iommu_attach_paging_domain() only sets GADE when the target
instance advertises AMO_HWAD. As a result, attaching a dirty-tracking
domain to a device behind an instance without AMO_HWAD would silently
succeed. 
I'll fix this in the next version.

Thanks,
Fangyu

>Jason