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?


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