Re: [PATCH v2.1] hvmloader: indicate dynamically allocated memory as ACPI NVS in e820

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 04/09/2020 15:40, Jan Beulich wrote:
> On 04.09.2020 13:49, Igor Druzhinin wrote:
>> On 04/09/2020 09:33, Jan Beulich wrote:
>>> On 01.09.2020 04:50, Igor Druzhinin wrote:
>>>> Guest kernel does need to know in some cases where the tables are located
>>>> to treat these regions properly. One example is kexec process where
>>>> the first kernel needs to pass firmware region locations to the second
>>>> kernel which is now a requirement after 02a3e3cdb7f12 ("x86/boot: Parse 
>>>> SRAT
>>>> table and count immovable memory regions").
>>>
>>> I'm still struggling with the connection here: Reserved regions
>>> surely are "immovable" too, aren't they? 
>>
>> "Immovable" regions here are RAM that doesn't go away by hot-unplug. That 
>> change
>> was necessary in Linux to avoid image randomized placement to these regions.
>>
>>> Where's the connection to
>>> the E820 map in the first place - the change cited above is entirely
>>> about SRAT? And I can't imagine kexec getting away with passing on
>>> ACPI NVS regions, but not reserved ones.
>>>
>>
>> They got away with it for as long as kexec exists I think. The point was that
>> those reserved regions were not accessed during early boot as long as kexec 
>> kernel stays
>> at transition tables. Now ACPI portion of it is accessed which highlighted 
>> our
>> imprecise reporting of memory layout to the guest - which I think should be 
>> fixed
>> either way.
> 
> Is this to mean they map ACPI regions into the transition page tables,
> but not reserved regions?

Yes.

> If so, perhaps that's what the description
> wants to say (and then possibly with a reference to the commit
> introducing this into Linux, instead of the seemingly unrelated SRAT
> one)?

The referenced commit is not unrelated - it's the commit introducing the access
causing kexec transition to fail. But I can add another one pointing to the 
mapping
of ACPI tables that was supposed to fix the failure caused by the first one.

Igor