On 04.10.2022 17:20, Roger Pau Monné wrote: > On Tue, Oct 04, 2022 at 04:39:26PM +0200, Jan Beulich wrote: >> On 04.10.2022 16:01, Roger Pau Monné wrote: >>> 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. >> >> Hmm, am I getting it right that you think I need to add further >> justification for a change I'm _not_ making? > > In the commit message you explicitly mentioned 'we don't actually > "reclaim" E820_ACPI memory' and I assumed that "we" in the sentence to > only include Xen. Now I see that the "we" there seems to include both > Xen and the dom0 kernel. This wasn't clear to me at first sight.
It was clear, actually, as I did mean Xen alone. It didn't even occur to me that one could consider Dom0 potentially trying to do so. >> And which, if we wanted >> to change our behavior, would require a similar change (or perhaps a >> change elsewhere) in E820 (i.e. non-EFI) handling? > > Why would that be required? Because if EFI can (ab)use that type for other purposes, why couldn't legacy firmware, too? > Without EFI dom0 should be fine in overwriting (some?) of the data in > E820_ACPI regions once it's finished with all ACPI processing, as a > region of type E820_ACPI is reclaimable and Xen won't try to access it > once handled to dom0. > >> The modification >> I'm making is solely towards Xen's internal memory management. I'm >> really having a hard time seeing how commenting on expected Dom0 >> behavior would fit here > > The type in the e820 memory map also gets propagated to dom0 in the > machine memory map hypercall, so it can have effect outside of Xen > itself. If used beyond the very limited intended purposes, yes. >> (leaving aside that I'm still puzzled by both >> you and Andrew thinking that there's any whatsoever remote indication >> anywhere that Dom0 recycling E820_ACPI could be an okay thing in a PV >> Dom0 kernel). The more that marking EfiACPIReclaimMemory anything >> other than E820_ACPI might, as iirc you did say yourself, also confuse >> e.g. the ACPI subsystem of Dom0's kernel. > > Indeed. There's no good way to convert a region of type > EfiACPIReclaimMemory that has the EFI_MEMORY_RUNTIME attribute set, as > there's no mapping to an e820 type. > > One of the quirks of trying to retrofit an EFI memory map into e820 > format. > >> But well, would extending that sentence to "While on x86 in theory the >> same would apply to EfiACPIReclaimMemory, we don't actually "reclaim" >> E820_ACPI memory there (and it would be a bug if the Dom0 kernel tried >> to do so, bypassing Xen's memory management), hence that type's >> handling can be left alone" satisfy your request? > > I think that would indeed make it clearer. Okay, I'll make the adjustment then and submit a v2. This will now need an ack also by Henry anyway. Jan
