Re: [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA
On Thu, 2 Jul 2020 10:39:26 +0200
Auger Eric wrote:
> Hi Jacob, Yi,
> On 7/1/20 5:33 PM, Jacob Pan wrote:
> > From: Liu Yi L
> >
> > 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
> > Signed-off-by: Liu Yi L
> > Signed-off-by: Jacob Pan
> > ---
> > 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
Re: [PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA
Hi Jacob, Yi,
On 7/1/20 5:33 PM, Jacob Pan wrote:
> From: Liu Yi L
>
> 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
> Signed-off-by: Liu Yi L
> Signed-off-by: Jacob Pan
> ---
> 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
>
[PATCH v3 5/7] iommu/vt-d: Fix devTLB flush for vSVA
From: Liu Yi L
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
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
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.
+*/
+ if (inv_info->granularity == IOMMU_INV_GRANU_PASID) {
+ size = 64 - VTD_PAGE_SHIFT;
+ addr = 0;
+ } 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");
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
