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.

Why do we want to open this door? Probably we want the generic iommu
layer to handle these things (it's called default domain).

So we can't just open the door but not cleanup the things right?

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

Reply via email to