Re: [PATCH v4 3/8] iommu/vt-d: Duplicate iommu_resv_region objects per device list

[Auger Eric]

Hi Joerg,

On 5/27/19 5:23 PM, Joerg Roedel wrote:
> On Mon, May 27, 2019 at 10:55:36AM +0200, Eric Auger wrote:
>> -			list_add_tail(&rmrr->resv->list, head);
>> +			length = rmrr->end_address - rmrr->base_address + 1;
>> +			resv = iommu_alloc_resv_region(rmrr->base_address,
>> +						       length, prot,
>> +						       IOMMU_RESV_DIRECT,
>> +						       GFP_ATOMIC);
>> +			if (!resv)
>> +				break;
>> +
>> +			list_add_tail(&resv->list, head);
> 
> Okay, so this happens in a rcu_read_locked section and must be atomic,
> but I don't like this extra parameter to iommu_alloc_resv_region().
> 
> How about replacing the rcu-lock with the dmar_global_lock, which
> protects against changes to the global rmrr list? This will make this
> loop preemptible and taking the global lock is okay because this
> function is in no way performance relevant.

After studying in more details the for_each_active_dev_scope macro and
rcu_dereference_check it looks OK to me. I respinned accordingly.

Thanks

Eric
> 
> Regards,
> 
> 	Joerg
>