Re: [RFC PATCH 2/5] iommu: Add an unmap_pages() op for IOMMU drivers

[isaacm@codeaurora.org]

On 2021-03-30 22:39, Lu Baolu wrote:
> On 3/31/21 1:36 PM, isaacm@codeaurora.org wrote:
>> On 2021-03-30 21:47, Lu Baolu wrote:
>>> On 3/31/21 11:00 AM, Isaac J. Manjarres wrote:
>>>> Add a callback for IOMMU drivers to provide a path for the
>>>> IOMMU framework to call into an IOMMU driver, which can call
>>>> into the io-pgtable code, to unmap a virtually contiguous
>>>> range of pages of the same size.
>>>> 
>>>> For IOMMU drivers that do not specify an unmap_pages() callback,
>>>> the existing logic of unmapping memory one page block at a time
>>>> will be used.
>>>> 
>>>> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
>>>> Suggested-by: Will Deacon <will@kernel.org>
>>>> ---
>>>>   include/linux/iommu.h | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>> 
>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>>> index 5e7fe519430a..9cf81242581a 100644
>>>> --- a/include/linux/iommu.h
>>>> +++ b/include/linux/iommu.h
>>>> @@ -193,6 +193,7 @@ struct iommu_iotlb_gather {
>>>>    * @detach_dev: detach device from an iommu domain
>>>>    * @map: map a physically contiguous memory region to an iommu 
>>>> domain
>>>>    * @unmap: unmap a physically contiguous memory region from an 
>>>> iommu domain
>>>> + * @unmap_pages: unmap a number of pages of the same size from an 
>>>> iommu domain
>>>>    * @flush_iotlb_all: Synchronously flush all hardware TLBs for 
>>>> this domain
>>>>    * @iotlb_sync_map: Sync mappings created recently using @map to 
>>>> the hardware
>>>>    * @iotlb_sync: Flush all queued ranges from the hardware TLBs and 
>>>> empty flush
>>>> @@ -245,6 +246,9 @@ struct iommu_ops {
>>>>              phys_addr_t paddr, size_t size, int prot, gfp_t gfp);
>>>>       size_t (*unmap)(struct iommu_domain *domain, unsigned long 
>>>> iova,
>>>>                size_t size, struct iommu_iotlb_gather 
>>>> *iotlb_gather);
>>>> +    size_t (*unmap_pages)(struct iommu_domain *domain, unsigned 
>>>> long iova,
>>>> +                  size_t pgsize, size_t pgcount,
>>>> +                  struct iommu_iotlb_gather *iotlb_gather);
>>> 
>>> Is it possible to add an equivalent map_pages() callback?
>> Yes, map_pages() can be implemented and can leverage a lot of the 
>> implementation
>> of unmap_pages(). The problem is that if you map several pages in one 
>> call, and then
>> encounter an error and have to rollback, you should do TLB 
>> maintenance, as iommu_map
>> does when it encounters an error. However, we can't call iommu_unmap 
>> from io-pgtable-arm
>> for example. We can call arm_lpae_unmap_pages() from the later 
>> patches, but that doesn't
>> solve the TLB maintenance issue. Do you have any thoughts on how to 
>> address this?
> 
> Call unmap_pages() with the same pages and size to roll back. Does it
> work?
> 
> Best regards,
> baolu
Hi Lu,

I've given map_pages() a shot. Here's the second version of the RFC 
series: 
https://lore.kernel.org/linux-iommu/20210402013452.4013-1-isaacm@codeaurora.org/T/#t.

Thanks,
Isaac
> 
>>> 
>>>>       void (*flush_iotlb_all)(struct iommu_domain *domain);
>>>>       void (*iotlb_sync_map)(struct iommu_domain *domain, unsigned 
>>>> long iova,
>>>>                      size_t size);
>>>> 
>>> 
>>> Best regards,
>>> baolu
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu