iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: Auger Eric <eric.auger@redhat.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>,
	Raj Ashok <ashok.raj@intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	iommu@lists.linux-foundation.org,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA
Date: Mon, 6 Jul 2020 14:05:17 -0700	[thread overview]
Message-ID: <20200706140517.3f6ca616@jacob-builder> (raw)
In-Reply-To: <f44409e8-7c68-9b26-cf87-b0887668e879@redhat.com>

On Thu, 2 Jul 2020 10:39:26 +0200
Auger Eric <eric.auger@redhat.com> wrote:

> Hi Jacob, Yi,
> On 7/1/20 5:33 PM, Jacob Pan wrote:
> > From: Liu Yi L <yi.l.liu@intel.com>
> > 
> > For guest SVA usage, in order to optimize for less VMEXIT, guest
> > request of IOTLB flush also includes device TLB.
> > 
> > On the host side, IOMMU driver performs IOTLB and implicit devTLB
> > invalidation. When PASID-selective granularity is requested by the
> > guest we need to derive the equivalent address range for devTLB
> > instead of using the address information in the UAPI data. The
> > reason for that is, unlike IOTLB flush, devTLB flush does not
> > support PASID-selective granularity. This is to say, we need to set
> > the following in the PASID based devTLB invalidation descriptor:
> > - entire 64 bit range in address ~(0x1 << 63)
> > - S bit = 1 (VT-d CH 6.5.2.6).
> > 
> > Without this fix, device TLB flush range is not set properly for
> > PASID selective granularity. This patch also merged devTLB flush
> > code for both implicit and explicit cases.
> > 
> > Fixes: 6ee1b77ba3ac ("iommu/vt-d: Add svm/sva invalidate function")
> > Acked-by: Lu Baolu <baolu.lu@linux.intel.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> >  drivers/iommu/intel/iommu.c | 28 ++++++++++++++++++----------
> >  1 file changed, 18 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/iommu/intel/iommu.c
> > b/drivers/iommu/intel/iommu.c index 96340da57075..6a0c62c7395c
> > 100644 --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -5408,7 +5408,7 @@ intel_iommu_sva_invalidate(struct
> > iommu_domain *domain, struct device *dev, sid = PCI_DEVID(bus,
> > devfn); 
> >  	/* Size is only valid in address selective invalidation */
> > -	if (inv_info->granularity != IOMMU_INV_GRANU_PASID)
> > +	if (inv_info->granularity == IOMMU_INV_GRANU_ADDR)
> >  		size =
> > to_vtd_size(inv_info->addr_info.granule_size,
> > inv_info->addr_info.nb_granules); 
> > @@ -5417,6 +5417,7 @@ intel_iommu_sva_invalidate(struct
> > iommu_domain *domain, struct device *dev, IOMMU_CACHE_INV_TYPE_NR) {
> >  		int granu = 0;
> >  		u64 pasid = 0;
> > +		u64 addr = 0;
> >  
> >  		granu = to_vtd_granularity(cache_type,
> > inv_info->granularity); if (granu == -EINVAL) {
> > @@ -5456,24 +5457,31 @@ intel_iommu_sva_invalidate(struct
> > iommu_domain *domain, struct device *dev, (granu ==
> > QI_GRAN_NONG_PASID) ? -1 : 1 << size, inv_info->addr_info.flags &
> > IOMMU_INV_ADDR_FLAGS_LEAF); 
> > +			if (!info->ats_enabled)
> > +				break;
> >  			/*
> >  			 * Always flush device IOTLB if ATS is
> > enabled. vIOMMU
> >  			 * in the guest may assume IOTLB flush is
> > inclusive,
> >  			 * which is more efficient.
> >  			 */
> > -			if (info->ats_enabled)
> > -				qi_flush_dev_iotlb_pasid(iommu,
> > sid,
> > -						info->pfsid, pasid,
> > -						info->ats_qdep,
> > -
> > inv_info->addr_info.addr,
> > -						size);
> > -			break;
> > +			fallthrough;
> >  		case IOMMU_CACHE_INV_TYPE_DEV_IOTLB:
> > +			/*
> > +			 * There is no PASID selective flush for
> > device TLB, so
> > +			 * the equivalent of that is we set the
> > size to be the
> > +			 * entire range of 64 bit. User only
> > provides PASID info
> > +			 * without address info. So we set addr to
> > 0.  
> The "PASID selective flush for device TLB" terminology above sounds a
> bit confusing to me. I would rather say Intel device TLB has no
> support for OMMU_INV_GRANU_PASID granularity but only supports
> IOMMU_INV_GRANU_ADDR. Indeed 6.5.2.6 title is "PASID-based-Device-TLB
> Invalidate Descriptor"
Sounds good, will rewrite comment accordingly.

> > +			 */
> > +			if (inv_info->granularity ==
> > IOMMU_INV_GRANU_PASID) {
> > +				size = 64 - VTD_PAGE_SHIFT;
> > +				addr = 0;  
> I have my answer for previous patch review question. In that case the
> addr is not formatted with the least significant 0 matching the
> size_order.
> 
> > +			} else if (inv_info->granularity ==
> > IOMMU_INV_GRANU_ADDR)
> > +				addr = inv_info->addr_info.addr;
> > +
> >  			if (info->ats_enabled)
> >  				qi_flush_dev_iotlb_pasid(iommu,
> > sid, info->pfsid, pasid,
> > -						info->ats_qdep,
> > -
> > inv_info->addr_info.addr,
> > +						info->ats_qdep,
> > addr, size);
> >  			else
> >  				pr_warn_ratelimited("Passdown
> > device IOTLB flush w/o ATS!\n"); 
> 
> Besides
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> 
> Thanks
> 
> Eric
> 

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

  reply	other threads:[~2020-07-06 20:59 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-01 15:33 [PATCH v3 0/7] iommu/vt-d: Misc tweaks and fixes " Jacob Pan
2020-07-01 15:33 ` [PATCH v3 1/7] iommu/vt-d: Enforce PASID devTLB field mask Jacob Pan
2020-07-02  7:16   ` Auger Eric
2020-07-01 15:33 ` [PATCH v3 2/7] iommu/vt-d: Remove global page support in devTLB flush Jacob Pan
2020-07-02  7:16   ` Auger Eric
2020-07-06 23:58     ` Jacob Pan
2020-07-01 15:33 ` [PATCH v3 3/7] iommu/vt-d: Fix PASID devTLB invalidation Jacob Pan
2020-07-02  0:50   ` Lu Baolu
2020-07-02  7:16   ` Auger Eric
2020-07-01 15:33 ` [PATCH v3 4/7] iommu/vt-d: Handle non-page aligned address Jacob Pan
2020-07-02  7:50   ` Auger Eric
2020-07-06 23:28     ` Jacob Pan
2020-07-01 15:33 ` [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA Jacob Pan
2020-07-02  8:39   ` Auger Eric
2020-07-06 21:05     ` Jacob Pan [this message]
2020-07-01 15:33 ` [PATCH v3 6/7] iommu/vt-d: Warn on out-of-range invalidation address Jacob Pan
2020-07-02  0:55   ` Lu Baolu
2020-07-02  8:47   ` Auger Eric
2020-07-02 13:43     ` Jacob Pan
2020-07-01 15:33 ` [PATCH v3 7/7] iommu/vt-d: Disable multiple GPASID-dev bind Jacob Pan
2020-07-02  8:50   ` Auger Eric

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=20200706140517.3f6ca616@jacob-builder \
    --to=jacob.jun.pan@linux.intel.com \
    --cc=ashok.raj@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=eric.auger@redhat.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA' \
    /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).