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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, Oct 04, 2022 at 03:10:57PM +0200, Jan Beulich wrote:
> On 04.10.2022 14:52, Roger Pau Monné wrote:
> > On Tue, Oct 04, 2022 at 02:18:31PM +0200, Jan Beulich wrote:
> >> On 04.10.2022 12:54, Roger Pau Monné wrote:
> >>> On Tue, Oct 04, 2022 at 12:44:16PM +0200, Jan Beulich wrote:
> >>>> On 04.10.2022 12:38, Roger Pau Monné wrote:
> >>>>> On Tue, Oct 04, 2022 at 12:23:23PM +0200, Jan Beulich wrote:
> >>>>>> On 04.10.2022 11:33, Roger Pau Monné wrote:
> >>>>>>> On Tue, Oct 04, 2022 at 10:06:36AM +0200, Jan Beulich wrote:
> >>>>>>>> On 30.09.2022 16:28, Roger Pau Monné wrote:
> >>>>>>>>> On Fri, Sep 30, 2022 at 09:50:40AM +0200, Jan Beulich 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.
> >>>>>>>>>
> >>>>>>>>> What about dom0?  Should it be translated to E820_RESERVED so that
> >>>>>>>>> dom0 doesn't try to use it either?
> >>>>>>>>
> >>>>>>>> I'm afraid I don't understand the questions. Not the least because I
> >>>>>>>> think "it" can't really mean "dom0" from the earlier sentence.
> >>>>>>>
> >>>>>>> Sorry, let me try again:
> >>>>>>>
> >>>>>>> The memory map provided to dom0 will contain E820_ACPI entries for
> >>>>>>> memory ranges with the EFI_MEMORY_RUNTIME attributes in the EFI memory
> >>>>>>> map.  Is there a risk from dom0 reclaiming such E820_ACPI ranges,
> >>>>>>> overwriting the data needed for runtime services?
> >>>>>>
> >>>>>> How would Dom0 go about doing so? It has no control over what we hand
> >>>>>> to the page allocator - it can only free pages which were actually
> >>>>>> allocated to it. E820_ACPI and E820_RESERVED pages are assigned to
> >>>>>> DomIO - Dom0 can map and access them, but it cannot free them.
> >>>>>
> >>>>> Maybe I'm very confused, but what about dom0 overwriting the data
> >>>>> there, won't it cause issues to runtime services?
> >>>>
> >>>> If it overwrites it, of course there are going to be issues. Just like
> >>>> there are going to be problems from anything else Dom0 does wrong.
> >>>
> >>> But would dom0 know it's doing something wrong?
> >>
> >> Yes. Please also see my reply to Andrew.
> >>
> >>> The region is just marked as E820_ACPI from dom0 PoV, so it doesn't
> >>> know it's required by EFI runtime services, and dom0 could
> >>> legitimately overwrite the region once it considers all ACPI parsing
> >>> done from it's side.
> >>
> >> PV Dom0 won't ever see E820_ACPI in the relevant E820 map; this type can
> >> only appear in the machine E820. In how far PVH Dom0 might need to take
> >> special care I can't tell right now (but at least for kexec purposes I
> >> expect Linux isn't going to recycle E820_ACPI regions even going forward).
> > 
> > Even if unlikely, couldn't some dom0 OS look at the machine map after
> > processing ACPI and just decide to overwrite the ACPI regions?
> > 
> > Not that it's useful from an OS PoV, but also we have no statement
> > saying that E820_ACPI in the machine memory map shouldn't be
> > overwritten.
> 
> There are many things we have no statements for, yet we imply certain
> behavior or restrictions. The machine memory map, imo, clearly isn't
> intended for this kind of use.

There isn't much I can say then.  I do feel we are creating rules out
of thin air.

I do think the commit message should mention that we rely on dom0 not
overwriting the data in the E820_ACPI regions on the machine memory
map.

Thanks, Roger.