Re: [PATCH][4.17] EFI: don't convert memory marked for runtime use to ordinary RAM

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.10.2022 12:44, Julien Grall wrote:
> On 04/10/2022 16:58, Jan Beulich wrote:
>> On 30.09.2022 14:51, Bertrand Marquis wrote:
>>>> On 30 Sep 2022, at 09:50, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>
>>>> efi_init_memory() in both relevant places is treating EFI_MEMORY_RUNTIME
>>>> higher priority than the type of the range. To avoid accessing memory at
>>>> runtime which was re-used for other purposes, make
>>>> efi_arch_process_memory_map() follow suit. While on x86 in theory the
>>>> same would apply to EfiACPIReclaimMemory, we don't actually "reclaim"
>>>> E820_ACPI memory there and hence that type's handling can be left alone.
>>>>
>>>> Fixes: bf6501a62e80 ("x86-64: EFI boot code")
>>>> Fixes: facac0af87ef ("x86-64: EFI runtime code")
>>>> Fixes: 6d70ea10d49f ("Add ARM EFI boot support")
>>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx> #arm
>>
>> Thanks. However ...
>>
>>>> ---
>>>> Partly RFC for Arm, for two reasons:
>>>>
>>>> On Arm I question the conversion of EfiACPIReclaimMemory, in two ways:
>>>> For one like on x86 such ranges would likely better be retained, as Dom0
>>>> may (will?) have a need to look at tables placed there. Plus converting
>>>> such ranges to RAM even if EFI_MEMORY_WB is not set looks suspicious to
>>>> me as well. I'd be inclined to make the latter adjustment right here
>>>> (while the other change probably would better be separate, if there
>>>> aren't actually reasons for the present behavior).
>>
>> ... any views on this WB aspect at least (also Stefano or Julien)? Would be
>> good to know before I send v2.
> 
> I don't quite understand what you are questioning here. Looking at the 
> code, EfiACPIReclaimMemory will not be converted to RAM but added in a 
> separate array.
> 
> Furthermore, all the EfiACPIReclaimMemory regions will be passed to dom0 
> (see acpi_create_efi_mmap_table()).
> 
> So to me the code looks correct.

Oh, I've indeed not paid enough attention to the first argument passed
to meminfo_add_bank(). I'm sorry for the extra noise. However, the
question I wanted to have addressed before sending out v3 was that
regarding the present using of memory when EFI_MEMORY_WB is not set.
Is that correct for the EfiACPIReclaimMemory case, i.e. is the
consumer (Dom0) aware that there might be a restriction? And would
this memory then be guaranteed to never be freed into the general pool
of RAM pages?

Jan