Xen project Mailing List

On Tue, 10 Mar, 2026, 7:44 pm Jan Beulich, <jbeulich@xxxxxxxx> wrote:

On 10.03.2026 14:05, Soumyajyotii Ssarkar wrote:
> On Mon, Mar 9, 2026 at 1:01 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
>> On 08.03.2026 19:30, Marek Marczykowski-Górecki wrote:
>>> On Fri, Mar 06, 2026 at 12:48:08AM +0530, Soumyajyotii Ssarkar wrote:
>>>> --- a/xen/common/efi/boot.c
>>>> +++ b/xen/common/efi/boot.c
>>>> @@ -7,6 +7,7 @@
>>>> #include <xen/ctype.h>
>>>> #include <xen/dmi.h>
>>>> #include <xen/domain_page.h>
>>>> +#include <xen/errno.h>
>>>> #include <xen/init.h>
>>>> #include <xen/keyhandler.h>
>>>> #include <xen/lib.h>
>>>> @@ -173,6 +174,14 @@ static struct file __initdata ramdisk;
>>>> static struct file __initdata xsm;
>>>> static const CHAR16 __initconst newline[] = L"\r\n";
>>>>
>>>> +static __initdata struct {
>>>> + bool preserved;
>>>> + uint64_t old_addr;
>>>> + uint64_t new_addr;
>>>> + uint32_t size;
>>>> + const char *failure_reason;
>>>> +} bgrt_debug_info;
>>>> +
>>>> static void __init PrintStr(const CHAR16 *s)
>>>> {
>>>> StdOut->OutputString(StdOut, (CHAR16 *)s );
>>>> @@ -747,6 +756,171 @@ static void __init
>> efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
>>>> efi_bs->FreePool(memory_map);
>>>> }
>>>>
>>>> +struct bmp_header {
>>>> + uint16_t signature;
>>>> + uint32_t file_size;
>>>> + uint16_t reserved_1;
>>>> + uint16_t reserved_2;
>>>> + uint32_t data_offset;
>>>> +} __attribute__((packed));
>>>> +
>>>> +/*
>>>> + * ACPI Structures - defined locally,
>>>> + * since we cannot include acpi headers
>>>> + * in EFI Context.
>>>> + */
>>>> +
>>>> +struct acpi_rsdp {
>>>> + char signature[8];
>>>> + uint8_t checksum;
>>>> + char oem_id[6];
>>>> + uint8_t revision;
>>>> + uint32_t rsdt_physical_address;
>>>> + uint32_t length;
>>>> + uint64_t xsdt_physical_address;
>>>> + uint8_t extended_checksum;
>>>> + uint8_t reserved[3];
>>>> +} __attribute__((packed));
>>>> +
>>>> +struct acpi_table_header {
>>>> + char signature[4];
>>>> + uint32_t length;
>>>> + uint8_t revision;
>>>> + uint8_t checksum;
>>>> + char oem_id[6];
>>>> + char oem_table_id[8];
>>>> + uint32_t oem_revision;
>>>> + uint32_t asl_compiler_id;
>>>> + uint32_t asl_compiler_revision;
>>>> +} __attribute__((packed));
>>>> +
>>>> +struct acpi_xsdt {
>>>> + struct acpi_table_header header;
>>>> + uint64_t table_offset_entry[1]; /* Variable array length */
>>>
>>> uint64_t table_offset_entry[];
>>>
>>> BTW, do we have some canonical place with list of files imported (and
>>> kept in sync) with other projects? xen/include/acpi/actbl.h doesn't
>>> exactly follow Xen coding style, but it's unclear to me if it needs to
>>> stay this way.
>>
>> I don't really understand why the headers we've got can't be used. Even
>> some of the library-like code under xen/acpi/ may be usable here.
>>
>> While we don't exactly keep xen/include/acpi/ in sync with Linux, when
>> things are added we preferably add them in the way Linux has them.
>>
>>
> I was trying to avoid including the headers from the xen/include/acpi/
> since it was specified in the comment. to not include them.
> Specific comment specified below this paragraph.
> Also since acpi was using datatypes like "u32" while boot.c had types of
> "uint32", so it felt a bit non-standardized.
> I checked the rest of the boot.c which followed the same manner. So I went
> with this choice.
>
> /* * Keep this arch-specific modified include in the common file, as moving
> * it to the arch specific include file would obscure that special care is
> * taken to include it with __ASSEMBLER__ defined.
> */
> #define __ASSEMBLER__ /* avoid pulling in ACPI stuff (conflicts with EFI)
> */
> #include <asm/fixmap.h>
> #undef __ASSEMBLER__
> #endif
>
> The ACPI headers in /xen/include/acpi uses defines such
> as ACPI_NAME_SIZE, ACPI_OEM_ID_SIZE
> and ACPI_OEM_TABLE_ID_SIZE these require adding additional
> <acpi/acconfig.h> header.
> Also since their is no acpi headers included in the boot.c file, so i
> thought to I avoid it.
>
> Thus to get it fully working with ACPI headers from the xen/include/acpi I
> would require these three headers.
> #include <acpi/acconfig.h>
> #include <acpi/actbl.h>
> #include <acpi/actbl3.h>
>
> I thought this would lead to cross contamination, and confusing to further
> modifications in future so weighing my options I thought best to redefine
> them,
> for code clarity.
> Can you suggest me best option to move forward, should I redefine them as
> is or include the headers?

First - see about re-using ACPI functions we already have. Then put new ACPI
code you need to add in a file different from the EFI one.

Jan

Hey Jan,

Referring to the latest BGRT RFC patch series (RFC v3): as you had suggested, I have reused the ACPI headers and made the changes you advised. I hope this version looks satisfactory.

If that approach seems acceptable, I would proceed with introducing a new file and moving the related changes there as part of the next patch series.

In that case, it might also make sense to involve the ESRT maintainers? Since I could lay the foundation for the new file and they could extend it by migrating the ESRT related code their? This might help keep boot.c cleaner?

I would be interested to know others think on this.

Thank you,

Soumyajyotii Ssarkar

Re: [PATCH 1/3] x86/efi: Add BGRT image preservation during boot