Re: [PATCH 1/5] iommu: Enhance domain allocation code to take additional flags

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

On 2024/8/26 16:08, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Friday, August 23, 2024 10:48 AM
>>
>> On 8/22/24 8:43 PM, Jason Gunthorpe wrote:
>>> On Thu, Aug 22, 2024 at 09:50:57AM +0800, Baolu Lu wrote:
>>>> On 8/22/24 12:31 AM, Jason Gunthorpe wrote:
>>>>>>      I think instead of having separate function it may be better to
>>>>>>      enhance __iommu_domain_alloc() such that:
>>>>>>        - Keep below changes from this patch
>>>>>>          - iommu_domain_init()
>>>>>>          - iommu_get_dma_cookie call inside
>> iommu_setup_default_domain()
>>>>>>        - modify __iommu_domain_alloc() to additional param (flags)
>>>>>>        - iommu_paging_domain_alloc_flags() will call
>> __iommu_domain_alloc()
>>>>> My expectation was to basically remove iommu_domain_alloc() entirely
>>>>> once Lu's work is merged.
>>>>>
>>>>> Instead we'd have these direct APIs:
>>>>>      iommu_domain_alloc_paging_flags()
>>>> Is it possible to use different domain allocation APIs for kernel DMA
>>>> and user-space DMA? Right now, we differentiate between these two
>> types
>>>> of domains using IOMMU_DOMAIN_DMA and
>> IOMMU_DOMAIN_UNMANAGED.
>>> I really don't want to have such a distinction.
>>>
>>>> I'm thinking about this because the Intel iommu driver has a similar
>>>> need to AMD. They both recommend using different page table formats
>> for
>>>> IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_UNMANAGED, which is
> Where is such recommendation coming from?
> 
>> currently stopping
>>>> us from implementing domain_alloc_paging in the Intel iommu driver.
>>> Why? What exactly is the issue?
>>>
>>> It is inhernetly wrong to behave differently based on DMA API or VFIO.
>>> They are not different things.
>>>
>>> If you have different behaviors and different properies, like AMD's
>>> PASID, then they should be described and mapped to some kind of flag.
>>>
>>> Otherwise the driver should always allocate a paging domain that gives
>>> the highest performance.
>> It relates to Intel VT-d's nested translation. Intel VT-d has two types
>> of page table formats for DMA translation: first level and second level.
>> In nested translation, the first level page table is used for first-
>> stage translation, and the second level page table is used for second-
>> stage translation.
>>
>> The iommu driver for vIOMMU in the guest kernel must use the first level
>> page table format for kernel DMA. This page table will then be nested on
>> a second level page table in the VMM host kernel.
> If a 'legacy-only' vIOMMU is exposed the guest kernel will certainly
> use the 2nd stage page table.
> 
> nested is an optimization manageable by VMM. Not something that
> the kernel driver needs to restrict.
> 
>> Our current design uses the first level page table for both the host and
>> guest kernel for simplicity. This is why we use different page table
>> formats for IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_UNMANAGED.
> As you said it's the current 'design', not an arch limitation. 😊
> 
>> We considered determining the page table format based on whether the
>> iommu has caching mode capability. This would result in the first level
>> page table format being used for guest kernel DMA and the second level
>> page table format being used for host kernel DMA. However, this approach
>> creates an inconsistency between the host and guest kernels.
> Why would one care about such consistency between the host and the guest?
> 
> In the end the iommu driver needs to decide based on the requested hwpt
> type and available iommu cap to decide the format.
> 
> e.g. is there a problem with a simple policy below?
> 
> - Default use stage-1 for both DMA/UNMANAGED, if nesting_parent is
>    not specified and stage-1 is supported by hw
> - Otherwise use stage-2

First-stage has some limitations for an UNMANAGED domain. For example,

- No separate controls for the Access/Dirty page tracking;
- No Write-only permission support;
- No page-level control for forcing snoop.

Thanks,
baolu