Re: [PATCH] hvmloader: flip "ACPI data" to ACPI NVS type for ACPI table region

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.10.2020 12:50, Igor Druzhinin wrote:
> ACPI specification contains statements describing memory marked with regular
> "ACPI data" type as reclaimable by the guest. Although the guest shouldn't
> really do it if it wants kexec or similar functionality to work, there
> could still be ambiguities in treating these regions as potentially regular
> RAM.
> 
> One such an example is SeaBIOS which currently reports "ACPI data" regions as
> RAM to the guest in its e801 call. The guest then tries to use this region
> for initrd placement and gets stuck.

Any theory on why it would get stuck? Having read the thread rooting
at Sander's report, it hasn't become clear to me where the collision
there is. A consumer of E801 (rather than E820) intends to not use
ACPI data, and hence I consider SeaBIOS right in this regard (the
lack of considering holes is a problem, though).

> --- a/tools/firmware/hvmloader/e820.c
> +++ b/tools/firmware/hvmloader/e820.c
> @@ -202,16 +202,17 @@ int build_e820_table(struct e820entry *e820,
>      nr++;
>  
>      /*
> -     * Mark populated reserved memory that contains ACPI tables as ACPI data.
> +     * Mark populated reserved memory that contains ACPI tables as ACPI NVS.
>       * That should help the guest to treat it correctly later: e.g. pass to
> -     * the next kernel on kexec or reclaim if necessary.
> +     * the next kernel on kexec and prevent space reclaim which is possible
> +     * with regular ACPI data type accoring to ACPI spec v6.3.

Preventing space reclaim is not the business of hvmloader. As per above,
an ACPI unaware OS ought to be permitted to use as ordinary RAM all the
space the tables occupy. Therefore at the very least the comment needs
to reflect that this preventing of space reclaim is a workaround, not
correct behavior.

Also as a nit: "according".

As a consequence I think we will also want to adjust Xen itself to
automatically disable ACPI when it ends up consuming E801 data. Or
alternatively we should consider dropping all E801-related code (as
being inapplicable to 64-bit systems).

Jan