Hi Baolu and all,
This is a series to address some of the issues we found in vSVA support.
Most of the patches deal with exception handling, we also removed some bits
that are not currently supported.
Many thanks to Kevin Tian's review.
Jacob & Yi
Changelog:
v3
- Use pr_err instead of WARN() for invalid user address range (6/7)
- Fix logic in PASID selective devTLB flush (3/7)
v2 Address reviews from Baolu
- Fixed addr field in devTLB flush (5/7)
- Assign address for single page devTLB invalidation (4/7)
- Coding style tweaks
*** SUBJECT HERE ***
*** BLURB HERE ***
Jacob Pan (4):
iommu/vt-d: Remove global page support in devTLB flush
iommu/vt-d: Fix PASID devTLB invalidation
iommu/vt-d: Warn on out-of-range invalidation address
iommu/vt-d: Disable multiple GPASID-dev bind
Liu Yi L (3):
iommu/vt-d: Enforce PASID devTLB field mask
iommu/vt-d: Handle non-page aligned address
iommu/vt-d: Fix devTLB flush for vSVA
drivers/iommu/intel/dmar.c | 24 +++++++++++++++++++-----
drivers/iommu/intel/iommu.c | 37 ++++++++++++++++++++++---------------
drivers/iommu/intel/pasid.c | 11 ++++++++++-
drivers/iommu/intel/svm.c | 22 +++++++++-------------
include/linux/intel-iommu.h | 5 ++---
5 files changed, 62 insertions(+), 37 deletions(-)
--
2.7.4
From: Liu Yi L <[email protected]>
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 <[email protected]>
Signed-off-by: Liu Yi L <[email protected]>
Signed-off-by: Jacob Pan <[email protected]>
---
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
Hi Jacob, Yi,
On 7/1/20 5:33 PM, Jacob Pan wrote:
> From: Liu Yi L <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Liu Yi L <[email protected]>
> Signed-off-by: Jacob Pan <[email protected]>
> ---
> 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 <[email protected]>
Thanks
Eric
On Thu, 2 Jul 2020 10:39:26 +0200
Auger Eric <[email protected]> wrote:
> Hi Jacob, Yi,
> On 7/1/20 5:33 PM, Jacob Pan wrote:
> > From: Liu Yi L <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Liu Yi L <[email protected]>
> > Signed-off-by: Jacob Pan <[email protected]>
> > ---
> > 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 <[email protected]>
>
> Thanks
>
> Eric
>
[Jacob Pan]