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"
> + */
> + 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
