lkml.org 
[lkml]   [2020]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA
    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]

    \
     
     \ /
      Last update: 2020-07-06 22:59    [W:2.458 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site