Re: [PATCH 2/5] iommu/vt-d: Fix unaligned addresses for intel_flush_svm_range_dev()

From: Lu Baolu
Date: Thu Jan 07 2021 - 18:54:40 EST

Hi Will,

On 2021/1/6 9:09, Lu Baolu wrote:
Hi Will,

Happy New Year!

On 2021/1/6 3:03, Will Deacon wrote:
On Thu, Dec 31, 2020 at 08:53:20AM +0800, Lu Baolu wrote:
The VT-d hardware will ignore those Addr bits which have been masked by
the AM field in the PASID-based-IOTLB invalidation descriptor. As the
result, if the starting address in the descriptor is not aligned with
the address mask, some IOTLB caches might not invalidate. Hence people
will see below errors.

[ 1093.704661] dmar_fault: 29 callbacks suppressed
[ 1093.704664] DMAR: DRHD: handling fault status reg 3
[ 1093.712738] DMAR: [DMA Read] Request device [7a:02.0] PASID 2
                fault addr 7f81c968d000 [fault reason 113]
                SM: Present bit in first-level paging entry is clear

Fix this by using aligned address for PASID-based-IOTLB invalidation.

Fixes: 1c4f88b7f1f92 ("iommu/vt-d: Shared virtual address in scalable mode")
Reported-and-tested-by: Guo Kaijie <Kaijie.Guo@xxxxxxxxx>
Signed-off-by: Lu Baolu <>
  drivers/iommu/intel/svm.c | 22 ++++++++++++++++++++--
  1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 69566695d032..b16a4791acfb 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -118,8 +118,10 @@ void intel_svm_check(struct intel_iommu *iommu)
      iommu->flags |= VTD_FLAG_SVM_CAPABLE;
-static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_dev *sdev,
-                unsigned long address, unsigned long pages, int ih)
+static void __flush_svm_range_dev(struct intel_svm *svm,
+                  struct intel_svm_dev *sdev,
+                  unsigned long address,
+                  unsigned long pages, int ih)
      struct qi_desc desc;
@@ -170,6 +172,22 @@ static void intel_flush_svm_range_dev (struct intel_svm *svm, struct intel_svm_d
+static void intel_flush_svm_range_dev(struct intel_svm *svm,
+                      struct intel_svm_dev *sdev,
+                      unsigned long address,
+                      unsigned long pages, int ih)
+    unsigned long shift = ilog2(__roundup_pow_of_two(pages));
+    unsigned long align = (1ULL << (VTD_PAGE_SHIFT + shift));
+    unsigned long start = ALIGN_DOWN(address, align);
+    unsigned long end = ALIGN(address + (pages << VTD_PAGE_SHIFT), align);
+    while (start < end) {
+        __flush_svm_range_dev(svm, sdev, start, align >> VTD_PAGE_SHIFT, ih);
+        start += align;
+    }

Given that this only seems to be called from intel_invalidate_range(), which
has to compute 'pages' only to have it pulled apart again here, perhaps it
would be cleaner for intel_flush_svm_range() to take something like an
'order' argument instead?

What do you think?

We need to clean up here. It's duplicate with the qi_flush_piotlb()
helper. I have a patch under testing for this. I will post it for review

I'm sorry, above reply is a little vague.

I meant to say, let's take 'pages' as the argument. We are going to use
qi_flush_piotlb() here to avoid duplicate QI interactions. The
qi_flush_piotlb() helper also takes 'pages', so keep 'pages' here will
make things easier.

My cleanup patch is for v5.12. Can you please take this for v5.11?

Best regards,