On 25.03.25 10:46, Orzel, Michal wrote:
Hello Michal > > > On 24/03/2025 22:27, 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. >> >> 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 (CONFIG_ACPI=y) and >> dt_unreserved_regions() which should simply skip that static range >> does not get called. The acpi_disabled will be set to the actual >> value later on in acpi_boot_table_init(). >> >> To avoid unreserving the whole region (including potential reserved >> memory ranges such as static memory) open code fw_unreserved_regions() >> and add a comment. >> >> Another solution could be to call acpi_boot_table_init() before >> setup_mm() in Arm64's start_xen(). > Interesting issue. However IMO the problem is located somewhere else. At the > moment, with CONFIG_ACPI=n, acpi_disabled is set to true. With CONFIG_ACPI=y, > it's set to false even though entire acpi_boot_table_init() is written with > assumption that ACPI is off by default at the start. That's why > acpi_boot_table_init() calls e.g. disable_acpi() if it finds that user > specified > acpi=off on cmdline. Why would it do that if the assumption was that > acpi_disabled is false? There's even a comment: > https://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/acpi/boot.c;hb=HEAD#l257 > that clearly says that "at this point ACPI is disabled". > > Therefore, I think the fix should look as follows: Very good, thanks. This is much better fix for the issue with incorrect unreserving than any of proposed by current patch. Will use your suggestion for V2. > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index ffcae900d72e..9e94f1a8c761 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -57,7 +57,7 @@ > struct cpuinfo_arm __read_mostly system_cpuinfo; > > #ifdef CONFIG_ACPI > -bool __read_mostly acpi_disabled; > +bool __read_mostly acpi_disabled = true; > #endif > > domid_t __read_mostly max_init_domid; > > ~Michal
