Re: [RFC] iommu/vt-d: Group and domain relationship

[Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

Hi,

On 11/8/18 1:55 AM, James Sewart wrote:
> Hey,
> 
>> On 7 Nov 2018, at 02:10, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> Hi,
>>
>> On 11/6/18 6:40 PM, James Sewart wrote:
>>> Hey Lu,
>>> Would you be able to go into more detail about the issues with
>>> allowing IOMMU_DOMAIN_DMA to be allocated via domain_alloc?
>>
>> This door is closed because intel iommu driver does everything for
>> IOMMU_DOMAIN_DMA: allocating a domain and setup the context entries
>> for the domain.
> 
> As far as I can tell, attach_device in the intel driver will handle
> cleaning up any old domain context mapping and ensure the new domain is
> mapped with calls to dmar_remove_one_dev_info and domain_add_dev_info.

That's only for domains of IOMMU_DOMAIN_UNMANAGED, right?

> 
>>
>> Why do we want to open this door? Probably we want the generic iommu
>> layer to handle these things (it's called default domain).
> 
> I’d like to allocate a domain and attach it to multiple devices in a
> group/multiple groups so that they share address translation, but still
> allow drivers for devices in those groups to use the dma_map_ops api.

Just out of curiosity, why do you want to share a single domain across
multiple groups? By default, the groups and DMA domains are normally
1-1 mapped, right?

> 
>> So we can't just open the door but not cleanup the things right?
> 
> A user of domain_alloc and attach_device is responsible for detaching a
> domain if it is no longer needed and calling domain_free.

Currently DMA API calls get_valid_domain_for_dev() to retrieve a DMA
domain. If the domain has already been allocated, return directly.
Otherwise, allocate and initialize a new one for the device. Let's call
domains allocated by get_valid_domain_for_dev() as "A".

If we open the door and allow another component to manage the DMA
domains through domain iommu_domain_alloc/free(). Let's call domains
allocated through iommu_domain_alloc() as "B".

So how can we sync between A and B?

Need to go through the code to find out more.

Best regards,
Lu Baolu
> 
> Cheers,
> James.
> 
> 
>>
>> I haven't spent time on details. So I cc'ed Jacob for corrections.
>>
>> Best regards,
>> Lu Baolu
>>
>>> Cheers,
>>> James.
>>> On Fri, Nov 2, 2018 at 2:43 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 10/30/18 10:18 PM, James Sewart via iommu wrote:
>>>>> Hey,
>>>>>
>>>>> I’ve been investigating the relationship between iommu groups and domains
>>>>> on our systems and have a few question. Why does the intel iommu code not
>>>>> allow allocating IOMMU_DOMAIN_DMA? Returning NULL when given this domain
>>>>> type has the side effect that the default_domain for an iommu group is not
>>>>> set, which, when using for e.g. dma_map_ops.map_page, means a domain is
>>>>> allocated per device.
>>>>
>>>> Intel vt-d driver doesn't implement the default domain and allocates
>>>> domain only on demanded. There are lots of things to do before we allow
>>>> iommu API to allocate domains other than IOMMU_DOMAIN_UNMANAGED.
>>>>
>>>> Best regards,
>>>> Lu Baolu
>>>>
>>>>>
>>>>> This seems to be the opposite behaviour to the AMD iommu code which
>>>>> supports allocating an IOMMU_DOMAIN_DMA and will only look to the iommu
>>>>> group if a domain is not attached to the device rather than allocating a
>>>>> new one. On AMD every device in an iommu group will share the same domain.
>>>>>
>>>>> Appended is what I think a patch to implement domain_alloc for
>>>>> IOMMU_DOMAIN_DMA and also IOMMU_DOMAIN_IDENTITY would look like. Testing
>>>>> shows each device in a group will share a domain by default, it also
>>>>> allows allocating a new dma domain that can be successfully attached to a
>>>>> group with iommu_attach_group.
>>>>>
>>>>> Looking for comment on why the behaviour is how it is currently and if
>>>>> there are any issues with the solution I’ve been testing.
>>>>>
>>>>> Cheers,
>>>>> James.
>>>>>
>>>>>
>>>>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>>>>> index bff2abd6..3a58389f 100644
>>>>> --- a/drivers/iommu/intel-iommu.c
>>>>> +++ b/drivers/iommu/intel-iommu.c
>>>>> @@ -5170,10 +5170,15 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
>>>>>        struct dmar_domain *dmar_domain;
>>>>>        struct iommu_domain *domain;
>>>>>
>>>>> -     if (type != IOMMU_DOMAIN_UNMANAGED)
>>>>> +     if (type == IOMMU_DOMAIN_UNMANAGED)
>>>>> +             dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
>>>>> +     else if(type == IOMMU_DOMAIN_DMA)
>>>>> +             dmar_domain = alloc_domain(0);
>>>>> +     else if(type == IOMMU_DOMAIN_IDENTITY)
>>>>> +             dmar_domain = alloc_domain(DOMAIN_FLAG_STATIC_IDENTITY);
>>>>> +     else
>>>>>                return NULL;
>>>>>
>>>>> -     dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
>>>>>        if (!dmar_domain) {
>>>>>                pr_err("Can't allocate dmar_domain\n");
>>>>>                return NULL;
>>>>> @@ -5186,9 +5191,12 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
>>>>>        domain_update_iommu_cap(dmar_domain);
>>>>>
>>>>>        domain = &dmar_domain->domain;
>>>>> -     domain->geometry.aperture_start = 0;
>>>>> -     domain->geometry.aperture_end   = __DOMAIN_MAX_ADDR(dmar_domain->gaw);
>>>>> -     domain->geometry.force_aperture = true;
>>>>> +
>>>>> +     if (type == IOMMU_DOMAIN_UNMANAGED) {
>>>>> +             domain->geometry.aperture_start = 0;
>>>>> +             domain->geometry.aperture_end   = __DOMAIN_MAX_ADDR(dmar_domain->gaw);
>>>>> +             domain->geometry.force_aperture = true;
>>>>> +     }
>>>>>
>>>>>        return domain;
>>>>>    }
>>>>> _______________________________________________
>>>>> iommu mailing list
>>>>> iommu@lists.linux-foundation.org
>>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>>>>
> 
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu