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

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Mar 25, 2026 at 04:16:25PM +0100, Jan Beulich wrote:
> On 24.03.2026 13:33, Soumyajyotii Ssarkar wrote:
> > @@ -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;
> >  }
> 
> Does this really need doing from here? If you called it ...
> 
> > --- 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)
> >  {
> 
> ... out of this function, it could be static and no stub (misplaced in
> the earlier patch) would be needed either.

It was here before, and I complained about it, because it printed the
invalidation reason way later than the actual invalidation.

> Furthermore, is the EFI_BOOT check really needed? Without taking either
> of the EFI boot paths, neither bgrt_info.preserved nor
> bgrt_info.failure_reason[0] would have been altered from their initial
> values.
> 
> Jan

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

Attachment: signature.asc
Description: PGP signature