On Thu, 2 Jul 2020 10:39:26 +0200
Auger Eric <eric.au...@redhat.com> wrote:
> Hi Jacob, Yi,
> On 7/1/20 5:33 PM, Jacob Pan wrote:
> > From: Liu Yi L <yi.l....@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...@linux.intel.com>
> > Signed-off-by: Liu Yi L <yi.l....@intel.com>
> > Signed-off-by: Jacob Pan <jacob.jun....@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.au...@redhat.com>
>
> Thanks
>
> Eric
>
[Jacob Pan]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
