[RESEND,2/3,4.16?] VT-d: fix reduced page table levels support when sharing tables

Commit Message

Jan Beulich Nov. 12, 2021, 10:33 a.m. UTC

domain_pgd_maddr() contains logic to adjust the root address to be put
in the context entry in case 4-level page tables aren't supported by an
IOMMU. This logic may not be bypassed when sharing page tables.

Fixes: 25ccd093425c ("iommu: remove the share_p2m operation")
Signed-off-by: Jan Beulich <[e-mail]>
Reviewed-by: Kevin Tian <[e-mail]>

Comments

Ian Jackson Nov. 12, 2021, 12:50 p.m. UTC | #1

Jan Beulich writes ("[PATCH RESEND 0/3][4.16?] VT-d: misc (regression) fixes"):
> (re-sending upon Ian's request with his address adjusted, including
> Kevin's R-b at this occasion)
> 
> 1: per-domain IOMMU bitmap needs to have dynamic size
...
> As to patch 1: Without the earlier change, systems with more than 32
> IOMMUs simply would fail to enable use of the IOMMUs altogether. Now
> systems with more than 64 IOMMUs would observe memory corruption
> (with unclear knock-on effects). Whether systems with this many IOMMUs
> actually exist I can't tell; I know of ones with 40, which isn't all
> that far away from 64.

Right.  I have given my R-A provided we can commit it today.

Obviously potentail corruption, even on machines we don't know exist,
is an RC bug.  But if this patch can't go in very soon (or turns out
to be troublesome) I think we could downgrade this bug from
by detecting systems with many IOMMUs and refusing to boot ?
That's not great but if we don't know of actual affected systems, it
might be better than risking introducing bugs for everyone else.

> 2: fix reduced page table levels support when sharing tables
> 3: don't needlessly engage the untrusted-MSI workaround
> 
> As to 4.16 considerations: Only patch 1 addresses a regression
> introduced after 4.15, the others are older. Patch 3 additionally
> only addresses an inefficiency; the code we have is correct from
> a functional pov.

I don't understand the impact of patch 2 at all.

I doubt an inefficiency would warrant a release ack at this stage,
unless the benefit of the patch is very substantial.

Thanks,
Ian.

Patch

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -340,19 +340,21 @@  static uint64_t domain_pgd_maddr(struct
     {
         pagetable_t pgt = p2m_get_pagetable(p2m_get_hostp2m(d));
 
-        return pagetable_get_paddr(pgt);
+        pgd_maddr = pagetable_get_paddr(pgt);
     }
-
-    if ( !hd->arch.vtd.pgd_maddr )
+    else
     {
-        /* Ensure we have pagetables allocated down to leaf PTE. */
-        addr_to_dma_page_maddr(d, 0, 1);
-
         if ( !hd->arch.vtd.pgd_maddr )
-            return 0;
-    }
+        {
+            /* Ensure we have pagetables allocated down to leaf PTE. */
+            addr_to_dma_page_maddr(d, 0, 1);
 
-    pgd_maddr = hd->arch.vtd.pgd_maddr;
+            if ( !hd->arch.vtd.pgd_maddr )
+                return 0;
+        }
+
+        pgd_maddr = hd->arch.vtd.pgd_maddr;
+    }
 
     /* Skip top levels of page tables for 2- and 3-level DRHDs. */
     for ( agaw = level_to_agaw(4);