Re: [PATCH v2 05/17] iommufd: Keep track of each device's reserved regions instead of groups

[Baolu Lu <baolu.lu@linux.intel.com>]

On 3/8/23 11:02 PM, Jason Gunthorpe wrote:
> On Wed, Mar 08, 2023 at 08:32:03PM +0800, Baolu Lu wrote:
>>> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
>>> index 2ff192777f27d3..22863759c3bfb0 100644
>>> --- a/drivers/iommu/iommufd/iommufd_private.h
>>> +++ b/drivers/iommu/iommufd/iommufd_private.h
>>> @@ -76,10 +76,9 @@ int iopt_table_add_domain(struct io_pagetable *iopt,
>>>    			  struct iommu_domain *domain);
>>>    void iopt_table_remove_domain(struct io_pagetable *iopt,
>>>    			      struct iommu_domain *domain);
>>> -int iopt_table_enforce_group_resv_regions(struct io_pagetable *iopt,
>>> -					  struct device *device,
>>> -					  struct iommu_group *group,
>>> -					  phys_addr_t *sw_msi_start);
>>> +int iopt_table_enforce_dev_resv_regions(struct io_pagetable *iopt,
>>> +					struct device *dev,
>>> +					phys_addr_t *sw_msi_start);
>>>    int iopt_set_allow_iova(struct io_pagetable *iopt,
>>>    			struct rb_root_cached *allowed_iova);
>>>    int iopt_reserve_iova(struct io_pagetable *iopt, unsigned long start,
>>
>> Perhaps a silly question. The reserved regions are enforced in IOVA when
>> a device is added to the igroup's device list. Should it be released
>> after the device is removed from the list?
> 
> Yes, it is, iommufd_hw_pagetable_detach() does it right after the
> list_del()
> 
>> This may not be appropriate because the devices may share some common
>> reserved regions, such as the IOMMU_RESV_MSI.
> 
> Common reserved regions are duplicated for every device just as they
> were duplicated for every group.
> 
> The duplicates are keyed with an owner that is equal to the 'struct
> device *' so we only remove the IOVA specific to the struct device.
> 
> The interval tree effectively unions the duplicated and overlapping
> IOVA regions during lookup.
> 
> It is done this way to avoid memory allocation on destruction
> paths. We never have to chop up merged IOVA regions or something to
> remove a device.

Ah! Yes. This has been considered. I should read further into
iopt_reserve_iova() and iopt_remove_reserved_iova(). Thanks for the
explanation.

Best regards,
baolu