> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
> Sent: Tuesday, September 18, 2018 11:52 PM
>
> On 15/09/2018 03:36, Tian, Kevin wrote:
> >> 4) Userspace opens another mdev.
> >> -> iommu.c calls domain->ops->attach_dev(domain2, dev)
> >
> > another mdev in same VFIO container or different? I assume the
> > latter since you mentioned a new domain2.
>
> I was thinking a different VFIO container actually. I used domain2 to
> try to make the example clearer
>
> >> 1)? When the container is closed, VFIO calls
> >> iommu_detach_device(domain2, parent_dev)
> >> -> iommu.c calls default_domain->ops->attach_dev(default_domain,
> dev)
> >> Given that auxiliary domains are attached, the IOMMU driver could
> deduce
> >> that this actually means "detach an auxiliary domain". But which one?
> >
> > I didn't get this one. There is no need to stick to 1) behavior for
> > 4), i.e. below is expected:
> > domain2->ops->detach_dev(domain2, dev)
>
> But in order to get that, the IOMMU core needs to know that domain2 is
> auxiliary. Otherwise, detach_dev is never called when a default_domain
> is present for the parent dev.
>
> I guess one solution is to add an "auxiliary" attribute to iommu_domain,
> so __iommu_detach_group would do something like:
this doesn't work. same domain can be also attached to another physical
device as non-aux domain (e.g. passthrough) at the same time (vfio-pci
device and vfio-mdev device in same container), then default domain
tweak is required in that case. "aux" takes effect only per-device, not
per-domain.
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 7113fe398b70..2b3e9b91aee7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1786,10 +1786,11 @@ static void __iommu_detach_group(struct
> iommu_domain *domain,
> {
> int ret;
>
> - if (!group->default_domain) {
> + if (!group->default_domain || domain->auxiliary) {
> __iommu_group_for_each_dev(group, domain,
> iommu_group_do_detach_device);
> - group->domain = NULL;
> + if (!domain->auxiliary)
> + group->domain = NULL;
> return;
> }
>
> Not sure who would set this "auxiliary" attribute... Maybe the IOMMU
> driver, when attaching the domain to a device that has auxiliary mode
> enabled?
>
> > why cannot ARM implement a detach_dev for aux_domain too? My
> > feeling is that default domain twist is only for switch between 1/2/3
> > in concept.
>
> If the core actually calls it, we can implement detach_dev :) The
> problem is that the core never calls detach_dev when default_domain is
> present (affects any IOMMU driver that relies on default_domain,
> including AMD), and even in case 4) the default_domain is present for
> the parent device
Then can we change that core logic so detach_dev is invoked in all
cases? yes there will be some changes in vendor drivers, but I expect
this change trivial (especially considering the gain in IOMMU API
simplicity side as described below).
>
> >> So the proposed interface doesn't seem to work as is. If we want to use
> >> iommu_attach/detach_device for auxiliary domains, the existing
> behavior
> >> of iommu.c, and IOMMU drivers that rely on it, have to change. Any
> >> change I can think of right now seems more daunting than introducing a
> >> different path for auxiliary domains, like iommu_attach_aux_domain for
> >> example.
> >>
> >
> > introducing *aux* specific API will cause different VFIO code path to
> > handle RID-based and PASID-based mdev, since RID-based still needs
> > to use normal attach_domain that way.
>
> The PASID-based mdev still requires a special case to retrieve the
> allocated PASID and program it in the parent device, so VFIO will need
> to know the difference between the two
>
that retrieve/program is down by parent driver, instead of VFIO.
Thanks
Kevin
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
