Re: [PATCH V2] xen/arm: Initialize acpi_disabled to true during declaration

[Julien Grall <julien@xxxxxxx>]

Hi Oleksandr, Michal,

On 25/03/2025 15:08, Oleksandr Tyshchenko wrote:
> On the device-tree-based Arm64 system, if Xen is built with
> CONFIG_ACPI=y, CONFIG_STATIC_MEMORY=y, and the static memory range
> is provided in the host device tree, the BUG is triggered in
> common/page_alloc.c during Xen's early boot. The BUG occurs when
> the first page from the static range is fed to the domain
> sub-allocator and finally ends up in mark_page_free().
> The pg->count_info & PGC_state is not in the state that
> the code expects to see there.
>
> (XEN) Checking for initrd in /chosen
> (XEN) Checking for "xen,static-mem" in domain node
> (XEN) RAM: 0000000040000000 - 00000000bfffffff
> (XEN)
> (XEN) MODULE[0]: 0000000043200000 - 0000000043343fff Xen
> (XEN) MODULE[1]: 0000000043400000 - 0000000043402fff Device Tree
> (XEN) MODULE[2]: 0000000042e00000 - 0000000043111f82 Ramdisk
> (XEN) MODULE[3]: 0000000040400000 - 0000000042cfffff Kernel
> (XEN)  RESVD[0]: 0000000050000000 - 000000005fffffff
> (XEN)
> (XEN) CMDLINE[0000000040400000]:domU0 console=ttyAMA0
> (XEN)
> (XEN) Command line: console=dtuart conswitch=ax
> (XEN) pg MFN 50000 c=0x2180000000000000 o=0 v=0 t=0
> (XEN) Xen BUG at common/page_alloc.c:1474
> [snip]
>
> The problem is that the static range gets mistakenly unreserved
> in populate_boot_allocator() and reaches init_boot_pages().
> This happens since by the time the populate_boot_allocator()
> is executed, the evaluated in fw_unreserved_regions()
> an acpi_disabled variable is still false and as the result
> the dt_unreserved_regions() which should simply skip that static range
> does not get called. The acpi_disabled will be set to the actual value
> (in our case it is true) later on in acpi_boot_table_init().
> > The important question is why acpi_disabled is false by the time
> setup_mm() is executed. With CONFIG_ACPI=n it is a macro that is always
> true, but with CONFIG_ACPI=y it is a boolean that is false from the very
> beggining, even though the entire acpi_boot_table_init() (which is called
> after setup_mm()) is written with the assumption that ACPI is off by default
> at the start. So, initialize acpi_disabled to true during declaration
> if CONFIG_ACPI=y to avoid an issue and match to acpi_boot_table_init().
While I agree that acpi_disabled should be false. It feels like a bit of 
a workaround for the issue you are trying to solve here. If 
fw_unreserved_regions() doesn't work with ACPI enabled, then it is still 
a problem after your patch.
Furthermore, what happen if we decide to use ACPI afterwards? Wouldn't 
this mean that the static regions would be reserved even if ACPI doesn't 
use static memory (all the memory is expected to be given to the allocator)?
Cheers,

--
Julien Grall