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

Soumyajyotii Ssarkar posted 3 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v4 1/3] x86/efi: Add BGRT image preservation infrastructure
Posted by Soumyajyotii Ssarkar 1 week, 5 days ago
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
Posted by Marek Marczykowski-Górecki 1 week, 5 days ago
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
Re: [PATCH v4 1/3] x86/efi: Add BGRT image preservation infrastructure
Posted by Jan Beulich 1 week, 5 days ago
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
Posted by Marek Marczykowski-Górecki 1 week, 5 days ago
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
Re: [PATCH v4 1/3] x86/efi: Add BGRT image preservation infrastructure
Posted by Jan Beulich 1 week, 5 days ago
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