[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH] xen/device-tree: Switch back to dt_unreserved_regions() in boot allocator


  • To: "Orzel, Michal" <michal.orzel@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>
  • Date: Tue, 25 Mar 2025 12:07:24 +0000
  • Accept-language: en-US, ru-RU
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=NyvVXZWcLNkbO4bIvTecDvpIxMLcLUetak3PCG2Vt/s=; b=GJ3aTiqyPKe+PP56rHYPI4L6uXyV12zAgQL3brxdKUj+0hy0NIMDewCRk1Ckcf1gsvAO0ziKV1JcBtekit0IRWoQk/QcsqnjAUclDM6nKFp/mDtoS3gBe4pK4R2WjUwnNNIB3sGy6+TUCQNN1MOvsswyNwMn6KNA1rTfiFjdDhoACeSxpo0UHUVHOxdfftqbUMbj4RxUALzOSM+h67FgW7RK/afhvdsn9YM4mLtInPkWzQaRXcg1H8vfC4l3Z0vxKtq+vemI/C3Q5GlRJBpL5mFyUn08QyHZaJyTiiY34z3x7RpgOBqVue3NwYAROARt7JXEseujmyLQJGfN/5u5Gg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=px2Qn646IxiFdCNX78zcadfzP8vHeYmka44OqfwonBJXtq9GpZa3T1tH0arCKQbKhyg3naaNDsfhvHurPAQxC7fTdsiyA7xqUGdJehyQjgg6EiSZ6BycvtUNUqR8NOYxad0pc0ezvD+lR1lPHb0zA9y3qG7zIsk/Llf2p/DKjW2oae31VTCfFgr23nfMGGWpYCIBlRevl+bp+BnBW/xBTKtPWIz9euTr07M124GeAhykmJ/CXxPDTeKf3b2AqkETjVWIPt6lmXA5i8qZgaarWYoQnvzw2cODP8SbM8E1ybNEjf0ktHWC3UorKHQpHDphv1Zs8lgVV85HEP9hRfKD+g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=epam.com;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>
  • Delivery-date: Tue, 25 Mar 2025 12:07:47 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHbnQONSMLBGwvKMUWMY6fOky1K0LODirsAgAA4J4A=
  • Thread-topic: [RFC PATCH] xen/device-tree: Switch back to dt_unreserved_regions() in boot allocator


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.