Hey,
> On 8 Nov 2018, at 01:42, Lu Baolu <baolu...@linux.intel.com> wrote:
>
> Hi,
>
> On 11/8/18 1:55 AM, James Sewart wrote:
>> Hey,
>>> On 7 Nov 2018, at 02:10, Lu Baolu <baolu...@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?
attach_device has logic for cleaning up domains that are allocated by the
intel driver.
old_domain = find_domain(dev);
if (old_domain) {
rcu_read_lock();
dmar_remove_one_dev_info(old_domain, dev);
rcu_read_unlock();
if (!domain_type_is_vm_or_si(old_domain) &&
list_empty(&old_domain->devices))
domain_exit(old_domain);
}
This is checking the type of the old domain only, freeing it if is not
attached to any devices. Looking at this now, maybe the solution would be
to distinguish between internally allocated dma domains and dma domains
allocated via the external api, so we avoid freeing a domain a driver has
reference to.
>>>
>>> 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?
Currently we see each device in a group with their own domain.
find_or_alloc_domain looks at dma aliases to determine who shares domains
whereas pci_device_group in the generic iommu code determines groups using
a few other checks. We have observed internally that devices under a pcie
switch will be put in the same group but they do not share a domain within
that group.
Getting every device within a group to share a domain would get us 90% of
the way to what we want. But we have some configurations where there exist
devices put in other groups that we want to share translations with.
>
>>> 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?
I’m not sure we need to sync them. Domain A would be replaced entirely for
a certain device with domain B by attach_device. As we saw above domain A
would be freed if there are no more devices attached to it. Domain B would
be managed by the user of the iommu api. If a device is detached from
domain B, the find_or_alloc_domain logic will take over once more and
allocate a fresh domain.
Cheers,
James.
>
> 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...@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
