iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Lu Baolu <baolu.lu@linux.intel.com>
To: Robin Murphy <robin.murphy@arm.com>,
	Joerg Roedel <joro@8bytes.org>,
	Alex Williamson <alex.williamson@redhat.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	kvm@vger.kernel.org, Cornelia Huck <cohuck@redhat.com>,
	linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org
Subject: Re: [PATCH 1/2] iommu: Add iommu_group_get/set_domain()
Date: Tue, 30 Jun 2020 09:03:11 +0800	[thread overview]
Message-ID: <5dc1cece-6111-9b56-d04c-9553d592675b@linux.intel.com> (raw)
In-Reply-To: <acc0a8fd-bd23-fc34-aecc-67796ab216e7@arm.com>

Hi Robin,

On 6/29/20 7:56 PM, Robin Murphy wrote:
> On 2020-06-27 04:15, Lu Baolu wrote:
>> The hardware assistant vfio mediated device is a use case of iommu
>> aux-domain. The interactions between vfio/mdev and iommu during mdev
>> creation and passthr are:
>>
>> - Create a group for mdev with iommu_group_alloc();
>> - Add the device to the group with
>>          group = iommu_group_alloc();
>>          if (IS_ERR(group))
>>                  return PTR_ERR(group);
>>
>>          ret = iommu_group_add_device(group, &mdev->dev);
>>          if (!ret)
>>                  dev_info(&mdev->dev, "MDEV: group_id = %d\n",
>>                           iommu_group_id(group));
>> - Allocate an aux-domain
>>     iommu_domain_alloc()
>> - Attach the aux-domain to the physical device from which the mdev is
>>    created.
>>     iommu_aux_attach_device()
>>
>> In the whole process, an iommu group was allocated for the mdev and an
>> iommu domain was attached to the group, but the group->domain leaves
>> NULL. As the result, iommu_get_domain_for_dev() doesn't work anymore.
>>
>> This adds iommu_group_get/set_domain() so that group->domain could be
>> managed whenever a domain is attached or detached through the aux-domain
>> api's.
> 
> Letting external callers poke around directly in the internals of 
> iommu_group doesn't look right to me.

Unfortunately, it seems that the vifo iommu abstraction is deeply bound
to the IOMMU subsystem. We can easily find other examples:

iommu_group_get/set_iommudata()
iommu_group_get/set_name()
...

> 
> If a regular device is attached to one or more aux domains for PASID 
> use, iommu_get_domain_for_dev() is still going to return the primary 
> domain, so why should it be expected to behave differently for mediated

Unlike the normal device attach, we will encounter two devices when it
comes to aux-domain.

- Parent physical device - this might be, for example, a PCIe device
with PASID feature support, hence it is able to tag an unique PASID
for DMA transfers originated from its subset. The device driver hence
is able to wrapper this subset into an isolated:

- Mediated device - a fake device created by the device driver mentioned
above.

Yes. All you mentioned are right for the parent device. But for mediated
device, iommu_get_domain_for_dev() doesn't work even it has an valid
iommu_group and iommu_domain.

iommu_get_domain_for_dev() is a necessary interface for device drivers
which want to support aux-domain. For example,

           struct iommu_domain *domain;
           struct device *dev = mdev_dev(mdev);
	  unsigned long pasid;

           domain = iommu_get_domain_for_dev(dev);
           if (!domain)
                   return -ENODEV;

           pasid = iommu_aux_get_pasid(domain, dev->parent);
	  if (pasid == IOASID_INVALID)
		  return -EINVAL;

	  /* Program the device context with the PASID value */
	  ....

Without this fix, iommu_get_domain_for_dev() always returns NULL and the
device driver has no means to support aux-domain.

Best regards,
baolu

> devices? AFAICS it's perfectly legitimate to have no primary domain if 
> traffic-without-PASID is invalid.
> 
> Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2020-06-30  1:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-27  3:15 Lu Baolu
2020-06-27  3:15 ` [PATCH 2/2] vfio/type1: Update group->domain after aux attach and detach Lu Baolu
2020-06-29 11:56 ` [PATCH 1/2] iommu: Add iommu_group_get/set_domain() Robin Murphy
2020-06-30  1:03   ` Lu Baolu [this message]
2020-06-30 16:51     ` Robin Murphy
2020-07-01  7:32       ` Lu Baolu
2020-07-01 12:18         ` Robin Murphy
2020-07-02  1:32           ` Lu Baolu
2020-07-02  2:36           ` Lu Baolu
2020-07-07  1:26             ` 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=5dc1cece-6111-9b56-d04c-9553d592675b@linux.intel.com \
    --to=baolu.lu@linux.intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=cohuck@redhat.com \
    --cc=dave.jiang@intel.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robin.murphy@arm.com \
    --subject='Re: [PATCH 1/2] iommu: Add iommu_group_get/set_domain()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).