On Tue, 10 Mar, 2026, 7:44 pm Jan Beulich, <[email protected]> wrote:
> On 10.03.2026 14:05, Soumyajyotii Ssarkar wrote: > > On Mon, Mar 9, 2026 at 1:01 PM Jan Beulich <[email protected]> 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 >
