* [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot
2026-03-06 13:29 [PATCH v2 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption Soumyajyotii Ssarkar
@ 2026-03-06 13:29 ` Soumyajyotii Ssarkar
2026-03-10 11:31 ` Jan Beulich
2026-03-10 12:07 ` Marek Marczykowski-Górecki
2026-03-06 13:29 ` [PATCH v2 2/3] x86/acpi: Clarify BGRT invalidation behavior with preservation Soumyajyotii Ssarkar
2026-03-06 13:29 ` [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation Soumyajyotii Ssarkar
2 siblings, 2 replies; 9+ messages in thread
From: Soumyajyotii Ssarkar @ 2026-03-06 13:29 UTC (permalink / raw)
To: xen-devel, sarkarsoumyajyoti23
Cc: Jan Beulich, Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper,
Soumyajyotii Ssarkar
The ACPI BGRT (Boot Graphics Resource Table) contains a pointer to a
boot logo image stored in BootServicesData memory. When Xen reclaims
this memory during boot, the image is lost and the BGRT table becomes
invalid, causing Linux dom0 to report ACPI checksum errors.
Add preservation logic similar to ESRT table handling:
- Locate BGRT table via XSDT during EFI boot services phase
- Validate BMP image signature and size (max 16 MB)
- Copy image to EfiACPIReclaimMemory (safe from reclamation)
- Update BGRT table with new image address
- Recalculate ACPI table checksum
The preservation runs automatically during efi_exit_boot() before
Boot Services are terminated. This ensures the image remains
accessible to dom0.
Open-coded ACPI parsing is used because Xen's ACPI subsystem is not
available during the EFI boot phase. The RSDP is obtained from the
EFI System Table, and the XSDT is walked manually to find BGRT.
Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
xen/arch/x86/efi/efi-boot.h | 2 +
xen/common/efi/boot.c | 187 ++++++++++++++++++++++++++++++++++++
2 files changed, 189 insertions(+)
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 42a2c46b5e..27792a56ff 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(SystemTable);
+
efi_exit_boot(ImageHandle, SystemTable);
}
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 967094994d..1e3489e902 100644
--- 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 */
+} __attribute__((packed));
+
+struct acpi_bgrt {
+ struct acpi_table_header header;
+ uint16_t version;
+ uint8_t status;
+ uint8_t image_type;
+ uint64_t image_address;
+ uint32_t image_offset_x;
+ uint32_t image_offset_y;
+} __attribute__((packed));
+
+static struct acpi_bgrt* __init find_bgrt_table(EFI_SYSTEM_TABLE *SystemTable)
+{
+ EFI_GUID acpi2_guid = ACPI_20_TABLE_GUID;
+ struct acpi_rsdp *rsdp = NULL;
+ struct acpi_xsdt *xsdt;
+ struct acpi_bgrt *bgrt;
+ uint32_t entry_count, actual_size;
+ unsigned int i;
+
+ for ( i = 0; i < SystemTable->NumberOfTableEntries; i++ )
+ {
+ if ( match_guid(&acpi2_guid, &SystemTable->ConfigurationTable[i].VendorGuid) )
+ {
+ rsdp = SystemTable->ConfigurationTable[i].VendorTable;
+ break;
+ }
+ }
+
+ if ( !rsdp || !rsdp->xsdt_physical_address )
+ return NULL;
+
+ xsdt = (struct acpi_xsdt *)rsdp->xsdt_physical_address;
+ if ( !xsdt )
+ return NULL;
+
+ actual_size = (xsdt->header.length - sizeof(struct acpi_table_header));
+ entry_count = (actual_size / sizeof(uint64_t));
+
+ for ( i = 0; i < entry_count; i++ )
+ {
+ struct acpi_table_header *header = (struct acpi_table_header *)xsdt->table_offset_entry[i];
+
+ if ( header->signature[0] == 'B'
+ && header->signature[1] == 'G'
+ && header->signature[2] == 'R'
+ && header->signature[3] == 'T' )
+ {
+ bgrt = (struct acpi_bgrt *)header;
+ return bgrt;
+ }
+ }
+ return NULL;
+}
+
+#define MAX_IMAGE_SIZE (16 * 1024 * 1024) /* Sanity check: reject if bigger */
+
+static void __init efi_preserve_bgrt_img(EFI_SYSTEM_TABLE *SystemTable)
+{
+ struct acpi_bgrt *bgrt;
+ struct bmp_header *bmp;
+ void *old_image, *new_image;
+ uint32_t image_size;
+ EFI_STATUS status;
+ uint8_t checksum;
+ unsigned int i;
+
+ bgrt_debug_info.preserved = false;
+ bgrt_debug_info.failure_reason = NULL;
+
+ bgrt = find_bgrt_table(SystemTable);
+ if ( !bgrt )
+ {
+ bgrt_debug_info.failure_reason = "BGRT table not found in XSDT";
+ return;
+ }
+
+ if ( !bgrt->image_address )
+ {
+ bgrt_debug_info.failure_reason = "BGRT image_address is NULL";
+ return;
+ }
+
+ old_image = (void *)bgrt->image_address;
+ bmp = (struct bmp_header *)old_image;
+
+ if ( bmp->signature != 0x4D42 )
+ {
+ bgrt_debug_info.failure_reason = "Invalid BMP signature";
+ return;
+ }
+
+ image_size = bmp->file_size;
+ if ( !image_size || image_size > MAX_IMAGE_SIZE )
+ {
+ bgrt_debug_info.failure_reason = "Invalid image size";
+ return;
+ }
+
+ status = efi_bs->AllocatePool(EfiACPIReclaimMemory, image_size, &new_image);
+ if ( status != EFI_SUCCESS || !new_image )
+ {
+ bgrt_debug_info.failure_reason = "Memory allocation failed";
+ return;
+ }
+
+ memcpy(new_image, old_image, image_size);
+
+ bgrt->image_address = (uint64_t)new_image;
+ bgrt->status |= 0x01;
+
+ bgrt->header.checksum = 0;
+ checksum = 0;
+ for ( i = 0; i < bgrt->header.length; i++ )
+ checksum += ((uint8_t *)bgrt)[i];
+ bgrt->header.checksum = (uint8_t)(0 - checksum);
+
+ bgrt_debug_info.preserved = true;
+ bgrt_debug_info.old_addr = (uint64_t)old_image;
+ bgrt_debug_info.new_addr = (uint64_t)new_image;
+ bgrt_debug_info.size = image_size;
+}
+
/*
* Include architecture specific implementation here, which references the
* static globals defined above.
@@ -1794,6 +1968,19 @@ void __init efi_init_memory(void)
if ( !efi_enabled(EFI_BOOT) )
return;
+ if ( bgrt_debug_info.preserved )
+ {
+ printk(XENLOG_INFO "EFI: BGRT image preserved: %u KB\n",
+ bgrt_debug_info.size / 1024);
+ printk(XENLOG_INFO "EFI: BGRT relocated from %#" PRIx64 " to %#" PRIx64 "\n",
+ bgrt_debug_info.old_addr, bgrt_debug_info.new_addr);
+ }
+ else if ( bgrt_debug_info.failure_reason )
+ {
+ printk(XENLOG_WARNING "EFI: BGRT preservation failed: %s\n",
+ bgrt_debug_info.failure_reason);
+ }
+
printk(XENLOG_DEBUG "EFI memory map:%s\n",
map_bs ? " (mapping BootServices)" : "");
for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot
2026-03-06 13:29 ` [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot Soumyajyotii Ssarkar
@ 2026-03-10 11:31 ` Jan Beulich
2026-03-10 12:07 ` Marek Marczykowski-Górecki
1 sibling, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2026-03-10 11:31 UTC (permalink / raw)
To: Soumyajyotii Ssarkar
Cc: Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper, xen-devel,
sarkarsoumyajyoti23
On 06.03.2026 14:29, Soumyajyotii Ssarkar wrote:
> @@ -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 */
> +} __attribute__((packed));
> +
> +struct acpi_bgrt {
> + struct acpi_table_header header;
> + uint16_t version;
> + uint8_t status;
> + uint8_t image_type;
> + uint64_t image_address;
> + uint32_t image_offset_x;
> + uint32_t image_offset_y;
> +} __attribute__((packed));
> +
> +static struct acpi_bgrt* __init find_bgrt_table(EFI_SYSTEM_TABLE *SystemTable)
> +{
> + EFI_GUID acpi2_guid = ACPI_20_TABLE_GUID;
> + struct acpi_rsdp *rsdp = NULL;
> + struct acpi_xsdt *xsdt;
> + struct acpi_bgrt *bgrt;
> + uint32_t entry_count, actual_size;
No need for fixed-width types here, I expect (see ./CODING_STYLE).
> + unsigned int i;
> +
> + for ( i = 0; i < SystemTable->NumberOfTableEntries; i++ )
> + {
> + if ( match_guid(&acpi2_guid, &SystemTable->ConfigurationTable[i].VendorGuid) )
> + {
> + rsdp = SystemTable->ConfigurationTable[i].VendorTable;
> + break;
> + }
> + }
Why would this be needed, when efi_tables() has already run?
> + if ( !rsdp || !rsdp->xsdt_physical_address )
> + return NULL;
> +
> + xsdt = (struct acpi_xsdt *)rsdp->xsdt_physical_address;
> + if ( !xsdt )
> + return NULL;
> +
> + actual_size = (xsdt->header.length - sizeof(struct acpi_table_header));
> + entry_count = (actual_size / sizeof(uint64_t));
Pleas prefer sizeof(<expression>) over sizeof(<type>), such that as a reader
one can know what is actually meant.
> + for ( i = 0; i < entry_count; i++ )
> + {
> + struct acpi_table_header *header = (struct acpi_table_header *)xsdt->table_offset_entry[i];
> +
> + if ( header->signature[0] == 'B'
> + && header->signature[1] == 'G'
> + && header->signature[2] == 'R'
> + && header->signature[3] == 'T' )
Nit (style): If this was to not be replaced by a suitable function call, the
operators belong at the end of the earlier line (again see ./CODING_STYLE).
> + {
> + bgrt = (struct acpi_bgrt *)header;
> + return bgrt;
> + }
> + }
> + return NULL;
> +}
Nit (style): Blank line please ahead of the main return statement of a
function.
> +#define MAX_IMAGE_SIZE (16 * 1024 * 1024) /* Sanity check: reject if bigger */
> +
> +static void __init efi_preserve_bgrt_img(EFI_SYSTEM_TABLE *SystemTable)
> +{
> + struct acpi_bgrt *bgrt;
> + struct bmp_header *bmp;
> + void *old_image, *new_image;
> + uint32_t image_size;
> + EFI_STATUS status;
> + uint8_t checksum;
> + unsigned int i;
> +
> + bgrt_debug_info.preserved = false;
> + bgrt_debug_info.failure_reason = NULL;
> +
> + bgrt = find_bgrt_table(SystemTable);
> + if ( !bgrt )
> + {
> + bgrt_debug_info.failure_reason = "BGRT table not found in XSDT";
> + return;
> + }
> +
> + if ( !bgrt->image_address )
> + {
> + bgrt_debug_info.failure_reason = "BGRT image_address is NULL";
> + return;
> + }
> +
> + old_image = (void *)bgrt->image_address;
> + bmp = (struct bmp_header *)old_image;
> +
> + if ( bmp->signature != 0x4D42 )
> + {
> + bgrt_debug_info.failure_reason = "Invalid BMP signature";
> + return;
> + }
> +
> + image_size = bmp->file_size;
> + if ( !image_size || image_size > MAX_IMAGE_SIZE )
> + {
> + bgrt_debug_info.failure_reason = "Invalid image size";
Why "invalid"? The cap is arbitrary, isn't it?
> + return;
> + }
> +
> + status = efi_bs->AllocatePool(EfiACPIReclaimMemory, image_size, &new_image);
> + if ( status != EFI_SUCCESS || !new_image )
> + {
> + bgrt_debug_info.failure_reason = "Memory allocation failed";
> + return;
> + }
> +
> + memcpy(new_image, old_image, image_size);
> +
> + bgrt->image_address = (uint64_t)new_image;
> + bgrt->status |= 0x01;
> +
> + bgrt->header.checksum = 0;
> + checksum = 0;
> + for ( i = 0; i < bgrt->header.length; i++ )
> + checksum += ((uint8_t *)bgrt)[i];
> + bgrt->header.checksum = (uint8_t)(0 - checksum);
> +
> + bgrt_debug_info.preserved = true;
> + bgrt_debug_info.old_addr = (uint64_t)old_image;
> + bgrt_debug_info.new_addr = (uint64_t)new_image;
Seeing how you need to cast here, imo using pointer type fields and ...
> + bgrt_debug_info.size = image_size;
> +}
> +
> /*
> * Include architecture specific implementation here, which references the
> * static globals defined above.
> @@ -1794,6 +1968,19 @@ void __init efi_init_memory(void)
> if ( !efi_enabled(EFI_BOOT) )
> return;
>
> + if ( bgrt_debug_info.preserved )
> + {
> + printk(XENLOG_INFO "EFI: BGRT image preserved: %u KB\n",
> + bgrt_debug_info.size / 1024);
> + printk(XENLOG_INFO "EFI: BGRT relocated from %#" PRIx64 " to %#" PRIx64 "\n",
... %p here would be preferable. With any casts between uint64_t / UINT64 and
pointer types you need to be aware that these will cause issues the moment we
gain a 32-bit use of this EFI interfacing code. Hence the fewer such casts,
the better.
> + bgrt_debug_info.old_addr, bgrt_debug_info.new_addr);
> + }
> + else if ( bgrt_debug_info.failure_reason )
> + {
> + printk(XENLOG_WARNING "EFI: BGRT preservation failed: %s\n",
> + bgrt_debug_info.failure_reason);
Did you verify this actually works? efi_preserve_bgrt_img() runs when we're
still in physical mode, and hence the pointers stored will be physical
addresses. Whereas here you need virtual ones. A trick to use may be to
initialize the field with a pointer to a string literal (perhaps simply an
empty string). That'll cause a relocation to be emitted for the field, and
hence the pointer will then be relocated together with the rest of the Xen
image.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot
2026-03-06 13:29 ` [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot Soumyajyotii Ssarkar
2026-03-10 11:31 ` Jan Beulich
@ 2026-03-10 12:07 ` Marek Marczykowski-Górecki
1 sibling, 0 replies; 9+ messages in thread
From: Marek Marczykowski-Górecki @ 2026-03-10 12:07 UTC (permalink / raw)
To: Soumyajyotii Ssarkar
Cc: xen-devel, sarkarsoumyajyoti23, Jan Beulich, Daniel P . Smith,
Roger Pau Monné, Andrew Cooper
[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]
On Fri, Mar 06, 2026 at 06:59:33PM +0530, Soumyajyotii Ssarkar wrote:
> The ACPI BGRT (Boot Graphics Resource Table) contains a pointer to a
> boot logo image stored in BootServicesData memory. When Xen reclaims
> this memory during boot, the image is lost and the BGRT table becomes
> invalid, causing Linux dom0 to report ACPI checksum errors.
>
> Add preservation logic similar to ESRT table handling:
> - Locate BGRT table via XSDT during EFI boot services phase
> - Validate BMP image signature and size (max 16 MB)
> - Copy image to EfiACPIReclaimMemory (safe from reclamation)
> - Update BGRT table with new image address
> - Recalculate ACPI table checksum
>
> The preservation runs automatically during efi_exit_boot() before
> Boot Services are terminated. This ensures the image remains
> accessible to dom0.
>
> Open-coded ACPI parsing is used because Xen's ACPI subsystem is not
> available during the EFI boot phase. The RSDP is obtained from the
> EFI System Table, and the XSDT is walked manually to find BGRT.
>
> Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
> ---
> xen/arch/x86/efi/efi-boot.h | 2 +
> xen/common/efi/boot.c | 187 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 189 insertions(+)
>
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 42a2c46b5e..27792a56ff 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(SystemTable);
> +
See my remark on v1 - efi_start() wants this change too.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] x86/acpi: Clarify BGRT invalidation behavior with preservation
2026-03-06 13:29 [PATCH v2 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption Soumyajyotii Ssarkar
2026-03-06 13:29 ` [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot Soumyajyotii Ssarkar
@ 2026-03-06 13:29 ` Soumyajyotii Ssarkar
2026-03-10 11:33 ` Jan Beulich
2026-03-06 13:29 ` [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation Soumyajyotii Ssarkar
2 siblings, 1 reply; 9+ messages in thread
From: Soumyajyotii Ssarkar @ 2026-03-06 13:29 UTC (permalink / raw)
To: xen-devel, sarkarsoumyajyoti23
Cc: Jan Beulich, Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper,
Soumyajyotii Ssarkar
Now that BGRT images are preserved during EFI boot (via
EfiACPIReclaimMemory allocation), the invalidation code in
acpi_parse_bgrt() is no longer needed.
However, The original invalidation code acts as a safety net for when
preservation fails or is disabled via "efi=no-bgrt".
Thus, Add comments to clarify this behavior for future reference.
Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
xen/arch/x86/acpi/boot.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
index 1ca2360e00..9462cc6195 100644
--- a/xen/arch/x86/acpi/boot.c
+++ b/xen/arch/x86/acpi/boot.c
@@ -327,6 +327,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 =
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/3] x86/acpi: Clarify BGRT invalidation behavior with preservation
2026-03-06 13:29 ` [PATCH v2 2/3] x86/acpi: Clarify BGRT invalidation behavior with preservation Soumyajyotii Ssarkar
@ 2026-03-10 11:33 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2026-03-10 11:33 UTC (permalink / raw)
To: Soumyajyotii Ssarkar
Cc: Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper, xen-devel,
sarkarsoumyajyoti23
On 06.03.2026 14:29, Soumyajyotii Ssarkar wrote:
> Now that BGRT images are preserved during EFI boot (via
> EfiACPIReclaimMemory allocation), the invalidation code in
> acpi_parse_bgrt() is no longer needed.
> However, The original invalidation code acts as a safety net for when
> preservation fails or is disabled via "efi=no-bgrt".
>
> Thus, Add comments to clarify this behavior for future reference.
>
> Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
I wonder though if this wasn't better folded into the earlier patch.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation
2026-03-06 13:29 [PATCH v2 0/3] Fixing ACPI BGRT (Boot Graphics Resource Table) corruption Soumyajyotii Ssarkar
2026-03-06 13:29 ` [PATCH v2 1/3] x86/efi: Add BGRT image preservation during boot Soumyajyotii Ssarkar
2026-03-06 13:29 ` [PATCH v2 2/3] x86/acpi: Clarify BGRT invalidation behavior with preservation Soumyajyotii Ssarkar
@ 2026-03-06 13:29 ` Soumyajyotii Ssarkar
2026-03-10 11:41 ` Jan Beulich
2 siblings, 1 reply; 9+ messages in thread
From: Soumyajyotii Ssarkar @ 2026-03-06 13:29 UTC (permalink / raw)
To: xen-devel, sarkarsoumyajyoti23
Cc: Jan Beulich, Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper,
Soumyajyotii Ssarkar
As described in the task, BGRT preservation is now enabled by default to fix ACPI corruption
for desktop/workstation systems (similar to ESRT).
Add an opt-out parameter 'efi=no-bgrt' to allow disabling BGRT
preservation on systems where the ~1MB memory overhead is not
desired.
The parameter is parsed during normal Xen boot (not during EFI
phase), so it only affects diagnostic logging. The opt-out flag
is checked at the start of efi_preserve_bgrt_img().
Usage:
Default: BGRT preserved automatically
Opt-out: Add 'efi=no-bgrt' to Xen command line
Signed-off-by: Soumyajyotii Ssarkar <soumyajyotisarkar23@gmail.com>
---
xen/common/efi/boot.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 1e3489e902..b735eac6b2 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -167,6 +167,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;
@@ -868,6 +869,9 @@ static void __init efi_preserve_bgrt_img(EFI_SYSTEM_TABLE *SystemTable)
bgrt_debug_info.preserved = false;
bgrt_debug_info.failure_reason = NULL;
+ if ( opt_bgrt_disabled )
+ return;
+
bgrt = find_bgrt_table(SystemTable);
if ( !bgrt )
{
@@ -1873,6 +1877,10 @@ static int __init cf_check parse_efi_param(const char *s)
else
__clear_bit(EFI_RS, &efi_flags);
}
+ else if ( (ss - s) == 7 && !memcmp(s, "no-bgrt", 7) )
+ {
+ opt_bgrt_disabled = true;
+ }
else if ( (ss - s) > 5 && !memcmp(s, "attr=", 5) )
{
if ( !cmdline_strcmp(s + 5, "uc") )
@@ -1968,7 +1976,11 @@ void __init efi_init_memory(void)
if ( !efi_enabled(EFI_BOOT) )
return;
- if ( bgrt_debug_info.preserved )
+ if ( opt_bgrt_disabled )
+ {
+ printk(XENLOG_INFO "EFI: BGRT preservation disabled\n");
+ }
+ else if ( bgrt_debug_info.preserved )
{
printk(XENLOG_INFO "EFI: BGRT image preserved: %u KB\n",
bgrt_debug_info.size / 1024);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation
2026-03-06 13:29 ` [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation Soumyajyotii Ssarkar
@ 2026-03-10 11:41 ` Jan Beulich
2026-03-10 12:05 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2026-03-10 11:41 UTC (permalink / raw)
To: Soumyajyotii Ssarkar
Cc: Daniel P . Smith, Roger Pau Monné,
Marek Marczykowski-Górecki, Andrew Cooper, xen-devel,
sarkarsoumyajyoti23
On 06.03.2026 14:29, Soumyajyotii Ssarkar wrote:
> As described in the task, BGRT preservation is now enabled by default to fix ACPI corruption
> for desktop/workstation systems (similar to ESRT).
What's "the task"?
> Add an opt-out parameter 'efi=no-bgrt' to allow disabling BGRT
> preservation on systems where the ~1MB memory overhead is not
> desired.
This looks to contradict ...
> The parameter is parsed during normal Xen boot (not during EFI
> phase), so it only affects diagnostic logging.
... this. For xen.efi you want to add code to the command line parsing
near the top of efi_start(). What to do for the MB2 boot path perhaps
the only thing there is to peek into the command line (see
arch/x86/boot/cmdline.c).
> @@ -1873,6 +1877,10 @@ static int __init cf_check parse_efi_param(const char *s)
> else
> __clear_bit(EFI_RS, &efi_flags);
> }
> + else if ( (ss - s) == 7 && !memcmp(s, "no-bgrt", 7) )
No "no-" prefixes please; you want to use parse_boolean().
> @@ -1968,7 +1976,11 @@ void __init efi_init_memory(void)
> if ( !efi_enabled(EFI_BOOT) )
> return;
>
> - if ( bgrt_debug_info.preserved )
> + if ( opt_bgrt_disabled )
> + {
> + printk(XENLOG_INFO "EFI: BGRT preservation disabled\n");
> + }
No need for figure braces here.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/3] x86/efi: Add opt-out mechanism for BGRT preservation
2026-03-10 11:41 ` Jan Beulich
@ 2026-03-10 12:05 ` Marek Marczykowski-Górecki
0 siblings, 0 replies; 9+ messages in thread
From: Marek Marczykowski-Górecki @ 2026-03-10 12:05 UTC (permalink / raw)
To: Jan Beulich
Cc: Soumyajyotii Ssarkar, Daniel P . Smith, Roger Pau Monné,
Andrew Cooper, xen-devel, sarkarsoumyajyoti23
[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]
On Tue, Mar 10, 2026 at 12:41:40PM +0100, Jan Beulich wrote:
> On 06.03.2026 14:29, Soumyajyotii Ssarkar wrote:
> > As described in the task, BGRT preservation is now enabled by default to fix ACPI corruption
> > for desktop/workstation systems (similar to ESRT).
>
> What's "the task"?
I asked him to work on this, but I don't think there is a gitlab issue
created, so reference to "the task" is not relevant here.
> > Add an opt-out parameter 'efi=no-bgrt' to allow disabling BGRT
> > preservation on systems where the ~1MB memory overhead is not
> > desired.
>
> This looks to contradict ...
>
> > The parameter is parsed during normal Xen boot (not during EFI
> > phase), so it only affects diagnostic logging.
>
> ... this. For xen.efi you want to add code to the command line parsing
> near the top of efi_start(). What to do for the MB2 boot path perhaps
> the only thing there is to peek into the command line (see
> arch/x86/boot/cmdline.c).
See also my response on v1 - for MB2 path, look at efi_multiboot2().
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread