[RFC PATCH v3 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption

[RFC PATCH v3 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption

[Soumyajyotii Ssarkar]

This RFC series plans to addres ACPI BGRT (Boot Graphics Resource Table) corruption,
that occured when Xen reuses the memory containing the boot logo
image before dom0 is able to parse ACPI tables.

The BGRT table contains a pointer to a BMP image stored in
BootServicesData memory. When Xen reclaims this memory early in boot,
the pointer becomes invalid, causing Linux dom0 to report:
Xen: `(XEN) ACPI: BGRT: invalidating v1 image at 0x47cc2018`
Linux (dom0): `ACPI BIOS Warning (bug): Incorrect checksum
in table [BGRT] - 0xF9, should be 0xB4 (20250807/utcksum-58)`

This series:
1. Adds BGRT image preservation infrastructure during EFI boot
(validates BMP format, allocates EfiACPIReclaimMemory, stores pointers)

2. Integrates preservation with ACPI subsystem
(clarifies acpi_invalidate_bgrt() safety net behavior,
adds status reporting via efi_bgrt_status_info())

3. Provides opt-out mechanism
(-nobgrt for xen.efi direct boot, efi=no-bgrt
for multiboot2, both via early EFI-phase parsing)

The preservation is enabled by default to fix the corruption for all
users, with minimal overhead (~1MB). Also, servers that don't need boot
graphics can disable it using the "efi=no-bgrt" option.

Thank you everyone for the constructive feedback! It is really helpful,
I hope this RFC series is upto standards, I would greatly appriciate further feedback.

Changes since v1:
- The BGRT invalidation behaviour should still presist
  and server as a safety net when "efi=no-bgrt" or the
  preservation fails for some reason thus: Add Clarify comment
  for BGRT invalidation behavior with preservation.
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Changes since v2:
- Use existing ACPI headers (actbl.h, actbl3.h) instead of custom structs
- Modify the function names and code in function to conform with Coding Style.
- Reuse efi.acpi20 from efi_tables() instead of manual RSDP search
- Use appropriate types: EFI types for firmware code, unsigned int for
  loop counters (matching ESRT pattern)
- Used of sizeof(<expression>) over sizeof(<type>)
- Remove fixed-type widths & limited typecasting
- Add Blank line ahead of the main return statement of a function.
- Better wording error message for image cap size
- Remove parse_boolean(no-bgrt) since it could be bit misleading
- Add const qualifiers throughout for safety
- Use %p format for pointers (32-bit compatibility)
- Initialize failure_reason with string literal for proper relocation
- Use memcmp() with ACPI_SIG_* constants for signature checks
- Add direct Xen.efi calling.
- Add opt-out mechanism with proper early parsing during EFI phase.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reported-by: Jan Beulich <jbeulich@suse.com>

Please refer to QubesOS Issue for more context:
https://github.com/QubesOS/qubes-issues/issues/10764

This is a companion series to Linux Kernel side at
https://patchew.org/linux/cover.751f45ebbb644244b1d9da3aff289d6b66db4c6b.1773058629.git-series.marmarek@invisiblethingslab.com/

Testing:
- Verified on Intel UEFI system with Fedora 43 dom0
- Before: ACPI checksum errors in dom0
- After: Clean boot, no ACPI warnings
- Memory overhead: ~972 KB (preserved image size)

Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>

Soumyajyotii Ssarkar (3):
  x86/efi: Add BGRT image preservation infrastructure
  x86/acpi: Integrate BGRT preservation with status reporting
  x86/efi: Add opt-out mechanism for BGRT preservation

 xen/arch/x86/acpi/boot.c    |   8 ++
 xen/arch/x86/efi/efi-boot.h |   5 ++
 xen/common/efi/boot.c       | 158 ++++++++++++++++++++++++++++++++++++
 xen/include/xen/efi.h       |   1 +
 4 files changed, 172 insertions(+)

--
2.53.0

[RFC PATCH v3 1/3] x86/efi: Add BGRT image preservation infrastructure

[Soumyajyotii Ssarkar]

Add core EFI boot services code to preserve BGRT (Boot Graphics Resource
Table) images during Xen boot. The BGRT contains a pointer to a boot logo
stored in BootServicesData memory. Without preservation, this memory is
reclaimed causing ACPI checksum errors in dom0.

Implementation:
- Walk XSDT to locate BGRT table (reusing efi.acpi20 from efi_tables())
- Validate BMP image signature and size constraints (max 16MB)
- Allocate EfiACPIReclaimMemory and copy image data
- Update BGRT table with new address and recalculate checksum

The preservation follows the ESRT pattern, running before
ExitBootServices() to ensure image remains accessible.

Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
 xen/arch/x86/efi/efi-boot.h |   2 +
 xen/common/efi/boot.c       | 133 ++++++++++++++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 42a2c46b5e..0547d845cd 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -910,6 +910,8 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle,

     efi_relocate_esrt(SystemTable);

+    efi_preserve_bgrt_img();
+
     efi_exit_boot(ImageHandle, SystemTable);
 }

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 967094994d..e6451130ce 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1,12 +1,16 @@
 #include "efi.h"
 #include <efi/efiprot.h>
 #include <efi/efipciio.h>
+#include <acpi/acconfig.h>
+#include <acpi/actbl.h>
+#include <acpi/actbl3.h>
 #include <public/xen.h>
 #include <xen/bitops.h>
 #include <xen/compile.h>
 #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>
@@ -747,6 +751,133 @@ static void __init efi_relocate_esrt(EFI_SYSTEM_TABLE *SystemTable)
     efi_bs->FreePool(memory_map);
 }

+typedef struct {
+    UINT16 signature;
+    UINT32 file_size;
+    UINT16 reserved[2];
+    UINT32 data_offset;
+} __attribute__((packed)) BMP_HEADER;
+
+static __initdata struct {
+    bool preserved;
+    const void *old_addr;
+    const void *new_addr;
+    UINTN size;
+    const char *failure_reason;
+} bgrt_info = {
+    /* We would prefer the failure_reason to print */
+    .failure_reason = "",
+};
+
+static const struct acpi_table_bgrt *__init efi_get_bgrt(void)
+{
+    const struct acpi_table_rsdp *rsdp;
+    const struct acpi_table_xsdt *xsdt;
+    UINTN entry_count;
+    unsigned int i;
+
+    if ( efi.acpi20 == EFI_INVALID_TABLE_ADDR )
+        return NULL;
+
+    rsdp = (const void *)(UINTN)efi.acpi20;
+    if ( !rsdp || !rsdp->xsdt_physical_address )
+        return NULL;
+
+    xsdt = (const void *)rsdp->xsdt_physical_address;
+
+    if ( memcmp(xsdt->header.signature, ACPI_SIG_XSDT, 4) != 0 )
+        return NULL;
+
+    if ( xsdt->header.length < sizeof(xsdt->header) )
+        return NULL;
+    entry_count = (xsdt->header.length - sizeof(xsdt->header)) /
+                  sizeof(xsdt->table_offset_entry[0]);
+
+    for ( i = 0; i < entry_count; i++ )
+    {
+        const struct acpi_table_header *hdr;
+
+        hdr = (const void *)xsdt->table_offset_entry[i];
+        if ( !hdr )
+            continue;
+
+        if ( memcmp(hdr->signature, ACPI_SIG_BGRT, 4) == 0 &&
+             hdr->length >= sizeof(struct acpi_table_bgrt) )
+            return (const struct acpi_table_bgrt *)hdr;
+    }
+
+    return NULL;
+}
+
+#define BMP_SIGNATURE 0x4D42
+#define MAX_BGRT_IMAGE_SIZE (16 * 1024 * 1024)
+
+static void __init efi_preserve_bgrt_img(void)
+{
+    const struct acpi_table_bgrt *bgrt;
+    const BMP_HEADER *bmp;
+    const void *old_image;
+    void *new_image;
+    UINTN image_size;
+    EFI_STATUS status;
+    UINT8 checksum;
+    unsigned int i;
+
+    bgrt_info.preserved = false;
+
+    bgrt = efi_get_bgrt();
+    if ( !bgrt )
+    {
+        bgrt_info.failure_reason = "BGRT table not found";
+        return;
+    }
+
+    if ( !bgrt->image_address )
+        return;
+
+    old_image = (const void *)bgrt->image_address;
+    bmp = old_image;
+
+    if ( bmp->signature != BMP_SIGNATURE )
+    {
+        bgrt_info.failure_reason = "Invalid BMP signature";
+        return;
+    }
+
+    image_size = bmp->file_size;
+    if ( !image_size || image_size > MAX_BGRT_IMAGE_SIZE )
+    {
+        bgrt_info.failure_reason = "Image size exceeds limit";
+        return;
+    }
+
+    /*
+     * Allocate memory of type EfiACPIReclaimMemory so that the image
+     * will remain available for the OS after ExitBootServices().
+     */
+    status = efi_bs->AllocatePool(EfiACPIReclaimMemory, image_size, &new_image);
+    if ( EFI_ERROR(status) )
+    {
+        bgrt_info.failure_reason = "Memory allocation failed";
+        return;
+    }
+    memcpy(new_image, old_image, image_size);
+    ((struct acpi_table_bgrt *)bgrt)->image_address = (UINTN)new_image;
+    ((struct acpi_table_bgrt *)bgrt)->header.checksum = 0;
+    checksum = 0;
+
+    for ( i = 0; i < bgrt->header.length; i++ )
+        checksum += ((const UINT8 *)bgrt)[i];
+
+    ((struct acpi_table_bgrt *)bgrt)->header.checksum = -checksum;
+
+    /* Filling the debug struct for printing later */
+    bgrt_info.preserved = true;
+    bgrt_info.old_addr = old_image;
+    bgrt_info.new_addr = new_image;
+    bgrt_info.size = image_size;
+}
+
 /*
  * Include architecture specific implementation here, which references the
  * static globals defined above.
@@ -1671,6 +1802,8 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle,

     efi_relocate_esrt(SystemTable);

+    efi_preserve_bgrt_img();
+
     efi_exit_boot(ImageHandle, SystemTable);

     efi_arch_post_exit_boot(); /* Doesn't return. */
--
2.53.0

[RFC PATCH v3 2/3] x86/acpi: Integrate BGRT preservation with status reporting

[Soumyajyotii Ssarkar]

Add status reporting for BGRT preservation and integrate with Xen's
ACPI subsystem:
- efi_bgrt_status_info() prints preservation status (success/failure)
- Called from acpi_boot_init() after ACPI tables are processed
- Clarifying comment explains why invalidation code remains

The invalidation code in acpi_invalidate_bgrt() now acts as a safety
net: if preservation fails, the image remains in conventional RAM
and gets invalidated. If preservation succeeds, the image is in
EfiACPIReclaimMemory which won't match the RAM_TYPE_CONVENTIONAL
check, leaving the table valid.

Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
 xen/arch/x86/acpi/boot.c |  8 ++++++++
 xen/common/efi/boot.c    | 16 ++++++++++++++++
 xen/include/xen/efi.h    |  1 +
 3 files changed, 25 insertions(+)

diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
index 1ca2360e00..20afe79db9 100644
--- a/xen/arch/x86/acpi/boot.c
+++ b/xen/arch/x86/acpi/boot.c
@@ -29,6 +29,7 @@
 #include <xen/mm.h>
 #include <xen/param.h>
 #include <xen/dmi.h>
+#include <xen/efi.h>
 #include <asm/fixmap.h>
 #include <asm/page.h>
 #include <asm/apic.h>
@@ -327,6 +328,11 @@ static int __init cf_check acpi_parse_hpet(struct acpi_table_header *table)
 	return 0;
 }

+/*
+ * Invalidate BGRT if image is in conventional RAM (preservation failed).
+ * If preservation succeeded, image is in EfiACPIReclaimMemory, which
+ * won't match RAM_TYPE_CONVENTIONAL check, so table remains valid.
+ */
 static int __init cf_check acpi_invalidate_bgrt(struct acpi_table_header *table)
 {
 	struct acpi_table_bgrt *bgrt_tbl =
@@ -754,5 +760,7 @@ int __init acpi_boot_init(void)

 	acpi_table_parse(ACPI_SIG_BGRT, acpi_invalidate_bgrt);

+	efi_bgrt_status_info();
+
 	return 0;
 }
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index e6451130ce..68e06d707c 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1911,6 +1911,22 @@ static bool __init cf_check rt_range_valid(unsigned long smfn, unsigned long emf
     return true;
 }

+void __init efi_bgrt_status_info(void)
+{
+    if ( !efi_enabled(EFI_BOOT) )
+        return;
+
+    if ( bgrt_info.preserved )
+    {
+        printk(XENLOG_INFO "EFI: BGRT image preserved: %lu KB\n",
+               bgrt_info.size / 1024);
+        printk(XENLOG_INFO "EFI: BGRT relocated from %p to %p\n",
+               bgrt_info.old_addr, bgrt_info.new_addr);
+    }
+    else if ( bgrt_info.failure_reason[0] )
+        printk(XENLOG_WARNING "EFI: BGRT preservation failed: %s\n",
+               bgrt_info.failure_reason);
+}

 void __init efi_init_memory(void)
 {
diff --git a/xen/include/xen/efi.h b/xen/include/xen/efi.h
index 723cb80852..e72ab3c6b5 100644
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -39,6 +39,7 @@ static inline bool efi_enabled(unsigned int feature)
 extern bool efi_secure_boot;

 void efi_init_memory(void);
+void efi_bgrt_status_info(void);
 bool efi_boot_mem_unused(unsigned long *start, unsigned long *end);
 bool efi_rs_using_pgtables(void);
 unsigned long efi_get_time(void);
--
2.53.0

[RFC PATCH v3 3/3] x86/efi: Add opt-out mechanism for BGRT preservation

[Soumyajyotii Ssarkar]

BGRT preservation is now enabled by default to fix ACPI corruption
for desktop/workstation systems (similar to ESRT).

As described in the task:
https://github.com/QubesOS/qubes-issues/issues/10764

Add an opt-out parameter to allow disabling BGRT preservation on
systems where the ~1MB memory overhead is not desired.

The opt-out is implemented through two boot paths with early parsing
during the EFI boot phase before preservation runs:

1. xen.efi direct boot: '-nobgrt' command line option (parsed in
   efi_start())
2. Multiboot2 (GRUB): 'efi=no-bgrt' peeked from mb2 cmdline tag
   using get_option() in efi_multiboot2()

The flag is checked at the start of efi_preserve_bgrt_img() to
skip preservation entirely when disabled. Status logging indicates
whether preservation was disabled, succeeded, or failed.

Usage:
  Default: BGRT preserved automatically
  xen.efi: Add '-nobgrt' option
  GRUB/MB2: Add 'efi=no-bgrt' to Xen command line

Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
 xen/arch/x86/efi/efi-boot.h |  3 +++
 xen/common/efi/boot.c       | 11 ++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 0547d845cd..6c986cf6c0 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -897,6 +897,9 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle,
         efi_arch_edid(gop_handle);
     }

+    if ( cmdline && get_option(cmdline, "efi=no-bgrt") )
+        opt_bgrt_disabled = true;
+
     efi_arch_edd();
     efi_arch_cpu();

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 68e06d707c..dc46e783f3 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -170,6 +170,7 @@ static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdErr;

 static UINT32 __initdata mdesc_ver;
 static bool __initdata map_bs;
+static bool __initdata opt_bgrt_disabled = false;

 static struct file __initdata cfg;
 static struct file __initdata kernel;
@@ -825,6 +826,9 @@ static void __init efi_preserve_bgrt_img(void)

     bgrt_info.preserved = false;

+    if ( opt_bgrt_disabled )
+        return;
+
     bgrt = efi_get_bgrt();
     if ( !bgrt )
     {
@@ -1582,6 +1586,8 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle,
                     base_video = true;
                 else if ( wstrcmp(ptr + 1, L"mapbs") == 0 )
                     map_bs = true;
+                else if ( wstrcmp(ptr + 1, L"nobgrt") == 0 )
+                    opt_bgrt_disabled = true;
                 else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 )
                     cfg_file_name = ptr + 5;
                 else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 )
@@ -1592,6 +1598,7 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle,
                     PrintStr(L"Xen EFI Loader options:\r\n");
                     PrintStr(L"-basevideo   retain current video mode\r\n");
                     PrintStr(L"-mapbs       map EfiBootServices{Code,Data}\r\n");
+                    PrintStr(L"-nobgrt      disable BGRT preservation\r\n");
                     PrintStr(L"-cfg=<file>  specify configuration file\r\n");
                     PrintStr(L"-help, -?    display this help\r\n");
                     blexit(NULL);
@@ -1916,7 +1923,9 @@ void __init efi_bgrt_status_info(void)
     if ( !efi_enabled(EFI_BOOT) )
         return;

-    if ( bgrt_info.preserved )
+    if ( opt_bgrt_disabled )
+        printk(XENLOG_INFO "EFI: BGRT preservation disabled\n");
+    else if ( bgrt_info.preserved )
     {
         printk(XENLOG_INFO "EFI: BGRT image preserved: %lu KB\n",
                bgrt_info.size / 1024);
--
2.53.0

Re: [RFC PATCH v3 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 3811 bytes --]

On Thu, Mar 12, 2026 at 04:44:11PM +0530, Soumyajyotii Ssarkar wrote:
> This RFC series plans to addres ACPI BGRT (Boot Graphics Resource Table) corruption,
> that occured when Xen reuses the memory containing the boot logo
> image before dom0 is able to parse ACPI tables.
> 
> The BGRT table contains a pointer to a BMP image stored in
> BootServicesData memory. When Xen reclaims this memory early in boot,
> the pointer becomes invalid, causing Linux dom0 to report:
> Xen: `(XEN) ACPI: BGRT: invalidating v1 image at 0x47cc2018`
> Linux (dom0): `ACPI BIOS Warning (bug): Incorrect checksum
> in table [BGRT] - 0xF9, should be 0xB4 (20250807/utcksum-58)`
> 
> This series:
> 1. Adds BGRT image preservation infrastructure during EFI boot
> (validates BMP format, allocates EfiACPIReclaimMemory, stores pointers)
> 
> 2. Integrates preservation with ACPI subsystem
> (clarifies acpi_invalidate_bgrt() safety net behavior,
> adds status reporting via efi_bgrt_status_info())
> 
> 3. Provides opt-out mechanism
> (-nobgrt for xen.efi direct boot, efi=no-bgrt
> for multiboot2, both via early EFI-phase parsing)
> 
> The preservation is enabled by default to fix the corruption for all
> users, with minimal overhead (~1MB). Also, servers that don't need boot
> graphics can disable it using the "efi=no-bgrt" option.
> 
> Thank you everyone for the constructive feedback! It is really helpful,
> I hope this RFC series is upto standards, I would greatly appriciate further feedback.
> 
> Changes since v1:
> - The BGRT invalidation behaviour should still presist
>   and server as a safety net when "efi=no-bgrt" or the
>   preservation fails for some reason thus: Add Clarify comment
>   for BGRT invalidation behavior with preservation.
> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> 
> Changes since v2:
> - Use existing ACPI headers (actbl.h, actbl3.h) instead of custom structs
> - Modify the function names and code in function to conform with Coding Style.
> - Reuse efi.acpi20 from efi_tables() instead of manual RSDP search
> - Use appropriate types: EFI types for firmware code, unsigned int for
>   loop counters (matching ESRT pattern)
> - Used of sizeof(<expression>) over sizeof(<type>)
> - Remove fixed-type widths & limited typecasting
> - Add Blank line ahead of the main return statement of a function.
> - Better wording error message for image cap size
> - Remove parse_boolean(no-bgrt) since it could be bit misleading
> - Add const qualifiers throughout for safety
> - Use %p format for pointers (32-bit compatibility)
> - Initialize failure_reason with string literal for proper relocation
> - Use memcmp() with ACPI_SIG_* constants for signature checks
> - Add direct Xen.efi calling.
> - Add opt-out mechanism with proper early parsing during EFI phase.
> 
> Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> Reported-by: Jan Beulich <jbeulich@suse.com>
> 
> Please refer to QubesOS Issue for more context:
> https://github.com/QubesOS/qubes-issues/issues/10764
> 
> This is a companion series to Linux Kernel side at
> https://patchew.org/linux/cover.751f45ebbb644244b1d9da3aff289d6b66db4c6b.1773058629.git-series.marmarek@invisiblethingslab.com/
> 
> Testing:
> - Verified on Intel UEFI system with Fedora 43 dom0
> - Before: ACPI checksum errors in dom0
> - After: Clean boot, no ACPI warnings
> - Memory overhead: ~972 KB (preserved image size)
> 
> Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>

Hi,

I've pushed this series to CI, and it fails to build in several targets:
https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/2390562809

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v3 2/3] x86/acpi: Integrate BGRT preservation with status reporting

[Andrew Cooper]

On 12/03/2026 11:14 am, Soumyajyotii Ssarkar wrote:
> diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
> index 1ca2360e00..20afe79db9 100644
> --- a/xen/arch/x86/acpi/boot.c
> +++ b/xen/arch/x86/acpi/boot.c
>  static int __init cf_check acpi_invalidate_bgrt(struct acpi_table_header *table)
>  {
>  	struct acpi_table_bgrt *bgrt_tbl =
> @@ -754,5 +760,7 @@ int __init acpi_boot_init(void)
>
>  	acpi_table_parse(ACPI_SIG_BGRT, acpi_invalidate_bgrt);
>
> +	efi_bgrt_status_info();

To help with your build failure, given this call from outside EFI code,
you need...

> +
>  	return 0;
>  }
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index e6451130ce..68e06d707c 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1911,6 +1911,22 @@ static bool __init cf_check rt_range_valid(unsigned long smfn, unsigned long emf
>      return true;
>  }
>
> +void __init efi_bgrt_status_info(void)
> +{
> +    if ( !efi_enabled(EFI_BOOT) )
> +        return;
> +
> +    if ( bgrt_info.preserved )
> +    {
> +        printk(XENLOG_INFO "EFI: BGRT image preserved: %lu KB\n",
> +               bgrt_info.size / 1024);
> +        printk(XENLOG_INFO "EFI: BGRT relocated from %p to %p\n",
> +               bgrt_info.old_addr, bgrt_info.new_addr);
> +    }
> +    else if ( bgrt_info.failure_reason[0] )
> +        printk(XENLOG_WARNING "EFI: BGRT preservation failed: %s\n",
> +               bgrt_info.failure_reason);
> +}

... a matching stub function in xen/common/efi/common-stub.c

xen.efi is constructed by taking non-efi Xen and linking in a few more
object files.  It is not a full rebuild with different toolchain settings.

~Andrew

Re: [RFC PATCH v3 1/3] x86/efi: Add BGRT image preservation infrastructure

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 3379 bytes --]

On Thu, Mar 12, 2026 at 04:44:12PM +0530, Soumyajyotii Ssarkar wrote:
> Add core EFI boot services code to preserve BGRT (Boot Graphics Resource
> Table) images during Xen boot. The BGRT contains a pointer to a boot logo
> stored in BootServicesData memory. Without preservation, this memory is
> reclaimed causing ACPI checksum errors in dom0.
> 
> Implementation:
> - Walk XSDT to locate BGRT table (reusing efi.acpi20 from efi_tables())
> - Validate BMP image signature and size constraints (max 16MB)
> - Allocate EfiACPIReclaimMemory and copy image data
> - Update BGRT table with new address and recalculate checksum
> 
> The preservation follows the ESRT pattern, running before
> ExitBootServices() to ensure image remains accessible.
> 
> Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
> ---
>  xen/arch/x86/efi/efi-boot.h |   2 +
>  xen/common/efi/boot.c       | 133 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 135 insertions(+)
> 

...

> +static void __init efi_preserve_bgrt_img(void)
> +{
> +    const struct acpi_table_bgrt *bgrt;
> +    const BMP_HEADER *bmp;
> +    const void *old_image;
> +    void *new_image;
> +    UINTN image_size;
> +    EFI_STATUS status;
> +    UINT8 checksum;
> +    unsigned int i;
> +
> +    bgrt_info.preserved = false;
> +
> +    bgrt = efi_get_bgrt();
> +    if ( !bgrt )
> +    {
> +        bgrt_info.failure_reason = "BGRT table not found";
> +        return;
> +    }
> +
> +    if ( !bgrt->image_address )
> +        return;
> +
> +    old_image = (const void *)bgrt->image_address;
> +    bmp = old_image;
> +
> +    if ( bmp->signature != BMP_SIGNATURE )
> +    {
> +        bgrt_info.failure_reason = "Invalid BMP signature";
> +        return;
> +    }
> +
> +    image_size = bmp->file_size;
> +    if ( !image_size || image_size > MAX_BGRT_IMAGE_SIZE )
> +    {
> +        bgrt_info.failure_reason = "Image size exceeds limit";
> +        return;
> +    }
> +
> +    /*
> +     * Allocate memory of type EfiACPIReclaimMemory so that the image
> +     * will remain available for the OS after ExitBootServices().
> +     */
> +    status = efi_bs->AllocatePool(EfiACPIReclaimMemory, image_size, &new_image);
> +    if ( EFI_ERROR(status) )
> +    {
> +        bgrt_info.failure_reason = "Memory allocation failed";
> +        return;
> +    }
> +    memcpy(new_image, old_image, image_size);
> +    ((struct acpi_table_bgrt *)bgrt)->image_address = (UINTN)new_image;
> +    ((struct acpi_table_bgrt *)bgrt)->header.checksum = 0;

Question to MISRA experts here - is this casting away of const okay
here? Or maybe better be done on the `bgrt` local variable? Or some
other way?

> +    checksum = 0;
> +
> +    for ( i = 0; i < bgrt->header.length; i++ )
> +        checksum += ((const UINT8 *)bgrt)[i];
> +
> +    ((struct acpi_table_bgrt *)bgrt)->header.checksum = -checksum;
> +
> +    /* Filling the debug struct for printing later */
> +    bgrt_info.preserved = true;
> +    bgrt_info.old_addr = old_image;
> +    bgrt_info.new_addr = new_image;
> +    bgrt_info.size = image_size;
> +}
> +
>  /*
>   * Include architecture specific implementation here, which references the
>   * static globals defined above.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [RFC PATCH v3 1/3] x86/efi: Add BGRT image preservation infrastructure

[Andrew Cooper]

On 17/03/2026 2:18 pm, Marek Marczykowski-Górecki wrote:
> On Thu, Mar 12, 2026 at 04:44:12PM +0530, Soumyajyotii Ssarkar wrote:
>> Add core EFI boot services code to preserve BGRT (Boot Graphics Resource
>> Table) images during Xen boot. The BGRT contains a pointer to a boot logo
>> stored in BootServicesData memory. Without preservation, this memory is
>> reclaimed causing ACPI checksum errors in dom0.
>>
>> Implementation:
>> - Walk XSDT to locate BGRT table (reusing efi.acpi20 from efi_tables())
>> - Validate BMP image signature and size constraints (max 16MB)
>> - Allocate EfiACPIReclaimMemory and copy image data
>> - Update BGRT table with new address and recalculate checksum
>>
>> The preservation follows the ESRT pattern, running before
>> ExitBootServices() to ensure image remains accessible.
>>
>> Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
>> ---
>>  xen/arch/x86/efi/efi-boot.h |   2 +
>>  xen/common/efi/boot.c       | 133 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 135 insertions(+)
>>
> ...
>
>> +static void __init efi_preserve_bgrt_img(void)
>> +{
>> +    const struct acpi_table_bgrt *bgrt;
>> +    const BMP_HEADER *bmp;
>> +    const void *old_image;
>> +    void *new_image;
>> +    UINTN image_size;
>> +    EFI_STATUS status;
>> +    UINT8 checksum;
>> +    unsigned int i;
>> +
>> +    bgrt_info.preserved = false;
>> +
>> +    bgrt = efi_get_bgrt();
>> +    if ( !bgrt )
>> +    {
>> +        bgrt_info.failure_reason = "BGRT table not found";
>> +        return;
>> +    }
>> +
>> +    if ( !bgrt->image_address )
>> +        return;
>> +
>> +    old_image = (const void *)bgrt->image_address;
>> +    bmp = old_image;
>> +
>> +    if ( bmp->signature != BMP_SIGNATURE )
>> +    {
>> +        bgrt_info.failure_reason = "Invalid BMP signature";
>> +        return;
>> +    }
>> +
>> +    image_size = bmp->file_size;
>> +    if ( !image_size || image_size > MAX_BGRT_IMAGE_SIZE )
>> +    {
>> +        bgrt_info.failure_reason = "Image size exceeds limit";
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * Allocate memory of type EfiACPIReclaimMemory so that the image
>> +     * will remain available for the OS after ExitBootServices().
>> +     */
>> +    status = efi_bs->AllocatePool(EfiACPIReclaimMemory, image_size, &new_image);
>> +    if ( EFI_ERROR(status) )
>> +    {
>> +        bgrt_info.failure_reason = "Memory allocation failed";
>> +        return;
>> +    }
>> +    memcpy(new_image, old_image, image_size);
>> +    ((struct acpi_table_bgrt *)bgrt)->image_address = (UINTN)new_image;
>> +    ((struct acpi_table_bgrt *)bgrt)->header.checksum = 0;
> Question to MISRA experts here - is this casting away of const okay
> here? Or maybe better be done on the `bgrt` local variable? Or some
> other way?

Casting away const is not ok.  The bug is in patch 1, with
efi_get_bgrt() returning a const pointer.

You should never lose the mutable pointer, and it is only objects in
rodata which legitimately lack a mutable pointer in the first place. 
Everything else is strictly mutable from C's point of view.

First fix the build issues, then run Eclair on the result.  There are
other issues in the series, I think.

~Andrew