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.hildebr...@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
>>
