[PATCH 0/6] arm: extended regions fixes

[PATCH 0/6] arm: extended regions fixes

[Stewart Hildebrand]

pipeline: https://gitlab.com/xen-project/people/stewarthildebrand/xen/-/pipelines/1799738957

Stewart Hildebrand (6):
  xen/arm: fix math in add_ext_regions
  xen/arm: fix math in add_hwdom_free_regions
  xen/arm: switch find_domU_holes to rangesets
  rangeset: introduce rangeset_subtract
  xen/arm: exclude xen,reg{-cacheable} from domU extended regions
  tools/arm: exclude iomem from domU extended regions

 tools/libs/light/libxl_arm.c            | 118 ++++++++++++++++++++----
 xen/arch/arm/dom0less-build.c           |  19 +++-
 xen/arch/arm/domain_build.c             |  63 ++++++++++---
 xen/arch/arm/include/asm/kernel.h       |   1 +
 xen/arch/arm/include/asm/static-shmem.h |   9 --
 xen/arch/arm/static-shmem.c             |  65 -------------
 xen/common/rangeset.c                   |  12 +++
 xen/include/xen/rangeset.h              |   3 +
 8 files changed, 182 insertions(+), 108 deletions(-)


base-commit: 78ce2be733b1e45e2e190c1765fe31da318d435f
-- 
2.49.0

[PATCH 1/6] xen/arm: fix math in add_ext_regions

[Stewart Hildebrand]

In commit f37a59813979, the arguments to add_ext_regions() were switched
from addresses to frame numbers. add_ext_regions() converts the frame
numbers back to addresses, but the end address (e) is rounded down to
page size alignment. The logic to calculate the size assumes e points to
the last address, not page, effectively leading to the region size being
erroneously calculated to be 2M smaller than the actual size of the
region.

Fix by adding 1 to the frame number before converting back to address.

Fixes: f37a59813979 ("xen/arm: domain_build: Track unallocated pages using the frame number")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 xen/arch/arm/domain_build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 270a6b97e42c..2f655bcc2237 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -864,7 +864,7 @@ int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
     struct membanks *ext_regions = data;
     paddr_t start, size;
     paddr_t s = pfn_to_paddr(s_gfn);
-    paddr_t e = pfn_to_paddr(e_gfn);
+    paddr_t e = pfn_to_paddr(e_gfn + 1) - 1;
 
     if ( ext_regions->nr_banks >= ext_regions->max_banks )
         return 0;
-- 
2.49.0

[PATCH 2/6] xen/arm: fix math in add_hwdom_free_regions

[Stewart Hildebrand]

Erroneous logic was duplicated from add_ext_regions() into
add_hwdom_free_regions(). Frame numbers are converted to addresses, but
the end address (e) is rounded down to page size alignment. The logic to
calculate the size assumes e points to the last address, not page,
effectively leading to the region size being erroneously calculated to
be 2M smaller than the actual size of the region.

Fix by adding 1 to the frame number before converting back to address.

Fixes: 02975cc38389 ("xen/arm: permit non direct-mapped Dom0 construction")
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 xen/arch/arm/domain_build.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 2f655bcc2237..a0f3c074337d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -905,7 +905,7 @@ static int __init add_hwdom_free_regions(unsigned long s_gfn,
     struct membanks *free_regions = data;
     paddr_t start, size;
     paddr_t s = pfn_to_paddr(s_gfn);
-    paddr_t e = pfn_to_paddr(e_gfn);
+    paddr_t e = pfn_to_paddr(e_gfn + 1) - 1;
     unsigned int i, j;
 
     if ( free_regions->nr_banks >= free_regions->max_banks )
-- 
2.49.0

[PATCH 3/6] xen/arm: switch find_domU_holes to rangesets

[Stewart Hildebrand]

remove_shm_holes_for_domU() is unnecessarily complex: it re-creates the
extended regions from scratch.

Move the rangeset into find_domU_holes() and create the extended regions
only once. This makes is simpler to further manipulate the rangeset for
removing other regions.

Remove now-unused remove_shm_holes_for_domU().

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 xen/arch/arm/domain_build.c             | 46 ++++++++++++-----
 xen/arch/arm/include/asm/static-shmem.h |  9 ----
 xen/arch/arm/static-shmem.c             | 65 -------------------------
 3 files changed, 35 insertions(+), 85 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a0f3c074337d..3dcdd7a8c46f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1256,34 +1256,58 @@ static int __init find_domU_holes(const struct kernel_info *kinfo,
                                   struct membanks *ext_regions)
 {
     unsigned int i;
-    uint64_t bankend;
     const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
     const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
     const struct membanks *kinfo_mem = kernel_info_get_mem_const(kinfo);
-    int res = -ENOENT;
+    struct rangeset *mem_holes;
+    int res;
+
+    mem_holes = rangeset_new(NULL, NULL, 0);
+    if ( !mem_holes )
+        return -ENOMEM;
 
     for ( i = 0; i < GUEST_RAM_BANKS; i++ )
     {
-        struct membank *ext_bank = &(ext_regions->bank[ext_regions->nr_banks]);
+        uint64_t bankend, start, size = 0;
 
-        ext_bank->start = ROUNDUP(bankbase[i] + kinfo_mem->bank[i].size, SZ_2M);
+        start = ROUNDUP(bankbase[i] + kinfo_mem->bank[i].size, SZ_2M);
 
         bankend = ~0ULL >> (64 - p2m_ipa_bits);
         bankend = min(bankend, bankbase[i] + banksize[i] - 1);
-        if ( bankend > ext_bank->start )
-            ext_bank->size = bankend - ext_bank->start + 1;
+
+        if ( bankend > start )
+            size = bankend - start + 1;
 
         /* 64MB is the minimum size of an extended region */
-        if ( ext_bank->size < MB(64) )
+        if ( size < MB(64) )
             continue;
-        ext_regions->nr_banks++;
-        res = 0;
+
+        res = rangeset_add_range(mem_holes, PFN_DOWN(start), PFN_DOWN(bankend));
+        if ( res )
+        {
+            printk(XENLOG_ERR "Failed to add: %#"PRIx64"->%#"PRIx64"\n",
+                   start, start + size - 1);
+            goto out;
+        }
     }
 
+    /* Remove static shared memory regions */
+    res = remove_shm_from_rangeset(kinfo, mem_holes);
     if ( res )
-        return res;
+        goto out;
+
+    res = rangeset_report_ranges(mem_holes, 0,
+                                 PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
+                                 add_ext_regions, ext_regions);
+    if ( res )
+        ext_regions->nr_banks = 0;
+    else if ( !ext_regions->nr_banks )
+        res = -ENOENT;
 
-    return remove_shm_holes_for_domU(kinfo, ext_regions);
+ out:
+    rangeset_destroy(mem_holes);
+
+    return res;
 }
 
 static int __init find_host_extended_regions(const struct kernel_info *kinfo,
diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
index 94eaa9d500f9..ad8267c6bfbe 100644
--- a/xen/arch/arm/include/asm/static-shmem.h
+++ b/xen/arch/arm/include/asm/static-shmem.h
@@ -28,9 +28,6 @@ void init_sharedmem_pages(void);
 int remove_shm_from_rangeset(const struct kernel_info *kinfo,
                              struct rangeset *rangeset);
 
-int remove_shm_holes_for_domU(const struct kernel_info *kinfo,
-                              struct membanks *ext_regions);
-
 int make_shm_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
                               int sizecells);
 
@@ -74,12 +71,6 @@ static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo,
     return 0;
 }
 
-static inline int remove_shm_holes_for_domU(const struct kernel_info *kinfo,
-                                            struct membanks *ext_regions)
-{
-    return 0;
-}
-
 static inline int make_shm_resv_memory_node(const struct kernel_info *kinfo,
                                             int addrcells, int sizecells)
 {
diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
index e8d4ca3ba3ff..06f32be097c8 100644
--- a/xen/arch/arm/static-shmem.c
+++ b/xen/arch/arm/static-shmem.c
@@ -820,71 +820,6 @@ int __init remove_shm_from_rangeset(const struct kernel_info *kinfo,
     return 0;
 }
 
-int __init remove_shm_holes_for_domU(const struct kernel_info *kinfo,
-                                     struct membanks *ext_regions)
-{
-    const struct membanks *shm_mem = kernel_info_get_shm_mem_const(kinfo);
-    struct rangeset *guest_holes;
-    unsigned int i;
-    paddr_t start;
-    paddr_t end;
-    int res;
-
-    /* No static shared memory region. */
-    if ( shm_mem->nr_banks == 0 )
-        return 0;
-
-    dt_dprintk("Remove static shared memory holes from extended regions of DomU\n");
-
-    guest_holes = rangeset_new(NULL, NULL, 0);
-    if ( !guest_holes )
-        return -ENOMEM;
-
-    /* Copy extended regions sets into the rangeset */
-    for ( i = 0; i < ext_regions->nr_banks; i++ )
-    {
-        start = ext_regions->bank[i].start;
-        end = start + ext_regions->bank[i].size;
-
-        res = rangeset_add_range(guest_holes, PFN_DOWN(start),
-                                 PFN_DOWN(end - 1));
-        if ( res )
-        {
-            printk(XENLOG_ERR
-                   "Failed to add: %#"PRIpaddr"->%#"PRIpaddr", error: %d\n",
-                   start, end, res);
-            goto out;
-        }
-    }
-
-    /* Remove static shared memory regions */
-    res = remove_shm_from_rangeset(kinfo, guest_holes);
-    if ( res )
-        goto out;
-
-    /*
-     * Take the interval of memory starting from the first extended region bank
-     * start address and ending to the end of the last extended region bank.
-     */
-    i = ext_regions->nr_banks - 1;
-    start = ext_regions->bank[0].start;
-    end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1;
-
-    /* Reset original extended regions to hold new value */
-    ext_regions->nr_banks = 0;
-    res = rangeset_report_ranges(guest_holes, PFN_DOWN(start), PFN_DOWN(end),
-                                 add_ext_regions, ext_regions);
-    if ( res )
-        ext_regions->nr_banks = 0;
-    else if ( !ext_regions->nr_banks )
-        res = -ENOENT;
-
- out:
-    rangeset_destroy(guest_holes);
-
-    return res;
-}
-
 void __init shm_mem_node_fill_reg_range(const struct kernel_info *kinfo,
                                         __be32 *reg, int *nr_cells,
                                         int addrcells, int sizecells)
-- 
2.49.0

[PATCH 4/6] rangeset: introduce rangeset_subtract

[Stewart Hildebrand]

Introduce rangeset_subtract() to remove regions in r2 from r1.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 xen/common/rangeset.c      | 12 ++++++++++++
 xen/include/xen/rangeset.h |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index e75871039087..b9e8912fb1c3 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -397,6 +397,18 @@ int rangeset_merge(struct rangeset *r1, struct rangeset *r2)
     return rangeset_report_ranges(r2, 0, ~0UL, merge, r1);
 }
 
+static int cf_check subtract(unsigned long s, unsigned long e, void *data)
+{
+    struct rangeset *r = data;
+
+    return rangeset_remove_range(r, s, e);
+}
+
+int rangeset_subtract(struct rangeset *r1, struct rangeset *r2)
+{
+    return rangeset_report_ranges(r2, 0, ~0UL, subtract, r1);
+}
+
 int rangeset_add_singleton(
     struct rangeset *r, unsigned long s)
 {
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index 96c918082501..817505badf6f 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -85,6 +85,9 @@ int rangeset_consume_ranges(struct rangeset *r,
 /* Merge rangeset r2 into rangeset r1. */
 int __must_check rangeset_merge(struct rangeset *r1, struct rangeset *r2);
 
+/* Subtract rangeset r2 from rangeset r1. */
+int __must_check rangeset_subtract(struct rangeset *r1, struct rangeset *r2);
+
 /* Add/remove/query a single number. */
 int __must_check rangeset_add_singleton(
     struct rangeset *r, unsigned long s);
-- 
2.49.0

[PATCH 5/6] xen/arm: exclude xen,reg{-cacheable} from domU extended regions

[Stewart Hildebrand]

When a device is passed through to a dom0less domU, the
xen,reg{-cacheable} ranges may overlap with the extended regions. Remove
xen,reg{-cacheable} from extended regions.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
Not sure if we need a Fixes: tag, but if we do:
Fixes: 2a2447757b3c ("xen/arm: implement domU extended regions")

I investigated an alternate approach of parsing the partial device tree
again to scan for xen,reg{-cacheable} properties, but it resulted in
quite a lot of code duplication. Adding a rangeset pointer to "struct
kernel_info" has a much smaller diffstat, and then we avoid the need to
parse the partial device tree a second time.

I discovered this issue when booting a dom0less domU with a device
passed through. Partial device tree excerpt:

    passthrough {
        ... <snip> ...

        axi_uart16550_0: serial@a0001000 {
            clocks = <&uart_fixed_clk>;
            compatible = "ns16550a";
            interrupt-parent = <&gic>;
            interrupts = <0 89 4>;
            reg = <0x0 0xa0001000 0x0 0x1000>;
            reg-shift = <2>;

            xen,reg = <0x0 0xa0001000 0x00 0x1000 0x0 0xa0001000>;
            xen,path = "/amba_pl@0/serial@a0000000";
            xen,force-assign-without-iommu;
        };
    };

The domU was assigned an extended region overlapping with MMIO of the
passed through device:

(XEN) d1: extended region 0: 0xa0000000->0x100000000
(XEN) d1: extended region 1: 0x200000000->0xf000000000

The domU panicked when attempting to initialize the device:

[    3.490068] a0001000.serial: ttyS0 at MMIO 0xa0001000 (irq = 15, base_baud = 6249375) is a 16550A
[    3.498843] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
[    3.498853] Mem abort info:
[    3.498855]   ESR = 0x0000000096000044
[    3.498859]   EC = 0x25: DABT (current EL), IL = 32 bits
[    3.498864]   SET = 0, FnV = 0
[    3.498867]   EA = 0, S1PTW = 0
[    3.498870]   FSC = 0x04: level 0 translation fault
[    3.498874] Data abort info:
[    3.498876]   ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
[    3.498879]   CM = 0, WnR = 1, TnD = 0, TagAccess = 0
[    3.498884]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    3.498888] [0000000000000010] user address but active_mm is swapper
[    3.498894] Internal error: Oops: 0000000096000044 [#1] SMP
[    3.498899] Modules linked in:
[    3.498908] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.10-stew #1
[    3.498917] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.498924] pc : mem_serial_out+0x18/0x20
[    3.498936] lr : serial8250_do_set_mctrl+0x6c/0xc0
[    3.498943] sp : ffff800081bab6d0
[    3.498946] x29: ffff800081bab6d0 x28: ffff8000815e0dc8 x27: ffff000001c29c60
[    3.498957] x26: 0000000000000000 x25: ffff00000347b900 x24: ffff000005504c00
[    3.498968] x23: ffff00000347b800 x22: 0000000000000000 x21: ffff800081b69d78
[    3.498978] x20: ffff800081b69d78 x19: 0000000000000000 x18: ffffffffffffffff
[    3.498989] x17: 3d20647561625f65 x16: 736162202c353120 x15: 3d20717269282030
[    3.498999] x14: 3030313030306178 x13: ffff800081a21ff0 x12: 00000000000007fe
[    3.499010] x11: 00000000000002aa x10: ffff800081a4dff0 x9 : ffff800081a21ff0
[    3.499020] x8 : 00000000fffff7ff x7 : ffff800081a4dff0 x6 : 0000000000000008
[    3.499030] x5 : 0000000000000000 x4 : ffff800080797584 x3 : 0000000000000002
[    3.499040] x2 : 0000000000000000 x1 : 0000000000000010 x0 : 0000000000000000
[    3.499050] Call trace:
[    3.499053]  mem_serial_out+0x18/0x20
[    3.499059]  serial8250_set_mctrl+0x34/0x40
[    3.499065]  serial_core_register_port+0x534/0x7dc
[    3.499075]  serial_ctrl_register_port+0x10/0x1c
[    3.499084]  uart_add_one_port+0x10/0x1c
[    3.499092]  serial8250_register_8250_port+0x308/0x4c0
[    3.499102]  of_platform_serial_probe+0x2c4/0x48c
[    3.499110]  platform_probe+0x68/0xdc
[    3.499120]  really_probe+0xbc/0x298
[    3.499128]  __driver_probe_device+0x78/0x12c
[    3.499135]  driver_probe_device+0xdc/0x160
[    3.499142]  __driver_attach+0x94/0x19c
[    3.499149]  bus_for_each_dev+0x74/0xd0
[    3.499155]  driver_attach+0x24/0x30
[    3.499162]  bus_add_driver+0xe4/0x208
[    3.499168]  driver_register+0x60/0x128
[    3.499176]  __platform_driver_register+0x24/0x30
[    3.499184]  of_platform_serial_driver_init+0x1c/0x28
[    3.499192]  do_one_initcall+0x6c/0x1b0
[    3.499199]  kernel_init_freeable+0x178/0x258
[    3.499209]  kernel_init+0x20/0x1d0
[    3.499218]  ret_from_fork+0x10/0x20
[    3.499228] Code: f9400800 1ac32021 8b21c001 d50332bf (39000022)
[    3.499233] ---[ end trace 0000000000000000 ]---
[    3.499237] note: swapper/0[1] exited with irqs disabled
[    3.499247] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    3.499251] SMP: stopping secondary CPUs
[    3.499284] Kernel Offset: disabled
[    3.499286] CPU features: 0x00,00000080,00200000,0200420b
[    3.499292] Memory Limit: none
[    3.792412] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
---
 xen/arch/arm/dom0less-build.c     | 19 ++++++++++++++++++-
 xen/arch/arm/domain_build.c       | 13 ++++++++++++-
 xen/arch/arm/include/asm/kernel.h |  1 +
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index a356fc94fc4d..23178a801818 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -274,6 +274,14 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
     int res;
     paddr_t mstart, size, gstart;
 
+    if ( !kinfo->xen_reg_assigned )
+    {
+        kinfo->xen_reg_assigned = rangeset_new(NULL, NULL, 0);
+
+        if ( !kinfo->xen_reg_assigned )
+            return -ENOMEM;
+    }
+
     /* xen,reg specifies where to map the MMIO region */
     cell = (const __be32 *)xen_reg->data;
     len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
@@ -315,6 +323,11 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
                    mstart, gstart);
             return -EFAULT;
         }
+
+        res = rangeset_add_range(kinfo->xen_reg_assigned, PFN_DOWN(gstart),
+                                 PFN_DOWN(gstart + size - 1));
+        if ( res )
+            return res;
     }
 
     /*
@@ -1006,7 +1019,11 @@ static int __init construct_domU(struct domain *d,
 
     domain_vcpu_affinity(d, node);
 
-    return alloc_xenstore_params(&kinfo);
+    rc = alloc_xenstore_params(&kinfo);
+
+    rangeset_destroy(kinfo.xen_reg_assigned);
+
+    return rc;
 }
 
 void __init create_domUs(void)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 3dcdd7a8c46f..da7c7c000f1f 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1296,6 +1296,13 @@ static int __init find_domU_holes(const struct kernel_info *kinfo,
     if ( res )
         goto out;
 
+    if ( kinfo->xen_reg_assigned )
+    {
+        res = rangeset_subtract(mem_holes, kinfo->xen_reg_assigned);
+        if ( res )
+            goto out;
+    }
+
     res = rangeset_report_ranges(mem_holes, 0,
                                  PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
                                  add_ext_regions, ext_regions);
@@ -2329,7 +2336,11 @@ static int __init construct_dom0(struct domain *d)
     if ( rc < 0 )
         return rc;
 
-    return construct_hwdom(&kinfo, NULL);
+    rc = construct_hwdom(&kinfo, NULL);
+
+    rangeset_destroy(kinfo.xen_reg_assigned);
+
+    return rc;
 }
 
 int __init construct_hwdom(struct kernel_info *kinfo,
diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
index bdc96f4c18eb..b3c2d50e1e3d 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -50,6 +50,7 @@ struct kernel_info {
 #ifdef CONFIG_STATIC_SHM
     struct shared_meminfo shm_mem;
 #endif
+    struct rangeset *xen_reg_assigned;
 
     /* kernel entry point */
     paddr_t entry;
-- 
2.49.0

[PATCH 6/6] tools/arm: exclude iomem from domU extended regions

[Stewart Hildebrand]

When a device is passed through to a xl domU, the iomem ranges may
overlap with the extended regions. Remove iomem from extended regions.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
Not sure if we need a Fixes: tag, but if we do:
Fixes: 57f87857dc2d ("libxl/arm: Add handling of extended regions for DomU")
---
 tools/libs/light/libxl_arm.c | 118 +++++++++++++++++++++++++++++------
 1 file changed, 99 insertions(+), 19 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 75c811053c7c..8ae16a1726fc 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -798,6 +798,8 @@ static int make_timer_node(libxl__gc *gc, void *fdt,
     return 0;
 }
 
+#define MAX_NR_EXT_REGIONS   256
+
 static int make_hypervisor_node(libxl__gc *gc, void *fdt,
                                 const libxl_version_info *vers)
 {
@@ -821,7 +823,7 @@ static int make_hypervisor_node(libxl__gc *gc, void *fdt,
      */
     res = fdt_property_reg_placeholder(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
                                        GUEST_ROOT_SIZE_CELLS,
-                                       GUEST_RAM_BANKS + 1);
+                                       MAX_NR_EXT_REGIONS + 1);
     if (res) return res;
 
     /*
@@ -1517,17 +1519,29 @@ static void finalise_one_node(libxl__gc *gc, void *fdt, const char *uname,
 
 #define EXT_REGION_MIN_SIZE   xen_mk_ullong(0x0004000000) /* 64MB */
 
-static int finalize_hypervisor_node(libxl__gc *gc, struct xc_dom_image *dom)
+static int compare_iomem(const void *a, const void *b)
+{
+    const libxl_iomem_range *x = a, *y = b;
+
+    if (x->gfn < y->gfn)
+        return -1;
+    if (x->gfn > y->gfn)
+        return 1;
+    return 0;
+}
+
+static int finalize_hypervisor_node(libxl__gc *gc,
+                                    libxl_domain_build_info *b_info,
+                                    struct xc_dom_image *dom)
 {
     void *fdt = dom->devicetree_blob;
-    uint64_t region_size[GUEST_RAM_BANKS] = {0}, region_base[GUEST_RAM_BANKS],
-        bankend[GUEST_RAM_BANKS];
+    uint64_t region_base[MAX_NR_EXT_REGIONS], region_size[MAX_NR_EXT_REGIONS];
     uint32_t regs[(GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS) *
-                  (GUEST_RAM_BANKS + 1)];
+                  (MAX_NR_EXT_REGIONS + 1)];
     be32 *cells = &regs[0];
     const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
     const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
-    unsigned int i, len, nr_regions = 0;
+    unsigned int i, j, len, nr_regions = 0;
     libxl_dominfo info;
     int offset, rc;
 
@@ -1542,20 +1556,90 @@ static int finalize_hypervisor_node(libxl__gc *gc, struct xc_dom_image *dom)
     if (info.gpaddr_bits > 64)
         return ERROR_INVAL;
 
+    qsort(b_info->iomem, b_info->num_iomem, sizeof(libxl_iomem_range),
+          compare_iomem);
+
     /*
      * Try to allocate separate 2MB-aligned extended regions from the first
      * and second RAM banks taking into the account the maximum supported
      * guest physical address space size and the amount of memory assigned
      * to the guest.
      */
-    for (i = 0; i < GUEST_RAM_BANKS; i++) {
-        region_base[i] = bankbase[i] +
+    for (i = 0; i < GUEST_RAM_BANKS && nr_regions < MAX_NR_EXT_REGIONS; i++) {
+        struct {
+            uint64_t start;
+            uint64_t end; /* inclusive */
+        } unallocated;
+        uint64_t size = 0;
+
+        unallocated.start = bankbase[i] +
             ALIGN_UP_TO_2MB((uint64_t)dom->rambank_size[i] << XC_PAGE_SHIFT);
 
-        bankend[i] = ~0ULL >> (64 - info.gpaddr_bits);
-        bankend[i] = min(bankend[i], bankbase[i] + banksize[i] - 1);
-        if (bankend[i] > region_base[i])
-            region_size[i] = bankend[i] - region_base[i] + 1;
+        unallocated.end = ~0ULL >> (64 - info.gpaddr_bits);
+        unallocated.end = min(unallocated.end, bankbase[i] + banksize[i] - 1);
+
+        if (unallocated.end > unallocated.start)
+            size = unallocated.end - unallocated.start + 1;
+
+        if (size < EXT_REGION_MIN_SIZE)
+            continue;
+
+        /* Exclude iomem */
+        for (j = 0; j < b_info->num_iomem && nr_regions < MAX_NR_EXT_REGIONS;
+             j++) {
+            struct {
+                uint64_t start;
+                uint64_t end; /* inclusive */
+            } iomem;
+
+            iomem.start = b_info->iomem[j].gfn << XC_PAGE_SHIFT;
+            iomem.end = ((b_info->iomem[j].gfn + b_info->iomem[j].number)
+                         << XC_PAGE_SHIFT) - 1;
+
+            if (iomem.end >= unallocated.start
+                && iomem.start <= unallocated.end) {
+
+                if (iomem.start <= unallocated.start) {
+                    unallocated.start = iomem.end + 1;
+
+                    if (iomem.end >= unallocated.end)
+                        /* Complete overlap, discard unallocated region */
+                        break;
+
+                    /* Beginning overlap */
+                    continue;
+                }
+
+                if (iomem.start > unallocated.start) {
+                    assert(unallocated.end > unallocated.start);
+                    size = iomem.start - unallocated.start;
+
+                    if (size >= EXT_REGION_MIN_SIZE) {
+                        region_base[nr_regions] = unallocated.start;
+                        region_size[nr_regions] = size;
+                        nr_regions++;
+                    }
+
+                    unallocated.start = iomem.end + 1;
+
+                    if (iomem.end >= unallocated.end)
+                        /* End overlap, discard remaining unallocated region */
+                        break;
+                }
+            }
+        }
+
+        if (unallocated.end > unallocated.start
+            && nr_regions < MAX_NR_EXT_REGIONS)
+        {
+            size = unallocated.end - unallocated.start + 1;
+
+            if (size >= EXT_REGION_MIN_SIZE) {
+                region_base[nr_regions] = unallocated.start;
+                region_size[nr_regions] = size;
+                nr_regions++;
+            }
+        }
     }
 
     /*
@@ -1565,16 +1649,12 @@ static int finalize_hypervisor_node(libxl__gc *gc, struct xc_dom_image *dom)
     set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
               GUEST_GNTTAB_BASE, GUEST_GNTTAB_SIZE);
 
-    for (i = 0; i < GUEST_RAM_BANKS; i++) {
-        if (region_size[i] < EXT_REGION_MIN_SIZE)
-            continue;
-
+    for (i = 0; i < nr_regions; i++) {
         LOG(DEBUG, "Extended region %u: %#"PRIx64"->%#"PRIx64"",
-            nr_regions, region_base[i], region_base[i] + region_size[i]);
+            i, region_base[i], region_base[i] + region_size[i]);
 
         set_range(&cells, GUEST_ROOT_ADDRESS_CELLS, GUEST_ROOT_SIZE_CELLS,
                   region_base[i], region_size[i]);
-        nr_regions++;
     }
 
     if (!nr_regions)
@@ -1626,7 +1706,7 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc,
 
     }
 
-    res = finalize_hypervisor_node(gc, dom);
+    res = finalize_hypervisor_node(gc, &d_config->b_info, dom);
     if (res)
         return res;
 
-- 
2.49.0

Re: [PATCH 1/6] xen/arm: fix math in add_ext_regions

[Orzel, Michal]

On 05/05/2025 04:56, Stewart Hildebrand wrote:
> In commit f37a59813979, the arguments to add_ext_regions() were switched
> from addresses to frame numbers. add_ext_regions() converts the frame
> numbers back to addresses, but the end address (e) is rounded down to
> page size alignment. The logic to calculate the size assumes e points to
> the last address, not page, effectively leading to the region size being
> erroneously calculated to be 2M smaller than the actual size of the
> region.
> 
> Fix by adding 1 to the frame number before converting back to address.
> 
> Fixes: f37a59813979 ("xen/arm: domain_build: Track unallocated pages using the frame number")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH 2/6] xen/arm: fix math in add_hwdom_free_regions

[Orzel, Michal]

On 05/05/2025 04:56, Stewart Hildebrand wrote:
> Erroneous logic was duplicated from add_ext_regions() into
> add_hwdom_free_regions(). Frame numbers are converted to addresses, but
> the end address (e) is rounded down to page size alignment. The logic to
> calculate the size assumes e points to the last address, not page,
> effectively leading to the region size being erroneously calculated to
> be 2M smaller than the actual size of the region.
> 
> Fix by adding 1 to the frame number before converting back to address.
> 
> Fixes: 02975cc38389 ("xen/arm: permit non direct-mapped Dom0 construction")
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH 3/6] xen/arm: switch find_domU_holes to rangesets

[Stefano Stabellini]

On Sun, 4 May 2025, Stewart Hildebrand wrote:
> remove_shm_holes_for_domU() is unnecessarily complex: it re-creates the
> extended regions from scratch.
> 
> Move the rangeset into find_domU_holes() and create the extended regions
> only once. This makes is simpler to further manipulate the rangeset for
> removing other regions.
> 
> Remove now-unused remove_shm_holes_for_domU().
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/domain_build.c             | 46 ++++++++++++-----
>  xen/arch/arm/include/asm/static-shmem.h |  9 ----
>  xen/arch/arm/static-shmem.c             | 65 -------------------------
>  3 files changed, 35 insertions(+), 85 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index a0f3c074337d..3dcdd7a8c46f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1256,34 +1256,58 @@ static int __init find_domU_holes(const struct kernel_info *kinfo,
>                                    struct membanks *ext_regions)
>  {
>      unsigned int i;
> -    uint64_t bankend;
>      const uint64_t bankbase[] = GUEST_RAM_BANK_BASES;
>      const uint64_t banksize[] = GUEST_RAM_BANK_SIZES;
>      const struct membanks *kinfo_mem = kernel_info_get_mem_const(kinfo);
> -    int res = -ENOENT;
> +    struct rangeset *mem_holes;
> +    int res;
> +
> +    mem_holes = rangeset_new(NULL, NULL, 0);
> +    if ( !mem_holes )
> +        return -ENOMEM;
>  
>      for ( i = 0; i < GUEST_RAM_BANKS; i++ )
>      {
> -        struct membank *ext_bank = &(ext_regions->bank[ext_regions->nr_banks]);
> +        uint64_t bankend, start, size = 0;
>  
> -        ext_bank->start = ROUNDUP(bankbase[i] + kinfo_mem->bank[i].size, SZ_2M);
> +        start = ROUNDUP(bankbase[i] + kinfo_mem->bank[i].size, SZ_2M);
>  
>          bankend = ~0ULL >> (64 - p2m_ipa_bits);
>          bankend = min(bankend, bankbase[i] + banksize[i] - 1);
> -        if ( bankend > ext_bank->start )
> -            ext_bank->size = bankend - ext_bank->start + 1;
> +
> +        if ( bankend > start )
> +            size = bankend - start + 1;
>  
>          /* 64MB is the minimum size of an extended region */
> -        if ( ext_bank->size < MB(64) )
> +        if ( size < MB(64) )
>              continue;
> -        ext_regions->nr_banks++;
> -        res = 0;
> +
> +        res = rangeset_add_range(mem_holes, PFN_DOWN(start), PFN_DOWN(bankend));
> +        if ( res )
> +        {
> +            printk(XENLOG_ERR "Failed to add: %#"PRIx64"->%#"PRIx64"\n",
> +                   start, start + size - 1);
> +            goto out;
> +        }
>      }
>  
> +    /* Remove static shared memory regions */
> +    res = remove_shm_from_rangeset(kinfo, mem_holes);
>      if ( res )
> -        return res;
> +        goto out;
> +
> +    res = rangeset_report_ranges(mem_holes, 0,
> +                                 PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
> +                                 add_ext_regions, ext_regions);
> +    if ( res )
> +        ext_regions->nr_banks = 0;
> +    else if ( !ext_regions->nr_banks )
> +        res = -ENOENT;
>  
> -    return remove_shm_holes_for_domU(kinfo, ext_regions);
> + out:
> +    rangeset_destroy(mem_holes);
> +
> +    return res;
>  }
>  
>  static int __init find_host_extended_regions(const struct kernel_info *kinfo,
> diff --git a/xen/arch/arm/include/asm/static-shmem.h b/xen/arch/arm/include/asm/static-shmem.h
> index 94eaa9d500f9..ad8267c6bfbe 100644
> --- a/xen/arch/arm/include/asm/static-shmem.h
> +++ b/xen/arch/arm/include/asm/static-shmem.h
> @@ -28,9 +28,6 @@ void init_sharedmem_pages(void);
>  int remove_shm_from_rangeset(const struct kernel_info *kinfo,
>                               struct rangeset *rangeset);
>  
> -int remove_shm_holes_for_domU(const struct kernel_info *kinfo,
> -                              struct membanks *ext_regions);
> -
>  int make_shm_resv_memory_node(const struct kernel_info *kinfo, int addrcells,
>                                int sizecells);
>  
> @@ -74,12 +71,6 @@ static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo,
>      return 0;
>  }
>  
> -static inline int remove_shm_holes_for_domU(const struct kernel_info *kinfo,
> -                                            struct membanks *ext_regions)
> -{
> -    return 0;
> -}
> -
>  static inline int make_shm_resv_memory_node(const struct kernel_info *kinfo,
>                                              int addrcells, int sizecells)
>  {
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index e8d4ca3ba3ff..06f32be097c8 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -820,71 +820,6 @@ int __init remove_shm_from_rangeset(const struct kernel_info *kinfo,
>      return 0;
>  }
>  
> -int __init remove_shm_holes_for_domU(const struct kernel_info *kinfo,
> -                                     struct membanks *ext_regions)
> -{
> -    const struct membanks *shm_mem = kernel_info_get_shm_mem_const(kinfo);
> -    struct rangeset *guest_holes;
> -    unsigned int i;
> -    paddr_t start;
> -    paddr_t end;
> -    int res;
> -
> -    /* No static shared memory region. */
> -    if ( shm_mem->nr_banks == 0 )
> -        return 0;
> -
> -    dt_dprintk("Remove static shared memory holes from extended regions of DomU\n");
> -
> -    guest_holes = rangeset_new(NULL, NULL, 0);
> -    if ( !guest_holes )
> -        return -ENOMEM;
> -
> -    /* Copy extended regions sets into the rangeset */
> -    for ( i = 0; i < ext_regions->nr_banks; i++ )
> -    {
> -        start = ext_regions->bank[i].start;
> -        end = start + ext_regions->bank[i].size;
> -
> -        res = rangeset_add_range(guest_holes, PFN_DOWN(start),
> -                                 PFN_DOWN(end - 1));
> -        if ( res )
> -        {
> -            printk(XENLOG_ERR
> -                   "Failed to add: %#"PRIpaddr"->%#"PRIpaddr", error: %d\n",
> -                   start, end, res);
> -            goto out;
> -        }
> -    }
> -
> -    /* Remove static shared memory regions */
> -    res = remove_shm_from_rangeset(kinfo, guest_holes);
> -    if ( res )
> -        goto out;
> -
> -    /*
> -     * Take the interval of memory starting from the first extended region bank
> -     * start address and ending to the end of the last extended region bank.
> -     */
> -    i = ext_regions->nr_banks - 1;
> -    start = ext_regions->bank[0].start;
> -    end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1;
> -
> -    /* Reset original extended regions to hold new value */
> -    ext_regions->nr_banks = 0;
> -    res = rangeset_report_ranges(guest_holes, PFN_DOWN(start), PFN_DOWN(end),
> -                                 add_ext_regions, ext_regions);
> -    if ( res )
> -        ext_regions->nr_banks = 0;
> -    else if ( !ext_regions->nr_banks )
> -        res = -ENOENT;
> -
> - out:
> -    rangeset_destroy(guest_holes);
> -
> -    return res;
> -}
> -
>  void __init shm_mem_node_fill_reg_range(const struct kernel_info *kinfo,
>                                          __be32 *reg, int *nr_cells,
>                                          int addrcells, int sizecells)
> -- 
> 2.49.0
> 

Re: [PATCH 5/6] xen/arm: exclude xen,reg{-cacheable} from domU extended regions

[Stefano Stabellini]

On Sun, 4 May 2025, Stewart Hildebrand wrote:
> When a device is passed through to a dom0less domU, the
> xen,reg{-cacheable} ranges may overlap with the extended regions. Remove
> xen,reg{-cacheable} from extended regions.

There is no reg-cacheable upstream, the commit message should avoid
mentioning it


> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
> Not sure if we need a Fixes: tag, but if we do:
> Fixes: 2a2447757b3c ("xen/arm: implement domU extended regions")
> 
> I investigated an alternate approach of parsing the partial device tree
> again to scan for xen,reg{-cacheable} properties, but it resulted in
> quite a lot of code duplication. Adding a rangeset pointer to "struct
> kernel_info" has a much smaller diffstat, and then we avoid the need to
> parse the partial device tree a second time.
> 
> I discovered this issue when booting a dom0less domU with a device
> passed through. Partial device tree excerpt:
> 
>     passthrough {
>         ... <snip> ...
> 
>         axi_uart16550_0: serial@a0001000 {
>             clocks = <&uart_fixed_clk>;
>             compatible = "ns16550a";
>             interrupt-parent = <&gic>;
>             interrupts = <0 89 4>;
>             reg = <0x0 0xa0001000 0x0 0x1000>;
>             reg-shift = <2>;
> 
>             xen,reg = <0x0 0xa0001000 0x00 0x1000 0x0 0xa0001000>;
>             xen,path = "/amba_pl@0/serial@a0000000";
>             xen,force-assign-without-iommu;
>         };
>     };
> 
> The domU was assigned an extended region overlapping with MMIO of the
> passed through device:
> 
> (XEN) d1: extended region 0: 0xa0000000->0x100000000
> (XEN) d1: extended region 1: 0x200000000->0xf000000000
> 
> The domU panicked when attempting to initialize the device:
> 
> [    3.490068] a0001000.serial: ttyS0 at MMIO 0xa0001000 (irq = 15, base_baud = 6249375) is a 16550A
> [    3.498843] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> [    3.498853] Mem abort info:
> [    3.498855]   ESR = 0x0000000096000044
> [    3.498859]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    3.498864]   SET = 0, FnV = 0
> [    3.498867]   EA = 0, S1PTW = 0
> [    3.498870]   FSC = 0x04: level 0 translation fault
> [    3.498874] Data abort info:
> [    3.498876]   ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
> [    3.498879]   CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> [    3.498884]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    3.498888] [0000000000000010] user address but active_mm is swapper
> [    3.498894] Internal error: Oops: 0000000096000044 [#1] SMP
> [    3.498899] Modules linked in:
> [    3.498908] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.10-stew #1
> [    3.498917] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    3.498924] pc : mem_serial_out+0x18/0x20
> [    3.498936] lr : serial8250_do_set_mctrl+0x6c/0xc0
> [    3.498943] sp : ffff800081bab6d0
> [    3.498946] x29: ffff800081bab6d0 x28: ffff8000815e0dc8 x27: ffff000001c29c60
> [    3.498957] x26: 0000000000000000 x25: ffff00000347b900 x24: ffff000005504c00
> [    3.498968] x23: ffff00000347b800 x22: 0000000000000000 x21: ffff800081b69d78
> [    3.498978] x20: ffff800081b69d78 x19: 0000000000000000 x18: ffffffffffffffff
> [    3.498989] x17: 3d20647561625f65 x16: 736162202c353120 x15: 3d20717269282030
> [    3.498999] x14: 3030313030306178 x13: ffff800081a21ff0 x12: 00000000000007fe
> [    3.499010] x11: 00000000000002aa x10: ffff800081a4dff0 x9 : ffff800081a21ff0
> [    3.499020] x8 : 00000000fffff7ff x7 : ffff800081a4dff0 x6 : 0000000000000008
> [    3.499030] x5 : 0000000000000000 x4 : ffff800080797584 x3 : 0000000000000002
> [    3.499040] x2 : 0000000000000000 x1 : 0000000000000010 x0 : 0000000000000000
> [    3.499050] Call trace:
> [    3.499053]  mem_serial_out+0x18/0x20
> [    3.499059]  serial8250_set_mctrl+0x34/0x40
> [    3.499065]  serial_core_register_port+0x534/0x7dc
> [    3.499075]  serial_ctrl_register_port+0x10/0x1c
> [    3.499084]  uart_add_one_port+0x10/0x1c
> [    3.499092]  serial8250_register_8250_port+0x308/0x4c0
> [    3.499102]  of_platform_serial_probe+0x2c4/0x48c
> [    3.499110]  platform_probe+0x68/0xdc
> [    3.499120]  really_probe+0xbc/0x298
> [    3.499128]  __driver_probe_device+0x78/0x12c
> [    3.499135]  driver_probe_device+0xdc/0x160
> [    3.499142]  __driver_attach+0x94/0x19c
> [    3.499149]  bus_for_each_dev+0x74/0xd0
> [    3.499155]  driver_attach+0x24/0x30
> [    3.499162]  bus_add_driver+0xe4/0x208
> [    3.499168]  driver_register+0x60/0x128
> [    3.499176]  __platform_driver_register+0x24/0x30
> [    3.499184]  of_platform_serial_driver_init+0x1c/0x28
> [    3.499192]  do_one_initcall+0x6c/0x1b0
> [    3.499199]  kernel_init_freeable+0x178/0x258
> [    3.499209]  kernel_init+0x20/0x1d0
> [    3.499218]  ret_from_fork+0x10/0x20
> [    3.499228] Code: f9400800 1ac32021 8b21c001 d50332bf (39000022)
> [    3.499233] ---[ end trace 0000000000000000 ]---
> [    3.499237] note: swapper/0[1] exited with irqs disabled
> [    3.499247] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [    3.499251] SMP: stopping secondary CPUs
> [    3.499284] Kernel Offset: disabled
> [    3.499286] CPU features: 0x00,00000080,00200000,0200420b
> [    3.499292] Memory Limit: none
> [    3.792412] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
> ---
>  xen/arch/arm/dom0less-build.c     | 19 ++++++++++++++++++-
>  xen/arch/arm/domain_build.c       | 13 ++++++++++++-
>  xen/arch/arm/include/asm/kernel.h |  1 +
>  3 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index a356fc94fc4d..23178a801818 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -274,6 +274,14 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>      int res;
>      paddr_t mstart, size, gstart;
>  
> +    if ( !kinfo->xen_reg_assigned )
> +    {
> +        kinfo->xen_reg_assigned = rangeset_new(NULL, NULL, 0);
> +
> +        if ( !kinfo->xen_reg_assigned )
> +            return -ENOMEM;
> +    }
> +
>      /* xen,reg specifies where to map the MMIO region */
>      cell = (const __be32 *)xen_reg->data;
>      len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
> @@ -315,6 +323,11 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>                     mstart, gstart);
>              return -EFAULT;
>          }
> +
> +        res = rangeset_add_range(kinfo->xen_reg_assigned, PFN_DOWN(gstart),
> +                                 PFN_DOWN(gstart + size - 1));
> +        if ( res )
> +            return res;
>      }
>  
>      /*
> @@ -1006,7 +1019,11 @@ static int __init construct_domU(struct domain *d,
>  
>      domain_vcpu_affinity(d, node);
>  
> -    return alloc_xenstore_params(&kinfo);
> +    rc = alloc_xenstore_params(&kinfo);
> +
> +    rangeset_destroy(kinfo.xen_reg_assigned);
> +
> +    return rc;
>  }
>  
>  void __init create_domUs(void)
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 3dcdd7a8c46f..da7c7c000f1f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1296,6 +1296,13 @@ static int __init find_domU_holes(const struct kernel_info *kinfo,
>      if ( res )
>          goto out;
>  
> +    if ( kinfo->xen_reg_assigned )
> +    {
> +        res = rangeset_subtract(mem_holes, kinfo->xen_reg_assigned);
> +        if ( res )
> +            goto out;
> +    }
> +
>      res = rangeset_report_ranges(mem_holes, 0,
>                                   PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
>                                   add_ext_regions, ext_regions);
> @@ -2329,7 +2336,11 @@ static int __init construct_dom0(struct domain *d)
>      if ( rc < 0 )
>          return rc;
>  
> -    return construct_hwdom(&kinfo, NULL);
> +    rc = construct_hwdom(&kinfo, NULL);
> +
> +    rangeset_destroy(kinfo.xen_reg_assigned);

handle_passthrough_prop is never called for the hardware domain


> +    return rc;
>  }
>  
>  int __init construct_hwdom(struct kernel_info *kinfo,
> diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
> index bdc96f4c18eb..b3c2d50e1e3d 100644
> --- a/xen/arch/arm/include/asm/kernel.h
> +++ b/xen/arch/arm/include/asm/kernel.h
> @@ -50,6 +50,7 @@ struct kernel_info {
>  #ifdef CONFIG_STATIC_SHM
>      struct shared_meminfo shm_mem;
>  #endif
> +    struct rangeset *xen_reg_assigned;
>  
>      /* kernel entry point */
>      paddr_t entry;
> -- 
> 2.49.0
> 

Re: [PATCH 5/6] xen/arm: exclude xen,reg{-cacheable} from domU extended regions

[Stewart Hildebrand]

On 5/5/25 19:31, Stefano Stabellini wrote:
> On Sun, 4 May 2025, Stewart Hildebrand wrote:
>> When a device is passed through to a dom0less domU, the
>> xen,reg{-cacheable} ranges may overlap with the extended regions. Remove
>> xen,reg{-cacheable} from extended regions.
> 
> There is no reg-cacheable upstream, the commit message should avoid
> mentioning it

I'll fix

>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> ---
>> Not sure if we need a Fixes: tag, but if we do:
>> Fixes: 2a2447757b3c ("xen/arm: implement domU extended regions")
>>
>> I investigated an alternate approach of parsing the partial device tree
>> again to scan for xen,reg{-cacheable} properties, but it resulted in
>> quite a lot of code duplication. Adding a rangeset pointer to "struct
>> kernel_info" has a much smaller diffstat, and then we avoid the need to
>> parse the partial device tree a second time.
>>
>> I discovered this issue when booting a dom0less domU with a device
>> passed through. Partial device tree excerpt:
>>
>>     passthrough {
>>         ... <snip> ...
>>
>>         axi_uart16550_0: serial@a0001000 {
>>             clocks = <&uart_fixed_clk>;
>>             compatible = "ns16550a";
>>             interrupt-parent = <&gic>;
>>             interrupts = <0 89 4>;
>>             reg = <0x0 0xa0001000 0x0 0x1000>;
>>             reg-shift = <2>;
>>
>>             xen,reg = <0x0 0xa0001000 0x00 0x1000 0x0 0xa0001000>;
>>             xen,path = "/amba_pl@0/serial@a0000000";
>>             xen,force-assign-without-iommu;
>>         };
>>     };
>>
>> The domU was assigned an extended region overlapping with MMIO of the
>> passed through device:
>>
>> (XEN) d1: extended region 0: 0xa0000000->0x100000000
>> (XEN) d1: extended region 1: 0x200000000->0xf000000000
>>
>> The domU panicked when attempting to initialize the device:
>>
>> [    3.490068] a0001000.serial: ttyS0 at MMIO 0xa0001000 (irq = 15, base_baud = 6249375) is a 16550A
>> [    3.498843] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
>> [    3.498853] Mem abort info:
>> [    3.498855]   ESR = 0x0000000096000044
>> [    3.498859]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [    3.498864]   SET = 0, FnV = 0
>> [    3.498867]   EA = 0, S1PTW = 0
>> [    3.498870]   FSC = 0x04: level 0 translation fault
>> [    3.498874] Data abort info:
>> [    3.498876]   ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000
>> [    3.498879]   CM = 0, WnR = 1, TnD = 0, TagAccess = 0
>> [    3.498884]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
>> [    3.498888] [0000000000000010] user address but active_mm is swapper
>> [    3.498894] Internal error: Oops: 0000000096000044 [#1] SMP
>> [    3.498899] Modules linked in:
>> [    3.498908] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.10-stew #1
>> [    3.498917] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> [    3.498924] pc : mem_serial_out+0x18/0x20
>> [    3.498936] lr : serial8250_do_set_mctrl+0x6c/0xc0
>> [    3.498943] sp : ffff800081bab6d0
>> [    3.498946] x29: ffff800081bab6d0 x28: ffff8000815e0dc8 x27: ffff000001c29c60
>> [    3.498957] x26: 0000000000000000 x25: ffff00000347b900 x24: ffff000005504c00
>> [    3.498968] x23: ffff00000347b800 x22: 0000000000000000 x21: ffff800081b69d78
>> [    3.498978] x20: ffff800081b69d78 x19: 0000000000000000 x18: ffffffffffffffff
>> [    3.498989] x17: 3d20647561625f65 x16: 736162202c353120 x15: 3d20717269282030
>> [    3.498999] x14: 3030313030306178 x13: ffff800081a21ff0 x12: 00000000000007fe
>> [    3.499010] x11: 00000000000002aa x10: ffff800081a4dff0 x9 : ffff800081a21ff0
>> [    3.499020] x8 : 00000000fffff7ff x7 : ffff800081a4dff0 x6 : 0000000000000008
>> [    3.499030] x5 : 0000000000000000 x4 : ffff800080797584 x3 : 0000000000000002
>> [    3.499040] x2 : 0000000000000000 x1 : 0000000000000010 x0 : 0000000000000000
>> [    3.499050] Call trace:
>> [    3.499053]  mem_serial_out+0x18/0x20
>> [    3.499059]  serial8250_set_mctrl+0x34/0x40
>> [    3.499065]  serial_core_register_port+0x534/0x7dc
>> [    3.499075]  serial_ctrl_register_port+0x10/0x1c
>> [    3.499084]  uart_add_one_port+0x10/0x1c
>> [    3.499092]  serial8250_register_8250_port+0x308/0x4c0
>> [    3.499102]  of_platform_serial_probe+0x2c4/0x48c
>> [    3.499110]  platform_probe+0x68/0xdc
>> [    3.499120]  really_probe+0xbc/0x298
>> [    3.499128]  __driver_probe_device+0x78/0x12c
>> [    3.499135]  driver_probe_device+0xdc/0x160
>> [    3.499142]  __driver_attach+0x94/0x19c
>> [    3.499149]  bus_for_each_dev+0x74/0xd0
>> [    3.499155]  driver_attach+0x24/0x30
>> [    3.499162]  bus_add_driver+0xe4/0x208
>> [    3.499168]  driver_register+0x60/0x128
>> [    3.499176]  __platform_driver_register+0x24/0x30
>> [    3.499184]  of_platform_serial_driver_init+0x1c/0x28
>> [    3.499192]  do_one_initcall+0x6c/0x1b0
>> [    3.499199]  kernel_init_freeable+0x178/0x258
>> [    3.499209]  kernel_init+0x20/0x1d0
>> [    3.499218]  ret_from_fork+0x10/0x20
>> [    3.499228] Code: f9400800 1ac32021 8b21c001 d50332bf (39000022)
>> [    3.499233] ---[ end trace 0000000000000000 ]---
>> [    3.499237] note: swapper/0[1] exited with irqs disabled
>> [    3.499247] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>> [    3.499251] SMP: stopping secondary CPUs
>> [    3.499284] Kernel Offset: disabled
>> [    3.499286] CPU features: 0x00,00000080,00200000,0200420b
>> [    3.499292] Memory Limit: none
>> [    3.792412] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>> ---
>>  xen/arch/arm/dom0less-build.c     | 19 ++++++++++++++++++-
>>  xen/arch/arm/domain_build.c       | 13 ++++++++++++-
>>  xen/arch/arm/include/asm/kernel.h |  1 +
>>  3 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
>> index a356fc94fc4d..23178a801818 100644
>> --- a/xen/arch/arm/dom0less-build.c
>> +++ b/xen/arch/arm/dom0less-build.c
>> @@ -274,6 +274,14 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>>      int res;
>>      paddr_t mstart, size, gstart;
>>  
>> +    if ( !kinfo->xen_reg_assigned )
>> +    {
>> +        kinfo->xen_reg_assigned = rangeset_new(NULL, NULL, 0);
>> +
>> +        if ( !kinfo->xen_reg_assigned )
>> +            return -ENOMEM;
>> +    }
>> +
>>      /* xen,reg specifies where to map the MMIO region */
>>      cell = (const __be32 *)xen_reg->data;
>>      len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
>> @@ -315,6 +323,11 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
>>                     mstart, gstart);
>>              return -EFAULT;
>>          }
>> +
>> +        res = rangeset_add_range(kinfo->xen_reg_assigned, PFN_DOWN(gstart),
>> +                                 PFN_DOWN(gstart + size - 1));
>> +        if ( res )
>> +            return res;
>>      }
>>  
>>      /*
>> @@ -1006,7 +1019,11 @@ static int __init construct_domU(struct domain *d,
>>  
>>      domain_vcpu_affinity(d, node);
>>  
>> -    return alloc_xenstore_params(&kinfo);
>> +    rc = alloc_xenstore_params(&kinfo);
>> +
>> +    rangeset_destroy(kinfo.xen_reg_assigned);
>> +
>> +    return rc;
>>  }
>>  
>>  void __init create_domUs(void)
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 3dcdd7a8c46f..da7c7c000f1f 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1296,6 +1296,13 @@ static int __init find_domU_holes(const struct kernel_info *kinfo,
>>      if ( res )
>>          goto out;
>>  
>> +    if ( kinfo->xen_reg_assigned )
>> +    {
>> +        res = rangeset_subtract(mem_holes, kinfo->xen_reg_assigned);
>> +        if ( res )
>> +            goto out;
>> +    }
>> +
>>      res = rangeset_report_ranges(mem_holes, 0,
>>                                   PFN_DOWN((1ULL << p2m_ipa_bits) - 1),
>>                                   add_ext_regions, ext_regions);
>> @@ -2329,7 +2336,11 @@ static int __init construct_dom0(struct domain *d)
>>      if ( rc < 0 )
>>          return rc;
>>  
>> -    return construct_hwdom(&kinfo, NULL);
>> +    rc = construct_hwdom(&kinfo, NULL);
>> +
>> +    rangeset_destroy(kinfo.xen_reg_assigned);
> 
> handle_passthrough_prop is never called for the hardware domain

I'll drop this call to rangeset_destroy

>> +    return rc;
>>  }
>>  
>>  int __init construct_hwdom(struct kernel_info *kinfo,
>> diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h
>> index bdc96f4c18eb..b3c2d50e1e3d 100644
>> --- a/xen/arch/arm/include/asm/kernel.h
>> +++ b/xen/arch/arm/include/asm/kernel.h
>> @@ -50,6 +50,7 @@ struct kernel_info {
>>  #ifdef CONFIG_STATIC_SHM
>>      struct shared_meminfo shm_mem;
>>  #endif
>> +    struct rangeset *xen_reg_assigned;
>>  
>>      /* kernel entry point */
>>      paddr_t entry;
>> -- 
>> 2.49.0
>>

Re: [PATCH 2/6] xen/arm: fix math in add_hwdom_free_regions

[Orzel, Michal]

On 05/05/2025 09:52, Orzel, Michal wrote:
> 
> 
> On 05/05/2025 04:56, Stewart Hildebrand wrote:
>> Erroneous logic was duplicated from add_ext_regions() into
>> add_hwdom_free_regions(). Frame numbers are converted to addresses, but
>> the end address (e) is rounded down to page size alignment. The logic to
>> calculate the size assumes e points to the last address, not page,
>> effectively leading to the region size being erroneously calculated to
>> be 2M smaller than the actual size of the region.
>>
>> Fix by adding 1 to the frame number before converting back to address.
>>
>> Fixes: 02975cc38389 ("xen/arm: permit non direct-mapped Dom0 construction")
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> Acked-by: Michal Orzel <michal.orzel@amd.com>

I wanted to commit your fixes but rebase is required after recent dom0less code
movement. Please do.

~Michal

Re: [PATCH 2/6] xen/arm: fix math in add_hwdom_free_regions

[Stewart Hildebrand]

On 5/8/25 02:56, Orzel, Michal wrote:
> On 05/05/2025 09:52, Orzel, Michal wrote:
>>
>>
>> On 05/05/2025 04:56, Stewart Hildebrand wrote:
>>> Erroneous logic was duplicated from add_ext_regions() into
>>> add_hwdom_free_regions(). Frame numbers are converted to addresses, but
>>> the end address (e) is rounded down to page size alignment. The logic to
>>> calculate the size assumes e points to the last address, not page,
>>> effectively leading to the region size being erroneously calculated to
>>> be 2M smaller than the actual size of the region.
>>>
>>> Fix by adding 1 to the frame number before converting back to address.
>>>
>>> Fixes: 02975cc38389 ("xen/arm: permit non direct-mapped Dom0 construction")
>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> Acked-by: Michal Orzel <michal.orzel@amd.com>
> 
> I wanted to commit your fixes but rebase is required after recent dom0less code
> movement. Please do.

Yes, I have already rebased locally. I'll send later today. Is it okay
to keep your A-b tag?

Re: [PATCH 2/6] xen/arm: fix math in add_hwdom_free_regions

[Orzel, Michal]

On 08/05/2025 12:30, Stewart Hildebrand wrote:
> On 5/8/25 02:56, Orzel, Michal wrote:
>> On 05/05/2025 09:52, Orzel, Michal wrote:
>>>
>>>
>>> On 05/05/2025 04:56, Stewart Hildebrand wrote:
>>>> Erroneous logic was duplicated from add_ext_regions() into
>>>> add_hwdom_free_regions(). Frame numbers are converted to addresses, but
>>>> the end address (e) is rounded down to page size alignment. The logic to
>>>> calculate the size assumes e points to the last address, not page,
>>>> effectively leading to the region size being erroneously calculated to
>>>> be 2M smaller than the actual size of the region.
>>>>
>>>> Fix by adding 1 to the frame number before converting back to address.
>>>>
>>>> Fixes: 02975cc38389 ("xen/arm: permit non direct-mapped Dom0 construction")
>>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>> Acked-by: Michal Orzel <michal.orzel@amd.com>
>>
>> I wanted to commit your fixes but rebase is required after recent dom0less code
>> movement. Please do.
> 
> Yes, I have already rebased locally. I'll send later today. Is it okay
> to keep your A-b tag?
Yes, of course.

~Michal