[RFC] xen/arm: fix arm_iommu_map_page after f9f6b22ab

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

Hi all,

Up until f9f6b22ab "xen/arm: Map ITS doorbell register to IOMMU page
tables" the only caller of iommu_map on ARM was grant_table.c which has
a specific usage model and restrictions as described by the in-code
comment in arm_iommu_map_page.

f9f6b22ab introduced a second caller to iommu_map on ARM:
vgic_v3_its_init_virtual. This specific statement in the
f9f6b22ab commit message is wrong:

"Note that the 1:1 check in arm_iommu_map_page remains for now, as
virtual ITSes are currently only created for hwdom where the doorbell
mapping is always 1:1."

Leading to crashes any time the hardware domain is not direct-mapped
(e.g. cache coloring and non-Dom0 hardware domain):

(XEN) Loading d1 initrd from 0000000045800000 to 
0x0000000048200000-0x000000004bdcd615
(XEN) Loading d1 DTB to 0x0000000048000000-0x00000000480005c4
(XEN) Xen BUG at drivers/passthrough/arm/iommu_helpers.c:49
(XEN) ----[ Xen-4.21-unstable  arm64  debug=y  Tainted: I      ]----
(XEN) CPU:    0
(XEN) PC:     00000a000024c758 arm_iommu_map_page+0x80/0x90
(XEN) LR:     00000a000024c750
(XEN) SP:     00000a000032fb70
(XEN) CPSR:   0000000080000249 MODE:64-bit EL2h (Hypervisor, handler)
(XEN)      X0: 0000000000000008  X1: 0000000000000000  X2: 0000000000000000
(XEN)      X3: 0000000000000001  X4: 00000a00002c98e0  X5: 0000000000000000
(XEN)      X6: 00008000ffffa710  X7: 00008000ffffa710  X8: 7f7f7f7f7f7f7f7f
(XEN)      X9: 0000000000000080 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101
(XEN)     X12: 0000000000000008 X13: 0000000000000020 X14: 0000000000000000
(XEN)     X15: ffffffffffffffff X16: 00000a000033cce8 X17: 0000000000000000
(XEN)     X18: 000000013e688d90 X19: 0000000000008090 X20: 0000000000000080
(XEN)     X21: 00008000f3a01000 X22: 0000000000008090 X23: 0000000000000080
(XEN)     X24: 0000000000008090 X25: 0000000000008090 X26: 0000000000000001
(XEN)     X27: 0000000000008090 X28: 0000000000000000  FP: 00000a000032fb70
(XEN)
(XEN)   VTCR_EL2: 0000000080043594
(XEN)  VTTBR_EL2: 000000013fdcb000
(XEN)
(XEN)  SCTLR_EL2: 0000000030cd183d
(XEN)    HCR_EL2: 0000000000000039
(XEN)  TTBR0_EL2: 000000004dd50000
(XEN)
(XEN)    ESR_EL2: 00000000f2000001
(XEN)  HPFAR_EL2: 0000000000000000
(XEN)    FAR_EL2: 0000000000000000
(XEN)
(XEN) Xen stack trace from sp=00000a000032fb70:
(XEN)    00000a000032fba0 00000a0000250884 0000000000000000 0000000000000000
(XEN)    00008000ffffa410 00008000f3a01000 00000a000032fc30 00000a0000288024
(XEN)    00008000f3a07010 00008000f3a01000 00008000ffffa410 0000000008080000
(XEN)    0000000008090040 00008000f3a01570 0000000000000010 0000000000000010
(XEN)    0000000000010040 3800000000000400 0000000000000010 00000a000032fc9c
(XEN)    0000000000008090 00008000f3a01150 00000a0000339388 00000a000033df80
(XEN)    00000a000032fca0 00000a0000285228 00008000f3a01000 0000000000000001
(XEN)    0000000000003800 0000000000000008 0000000000000000 0000000000000000
(XEN)    0000000000000001 0000000000000003 000000013e6c9950 000000013e6c9920
(XEN)    00008000f3a01000 0000000000000008 00000a000032fce0 00000a0000281dcc
(XEN)    00008000f3a01000 0000000000000100 00000a000032fce0 00000a0000281d40
(XEN)    0000000000000000 00000000000000ff 00000a000032fd10 00000a00002705a4
(XEN)    0000000000000000 00008000f3a01000 00000a000032fe08 00000a0000270578
(XEN)    00000a000032fd50 00000a00002055e8 00008000f3a01000 00000a000032fe08
(XEN)    0000000000000000 00000a00002053a8 00008000f3a01000 000000030032fda0
(XEN)    00000a000032fda0 00000a00002cf090 00008000fffed4c8 0000000000000003
(XEN)    0000000000000008 00000a00002f1bb8 0000000000000004 0000000000000048
(XEN)    19170c1f5f0cee0e 0000000000000002 00000a000032fe60 00000a00002eb588
(XEN)    0000000000000004 00000a00002bc000 00000a00002bd900 00000a00002b7c28
(XEN)    0000000000000004 0000000000000000 0000000000000000 0000000000000000
(XEN)    00000a000032fe20 00000000ffffffc8 000000030032fe60 0000000000000000
(XEN)    0000000000000000 0000012300000000 0000000100000000 00000040ffffffff
(XEN)    00000001ffffffff 0000000000000000 0000000200000000 0000000000000100
(XEN)    0000000000000000 00008000ffffa710 000000013e588620 00000a000020018c
(XEN)    000000004dc00000 fffff6004da00000 000000004de00000 0000000000001710
(XEN)    000000013e5887a0 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000 0000000000000000 0000000000000000
(XEN)    0000000000000000 0000000000000000
(XEN) Xen call trace:
(XEN)    [<00000a000024c758>] arm_iommu_map_page+0x80/0x90 (PC)
(XEN)    [<00000a000024c750>] arm_iommu_map_page+0x78/0x90 (LR)
(XEN)    [<00000a0000250884>] iommu_map+0xcc/0x29c
(XEN)    [<00000a0000288024>] vgic_v3_its_init_domain+0x18c/0x1e8
(XEN)    [<00000a0000285228>] vgic-v3.c#vgic_v3_domain_init+0x168/0x21c
(XEN)    [<00000a0000281dcc>] domain_vgic_init+0x14c/0x210
(XEN)    [<00000a00002705a4>] arch_domain_create+0x150/0x1f0
(XEN)    [<00000a00002055e8>] domain_create+0x47c/0x6c0
(XEN)    [<00000a00002cf090>] create_domUs+0x7f8/0x8cc
(XEN)    [<00000a00002eb588>] start_xen+0x8f4/0x998
(XEN)    [<00000a000020018c>] head.o#primary_switched+0x4/0x10
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Xen BUG at drivers/passthrough/arm/iommu_helpers.c:49
(XEN) ****************************************
(XEN)
(XEN) Reboot in five seconds...

Credits go to Jason for spotting this problem.

It makes sense to generalize the arm_iommu_map_page function to be more
generic and unopinionated. The comment specific to the grant table can
live in grant-table.c instead.

Opinions?

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index cf131c43a1..5c841d4f17 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1274,8 +1274,14 @@ map_grant_ref(
         }
 
         /*
-         * We're not translated, so we know that dfns and mfns are
-         * the same things, so the IOMMU entry is always 1-to-1.
+         * Grant mappings can be used for DMA requests. The dev_bus_addr
+         * returned by the hypercall is the MFN (not the IPA). For
+         * device protected by an IOMMU, Xen needs to add a 1:1 mapping
+         * in the domain p2m to allow DMA request to work. This is only
+         * valid when the domain is directed mapped.
+         * 
+         * We're not translated, so we know that dfns and mfns are the
+         * same things, so the IOMMU entry is always 1-to-1.
          */
         if ( !(op->flags & GNTMAP_readonly) && node.cnt.wr == 1 )
             kind = IOMMUF_readable | IOMMUF_writable;
diff --git a/xen/drivers/passthrough/arm/iommu_helpers.c 
b/xen/drivers/passthrough/arm/iommu_helpers.c
index 5cb1987481..2585f2e526 100644
--- a/xen/drivers/passthrough/arm/iommu_helpers.c
+++ b/xen/drivers/passthrough/arm/iommu_helpers.c
@@ -36,17 +36,6 @@ int __must_check arm_iommu_map_page(struct domain *d, dfn_t 
dfn, mfn_t mfn,
 {
     p2m_type_t t;
 
-    /*
-     * Grant mappings can be used for DMA requests. The dev_bus_addr
-     * returned by the hypercall is the MFN (not the IPA). For device
-     * protected by an IOMMU, Xen needs to add a 1:1 mapping in the domain
-     * p2m to allow DMA request to work.
-     * This is only valid when the domain is directed mapped. Hence this
-     * function should only be used by gnttab code with gfn == mfn == dfn.
-     */
-    BUG_ON(!is_domain_direct_mapped(d));
-    BUG_ON(mfn_x(mfn) != dfn_x(dfn));
-
     /* We only support readable and writable flags */
     if ( !(flags & (IOMMUF_readable | IOMMUF_writable)) )
         return -EINVAL;
@@ -57,7 +46,7 @@ int __must_check arm_iommu_map_page(struct domain *d, dfn_t 
dfn, mfn_t mfn,
      * The function guest_physmap_add_entry replaces the current mapping
      * if there is already one...
      */
-    return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(dfn)),
+    return guest_physmap_add_entry(d, _gfn(dfn_x(dfn)), _mfn(dfn_x(mfn)),
                                    IOMMUF_order(flags), t);
 }