Re: Re: [RFC PATCH 08/11] iommu/riscv: Add dirty tracking support for second-stage domains

[fangyu.yu@linux.alibaba.com]

>> @@ -1247,6 +1247,84 @@ static int riscv_iommu_attach_paging_domain(struct iommu_domain *iommu_domain,
>>  	return 0;
>>  }
>>  
>> +/*
>> + * Enable or disable hardware A/D bit updates (GADE) in the device context for
>> + * all devices attached to a second-stage domain. When dirty tracking is
>> + * enabled the IOMMU hardware will set the dirty bit in PTEs on write access,
>> + * making them visible to read_and_clear_dirty().
>> + */
>> +static int riscv_iommu_set_dirty_tracking(struct iommu_domain *iommu_domain,
>> +					  bool enable)
>> +{
>> +	struct riscv_iommu_domain *domain = iommu_domain_to_riscv(iommu_domain);
>> +	struct riscv_iommu_bond *bond;
>> +	struct riscv_iommu_device *iommu, *prev;
>> +	struct riscv_iommu_dc *dc;
>> +	struct iommu_fwspec *fwspec;
>> +	struct riscv_iommu_command cmd;
>> +	u64 tc;
>> +	int i;
>> +
>> +	rcu_read_lock();
>> +
>> +	list_for_each_entry_rcu(bond, &domain->bonds, list) {
>> +		iommu = dev_to_iommu(bond->dev);
>> +		fwspec = dev_iommu_fwspec_get(bond->dev);
>> +
>> +		for (i = 0; i < fwspec->num_ids; i++) {
>> +			dc = riscv_iommu_get_dc(iommu, fwspec->ids[i]);
>> +			tc = READ_ONCE(dc->tc);
>> +			if (!(tc & RISCV_IOMMU_DC_TC_V))
>> +				continue;
>> +
>> +			if (enable)
>> +				tc |= RISCV_IOMMU_DC_TC_GADE;
>> +			else
>> +				tc &= ~RISCV_IOMMU_DC_TC_GADE;
>> +			WRITE_ONCE(dc->tc, tc);
>
>I'm pretty sure you don't need to do this. Just preset GADE when ever
>a S2 domain is attached, rely on the pre-set D to avoid any HW cost
>and you are fine. No need to change it dynamically unless something is
>reall weird about riscv.
>

Thanks, that’s a good suggestion. I will follow that approach: preset GADE
on second-stage domain attach and rely on the core-managed D-bit behavior.

Fangyu

>Jason