KVM Archive on lore.kernel.org
 help / color / Atom feed
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Lu Baolu <baolu.lu@linux.intel.com>,
	Joerg Roedel <joro@8bytes.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Cornelia Huck <cohuck@redhat.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	"Jiang, Dave" <dave.jiang@intel.com>,
	"Liu, Yi L" <yi.l.liu@intel.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH v3 3/4] iommu: Add iommu_aux_get_domain_for_dev()
Date: Fri, 31 Jul 2020 00:26:06 +0000
Message-ID: <MWHPR11MB16452F5E657F26B1137B80C98C4E0@MWHPR11MB1645.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20200730141725.5f63b508@x1.home>

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, July 31, 2020 4:17 AM
> 
> On Wed, 29 Jul 2020 23:49:20 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > > From: Alex Williamson <alex.williamson@redhat.com>
> > > Sent: Thursday, July 30, 2020 4:25 AM
> > >
> > > On Tue, 14 Jul 2020 13:57:02 +0800
> > > Lu Baolu <baolu.lu@linux.intel.com> wrote:
> > >
> > > > The device driver needs an API to get its aux-domain. A typical usage
> > > > scenario is:
> > > >
> > > >         unsigned long pasid;
> > > >         struct iommu_domain *domain;
> > > >         struct device *dev = mdev_dev(mdev);
> > > >         struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
> > > >
> > > >         domain = iommu_aux_get_domain_for_dev(dev);
> > > >         if (!domain)
> > > >                 return -ENODEV;
> > > >
> > > >         pasid = iommu_aux_get_pasid(domain, iommu_device);
> > > >         if (pasid <= 0)
> > > >                 return -EINVAL;
> > > >
> > > >          /* Program the device context */
> > > >          ....
> > > >
> > > > This adds an API for such use case.
> > > >
> > > > Suggested-by: Alex Williamson <alex.williamson@redhat.com>
> > > > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> > > > ---
> > > >  drivers/iommu/iommu.c | 18 ++++++++++++++++++
> > > >  include/linux/iommu.h |  7 +++++++
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > > index cad5a19ebf22..434bf42b6b9b 100644
> > > > --- a/drivers/iommu/iommu.c
> > > > +++ b/drivers/iommu/iommu.c
> > > > @@ -2817,6 +2817,24 @@ void iommu_aux_detach_group(struct
> > > iommu_domain *domain,
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(iommu_aux_detach_group);
> > > >
> > > > +struct iommu_domain *iommu_aux_get_domain_for_dev(struct
> device
> > > *dev)
> > > > +{
> > > > +	struct iommu_domain *domain = NULL;
> > > > +	struct iommu_group *group;
> > > > +
> > > > +	group = iommu_group_get(dev);
> > > > +	if (!group)
> > > > +		return NULL;
> > > > +
> > > > +	if (group->aux_domain_attached)
> > > > +		domain = group->domain;
> > >
> > > Why wouldn't the aux domain flag be on the domain itself rather than
> > > the group?  Then if we wanted sanity checking in patch 1/ we'd only
> > > need to test the flag on the object we're provided.
> > >
> > > If we had such a flag, we could create an iommu_domain_is_aux()
> > > function and then simply use iommu_get_domain_for_dev() and test that
> > > it's an aux domain in the example use case.  It seems like that would
> >
> > IOMMU layer manages domains per parent device. Here given a
> 
> Is this the IOMMU layer or the VT-d driver?  I don't see any notion of
> managing domains relative to a parent in the IOMMU layer.  Please point
> to something more specific if I'm wrong here.

it's maintained in VT-d driver (include/linux/intel-iommu.h)

struct device_domain_info {
        struct list_head link;  /* link to domain siblings */
        struct list_head global; /* link to global list */
        struct list_head table; /* link to pasid table */
        struct list_head auxiliary_domains; /* auxiliary domains
                                             * attached to this device
                                             */
	...

> 
> > dev (of mdev), we need a way to find its associated domain under its
> > parent device. And we cannot simply use iommu_get_domain_for_dev
> > on the parent device of the mdev, as it will give us the primary domain
> > of parent device.
> 
> Not the parent device of the mdev, but the mdev_dev(mdev) device.
> Isn't that what this series is enabling, being able to return the
> domain from the group that contains the mdev_dev?  We shouldn't need to
> leave breadcrumbs on the group to know about the domain, the domain
> itself should be the source of knowledge, or provide a mechanism/ops to
> learn that knowledge.  Thanks,
> 
> Alex

It's the tradeoff between leaving breadcrumb in domain or in group. 
Today the domain has no knowledge of mdev. It just includes a list
of physical devices which are attached to the domain (either due to
the device is assigned in a whole or as the parent device of an assigned
mdev). Then we have two choices. One is to save the mdev_dev info
in device_domain_info and maintain a mapping between mdev_dev
and related aux domain. The other is to record the domain info directly
in group. Earlier we choose the latter one as it looks simpler. If you
prefer to the former one, we can think more and have a try.

Thanks
Kevin

  reply index

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14  5:56 [PATCH v3 0/4] iommu aux-domain APIs extensions Lu Baolu
2020-07-14  5:57 ` [PATCH v3 1/4] iommu: Check IOMMU_DEV_FEAT_AUX feature in aux api's Lu Baolu
2020-07-29 20:03   ` Alex Williamson
2020-07-30  1:46     ` Lu Baolu
2020-07-14  5:57 ` [PATCH v3 2/4] iommu: Add iommu_aux_at(de)tach_group() Lu Baolu
2020-07-14 16:39   ` Jacob Pan
2020-07-15  0:47     ` Lu Baolu
2020-07-15 16:01       ` Jacob Pan
2020-07-16  1:07         ` Lu Baolu
2020-07-29 20:03           ` Alex Williamson
2020-07-29 23:34             ` Tian, Kevin
2020-07-30 19:46               ` Alex Williamson
2020-07-31  5:47                 ` Lu Baolu
2020-07-31 18:05                   ` Alex Williamson
2020-08-03  1:57                     ` Lu Baolu
2020-07-14  5:57 ` [PATCH v3 3/4] iommu: Add iommu_aux_get_domain_for_dev() Lu Baolu
2020-07-29 20:25   ` Alex Williamson
2020-07-29 23:49     ` Tian, Kevin
2020-07-30 20:17       ` Alex Williamson
2020-07-31  0:26         ` Tian, Kevin [this message]
2020-07-31  2:17         ` Tian, Kevin
2020-07-31  6:30     ` Lu Baolu
2020-07-31 18:14       ` Alex Williamson
2020-08-03  2:15         ` Lu Baolu
2020-07-14  5:57 ` [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs Lu Baolu
2020-07-14  8:25   ` Christoph Hellwig
2020-07-14 16:29     ` Jacob Pan
2020-07-15  1:00       ` Lu Baolu
2020-07-15  1:23         ` Tian, Kevin
2020-07-29 20:32   ` Alex Williamson
2020-07-30  2:41     ` Lu Baolu
2020-07-30 21:17       ` Alex Williamson
2020-07-31  1:37         ` Lu Baolu
2020-07-30  9:36   ` Liu, Yi L
2020-07-31  1:39     ` Lu Baolu
2020-07-23 13:55 ` [PATCH v3 0/4] iommu aux-domain APIs extensions Lu Baolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MWHPR11MB16452F5E657F26B1137B80C98C4E0@MWHPR11MB1645.namprd11.prod.outlook.com \
    --to=kevin.tian@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=cohuck@redhat.com \
    --cc=dave.jiang@intel.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jean-philippe@linaro.org \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=yi.l.liu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git