All of lore.kernel.org
 help / color / mirror / Atom feed
From: Auger Eric <eric.auger@redhat.com>
To: "Tian, Kevin" <kevin.tian@intel.com>,
	Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Yi L <yi.l.liu@linux.intel.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	David Woodhouse <dwmw2@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Date: Fri, 17 Apr 2020 09:46:55 +0200	[thread overview]
Message-ID: <2025736d-e7f2-d746-e030-e609b2f465e2@redhat.com> (raw)
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D824106@SHSMSX104.ccr.corp.intel.com>

Hi Kevin,
On 4/17/20 4:45 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Thursday, April 16, 2020 6:43 PM
>>
> [...]
>>>>> +	if (svm) {
>>>>> +		/*
>>>>> +		 * If we found svm for the PASID, there must be at
>>>>> +		 * least one device bond, otherwise svm should be
>>>>> freed.
>>>>> +		 */
>>>>> +		if (WARN_ON(list_empty(&svm->devs))) {
>>>>> +			ret = -EINVAL;
>>>>> +			goto out;
>>>>> +		}
>>>>> +
>>>>> +		for_each_svm_dev(sdev, svm, dev) {
>>>>> +			/* In case of multiple sub-devices of the
>>>>> same pdev
>>>>> +			 * assigned, we should allow multiple bind
>>>>> calls with
>>>>> +			 * the same PASID and pdev.
>>>>> +			 */
>>>>> +			sdev->users++;
>>>> What if this is not an mdev device. Is it also allowed?
>>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
>>> example of normal use case. You can bind the same PCI device (PF or
>>> SRIOV VF) more than once to the same PASID. Just need to unbind also.
>>
>> I don't get the point of binding a non mdev device several times with
>> the same PASID. Do you intend to allow that at userspace level or
>> prevent this from happening in VFIO?
> 
> I feel it's better to prevent this from happening, otherwise VFIO also
> needs to track the bind count and do multiple unbinds at mm_exit.
> But it's not necessary to prevent it in VFIO. We can check here
> upon whether aux_domain is valid, and if not return -EBUSY.
Ah OK. So if we can detect the case here it is even better

Thanks

Eric
> 
>>
>> Besides, the comment is a bit misleading as it gives the impression it
>> is only true for mdev and there is no associated check.
> 
> Thanks
> Kevin
> 


WARNING: multiple messages have this Message-ID (diff)
From: Auger Eric <eric.auger@redhat.com>
To: "Tian, Kevin" <kevin.tian@intel.com>,
	Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Yi L <yi.l.liu@linux.intel.com>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.com>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Date: Fri, 17 Apr 2020 09:46:55 +0200	[thread overview]
Message-ID: <2025736d-e7f2-d746-e030-e609b2f465e2@redhat.com> (raw)
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D824106@SHSMSX104.ccr.corp.intel.com>

Hi Kevin,
On 4/17/20 4:45 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Thursday, April 16, 2020 6:43 PM
>>
> [...]
>>>>> +	if (svm) {
>>>>> +		/*
>>>>> +		 * If we found svm for the PASID, there must be at
>>>>> +		 * least one device bond, otherwise svm should be
>>>>> freed.
>>>>> +		 */
>>>>> +		if (WARN_ON(list_empty(&svm->devs))) {
>>>>> +			ret = -EINVAL;
>>>>> +			goto out;
>>>>> +		}
>>>>> +
>>>>> +		for_each_svm_dev(sdev, svm, dev) {
>>>>> +			/* In case of multiple sub-devices of the
>>>>> same pdev
>>>>> +			 * assigned, we should allow multiple bind
>>>>> calls with
>>>>> +			 * the same PASID and pdev.
>>>>> +			 */
>>>>> +			sdev->users++;
>>>> What if this is not an mdev device. Is it also allowed?
>>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
>>> example of normal use case. You can bind the same PCI device (PF or
>>> SRIOV VF) more than once to the same PASID. Just need to unbind also.
>>
>> I don't get the point of binding a non mdev device several times with
>> the same PASID. Do you intend to allow that at userspace level or
>> prevent this from happening in VFIO?
> 
> I feel it's better to prevent this from happening, otherwise VFIO also
> needs to track the bind count and do multiple unbinds at mm_exit.
> But it's not necessary to prevent it in VFIO. We can check here
> upon whether aux_domain is valid, and if not return -EBUSY.
Ah OK. So if we can detect the case here it is even better

Thanks

Eric
> 
>>
>> Besides, the comment is a bit misleading as it gives the impression it
>> is only true for mdev and there is no associated check.
> 
> Thanks
> Kevin
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  reply	other threads:[~2020-04-17  7:47 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-03 18:42 [PATCH v11 00/10] Nested Shared Virtual Address (SVA) VT-d support Jacob Pan
2020-04-03 18:42 ` Jacob Pan
2020-04-03 18:42 ` [PATCH v11 01/10] iommu/vt-d: Move domain helper to header Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-03 18:42 ` [PATCH v11 02/10] iommu/uapi: Define a mask for bind data Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-08 13:07   ` Joerg Roedel
2020-04-08 13:07     ` Joerg Roedel
2020-04-08 16:11     ` Jacob Pan
2020-04-08 16:11       ` Jacob Pan
2020-04-03 18:42 ` [PATCH v11 03/10] iommu/vt-d: Use a helper function to skip agaw for SL Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-05  6:37   ` Auger Eric
2020-04-05  6:37     ` Auger Eric
2020-04-03 18:42 ` [PATCH v11 04/10] iommu/vt-d: Add nested translation helper function Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-08 13:33   ` Auger Eric
2020-04-08 13:33     ` Auger Eric
2020-04-10 23:54   ` kbuild test robot
2020-04-03 18:42 ` [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-09  7:41   ` Auger Eric
2020-04-09  7:41     ` Auger Eric
2020-04-10 19:45     ` Jacob Pan
2020-04-10 19:45       ` Jacob Pan
2020-04-16 10:43       ` Auger Eric
2020-04-16 10:43         ` Auger Eric
2020-04-17  2:45         ` Tian, Kevin
2020-04-17  2:45           ` Tian, Kevin
2020-04-17  7:46           ` Auger Eric [this message]
2020-04-17  7:46             ` Auger Eric
2020-04-17 15:28             ` Jacob Pan
2020-04-17 15:28               ` Jacob Pan
2020-04-17 23:46               ` Tian, Kevin
2020-04-17 23:46                 ` Tian, Kevin
2020-04-27 17:28                 ` Jacob Pan
2020-04-27 17:28                   ` Jacob Pan
2020-04-10 21:06     ` Jacob Pan
2020-04-10 21:06       ` Jacob Pan
2020-04-16 10:46       ` Auger Eric
2020-04-16 10:46         ` Auger Eric
2020-04-03 18:42 ` [PATCH v11 06/10] iommu/vt-d: Support flushing more translation cache types Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-05  6:37   ` Auger Eric
2020-04-05  6:37     ` Auger Eric
2020-04-03 18:42 ` [PATCH v11 07/10] iommu/vt-d: Add svm/sva invalidate function Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-09  8:50   ` Auger Eric
2020-04-09  8:50     ` Auger Eric
2020-04-10 21:56     ` Jacob Pan
2020-04-10 21:56       ` Jacob Pan
2020-04-16 10:59       ` Auger Eric
2020-04-16 10:59         ` Auger Eric
2020-04-17 16:14         ` Jacob Pan
2020-04-17 16:14           ` Jacob Pan
2020-04-03 18:42 ` [PATCH v11 08/10] iommu/vt-d: Cache virtual command capability register Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-09 10:14   ` Auger Eric
2020-04-09 10:14     ` Auger Eric
2020-04-03 18:42 ` [PATCH v11 09/10] iommu/vt-d: Enlightened PASID allocation Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-03 18:42 ` [PATCH v11 10/10] iommu/vt-d: Add custom allocator for IOASID Jacob Pan
2020-04-03 18:42   ` Jacob Pan
2020-04-09 10:31   ` Auger Eric
2020-04-09 10:31     ` Auger Eric
2020-04-05  8:28 ` [PATCH v11 00/10] Nested Shared Virtual Address (SVA) VT-d support Lu Baolu
2020-04-05  8:28   ` 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=2025736d-e7f2-d746-e030-e609b2f465e2@redhat.com \
    --to=eric.auger@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jacob.jun.pan@linux.intel.com \
    --cc=jean-philippe@linaro.com \
    --cc=jic23@kernel.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=yi.l.liu@linux.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.