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

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

[Soumyajyotii Ssarkar]

This patch 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>

Changes since v3:
- Fix casting away const issue.
- Add Stub function
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.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/common/efi/common-stub.c |   1 +
 xen/include/xen/efi.h        |   1 +
 5 files changed, 173 insertions(+)

--
2.53.0

[PATCH v4 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 +++++++++++++++++++++++++++++++++++
 xen/common/efi/common-stub.c |   1 +
 3 files changed, 136 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..47d5b9b2a8 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 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 (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)
+{
+    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);
+    bgrt->image_address = (UINTN)new_image;
+    bgrt->header.checksum = 0;
+    checksum = 0;
+
+    for ( i = 0; i < bgrt->header.length; i++ )
+        checksum += ((const UINT8 *)bgrt)[i];
+
+    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. */
diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
index 77f138a6c5..9e595600d7 100644
--- a/xen/common/efi/common-stub.c
+++ b/xen/common/efi/common-stub.c
@@ -20,6 +20,7 @@ unsigned long efi_get_time(void)

 void efi_halt_system(void) { }
 void efi_reset_system(bool warm) { }
+void __init efi_bgrt_status_info(void) { }

 int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
 {
--
2.53.0

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

[Marek Marczykowski-Górecki]

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

On Tue, Mar 24, 2026 at 03:47:24PM +0530, Soumyajyotii Ssarkar wrote:
> diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
> index 77f138a6c5..9e595600d7 100644
> --- a/xen/common/efi/common-stub.c
> +++ b/xen/common/efi/common-stub.c
> @@ -20,6 +20,7 @@ unsigned long efi_get_time(void)
> 
>  void efi_halt_system(void) { }
>  void efi_reset_system(bool warm) { }
> +void __init efi_bgrt_status_info(void) { }

This has a minor conflict with the current staging, but then for __init
you'd need include xen/init.h.
See:
https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/2405049544

Did it built for you without it?

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

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

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

[Jan Beulich]

On 24.03.2026 12:12, Marek Marczykowski-Górecki wrote:
> On Tue, Mar 24, 2026 at 03:47:24PM +0530, Soumyajyotii Ssarkar wrote:
>> diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
>> index 77f138a6c5..9e595600d7 100644
>> --- a/xen/common/efi/common-stub.c
>> +++ b/xen/common/efi/common-stub.c
>> @@ -20,6 +20,7 @@ unsigned long efi_get_time(void)
>>
>>  void efi_halt_system(void) { }
>>  void efi_reset_system(bool warm) { }
>> +void __init efi_bgrt_status_info(void) { }
> 
> This has a minor conflict with the current staging,

In which case - against which tree was the submission? It shouldn't be
against anything else than current staging, and I don't think there were
any changes in this place recently.

Jan

> but then for __init
> you'd need include xen/init.h.
> See:
> https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/2405049544
> 
> Did it built for you without it?
> 

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

[Marek Marczykowski-Górecki]

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

On Tue, Mar 24, 2026 at 12:23:19PM +0100, Jan Beulich wrote:
> On 24.03.2026 12:12, Marek Marczykowski-Górecki wrote:
> > On Tue, Mar 24, 2026 at 03:47:24PM +0530, Soumyajyotii Ssarkar wrote:
> >> diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
> >> index 77f138a6c5..9e595600d7 100644
> >> --- a/xen/common/efi/common-stub.c
> >> +++ b/xen/common/efi/common-stub.c
> >> @@ -20,6 +20,7 @@ unsigned long efi_get_time(void)
> >>
> >>  void efi_halt_system(void) { }
> >>  void efi_reset_system(bool warm) { }
> >> +void __init efi_bgrt_status_info(void) { }
> > 
> > This has a minor conflict with the current staging,
> 
> In which case - against which tree was the submission? It shouldn't be
> against anything else than current staging, and I don't think there were
> any changes in this place recently.

There was - removal of efi_halt_system().

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

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

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

[Jan Beulich]

On 24.03.2026 12:30, Marek Marczykowski-Górecki wrote:
> On Tue, Mar 24, 2026 at 12:23:19PM +0100, Jan Beulich wrote:
>> On 24.03.2026 12:12, Marek Marczykowski-Górecki wrote:
>>> On Tue, Mar 24, 2026 at 03:47:24PM +0530, Soumyajyotii Ssarkar wrote:
>>>> diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
>>>> index 77f138a6c5..9e595600d7 100644
>>>> --- a/xen/common/efi/common-stub.c
>>>> +++ b/xen/common/efi/common-stub.c
>>>> @@ -20,6 +20,7 @@ unsigned long efi_get_time(void)
>>>>
>>>>  void efi_halt_system(void) { }
>>>>  void efi_reset_system(bool warm) { }
>>>> +void __init efi_bgrt_status_info(void) { }
>>>
>>> This has a minor conflict with the current staging,
>>
>> In which case - against which tree was the submission? It shouldn't be
>> against anything else than current staging, and I don't think there were
>> any changes in this place recently.
> 
> There was - removal of efi_halt_system().

Three weeks ago, yes.

Jan

[PATCH v4 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 47d5b9b2a8..e22a42c15b 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

[PATCH v4 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 e22a42c15b..a4db7ee516 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: [PATCH v4 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption

[Jan Beulich]

On 24.03.2026 11:17, Soumyajyotii Ssarkar wrote:
> This patch 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.

There was another v4 submission about 10 minutes earlier. Which one is the
definitive one?

Jan

[PATCH v4 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>

Changes since v3:
- Fix casting away const issue.
- Add Stub function
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reported-by: Andrew Cooper <andrew.cooper3@citrix.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/common/efi/common-stub.c |   1 +
 xen/include/xen/efi.h        |   1 +
 5 files changed, 173 insertions(+)

--
2.53.0

[PATCH v4 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 +++++++++++++++++++++++++++++++++++
 xen/common/efi/common-stub.c |   1 +
 3 files changed, 136 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..47d5b9b2a8 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 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 (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)
+{
+    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);
+    bgrt->image_address = (UINTN)new_image;
+    bgrt->header.checksum = 0;
+    checksum = 0;
+
+    for ( i = 0; i < bgrt->header.length; i++ )
+        checksum += ((const UINT8 *)bgrt)[i];
+
+    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. */
diff --git a/xen/common/efi/common-stub.c b/xen/common/efi/common-stub.c
index 77f138a6c5..9e595600d7 100644
--- a/xen/common/efi/common-stub.c
+++ b/xen/common/efi/common-stub.c
@@ -20,6 +20,7 @@ unsigned long efi_get_time(void)

 void efi_halt_system(void) { }
 void efi_reset_system(bool warm) { }
+void __init efi_bgrt_status_info(void) { }

 int efi_get_info(uint32_t idx, union xenpf_efi_info *info)
 {
--
2.53.0