[PATCH v7 2/2] schemas: Add some common reserved-memory usages
Chiu, Chasel
chasel.chiu at intel.com
Thu Jan 4 18:53:29 CET 2024
> -----Original Message-----
> From: Ard Biesheuvel <ardb at kernel.org>
> Sent: Thursday, January 4, 2024 12:43 AM
> To: Chiu, Chasel <chasel.chiu at intel.com>
> Cc: Simon Glass <sjg at chromium.org>; devicetree at vger.kernel.org; Mark Rutland
> <mark.rutland at arm.com>; Rob Herring <robh at kernel.org>; Tan, Lean Sheng
> <sheng.tan at 9elements.com>; lkml <linux-kernel at vger.kernel.org>; Dhaval
> Sharma <dhaval at rivosinc.com>; Brune, Maximilian
> <maximilian.brune at 9elements.com>; Yunhui Cui <cuiyunhui at bytedance.com>;
> Dong, Guo <guo.dong at intel.com>; Tom Rini <trini at konsulko.com>; ron minnich
> <rminnich at gmail.com>; Guo, Gua <gua.guo at intel.com>; linux-
> acpi at vger.kernel.org; U-Boot Mailing List <u-boot at lists.denx.de>
> Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> usages
>
> On Thu, 4 Jan 2024 at 01:25, Chiu, Chasel <chasel.chiu at intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb at kernel.org>
> > > Sent: Wednesday, January 3, 2024 7:22 AM
> > > To: Chiu, Chasel <chasel.chiu at intel.com>
> > > Cc: Simon Glass <sjg at chromium.org>; devicetree at vger.kernel.org; Mark
> > > Rutland <mark.rutland at arm.com>; Rob Herring <robh at kernel.org>; Tan,
> > > Lean Sheng <sheng.tan at 9elements.com>; lkml
> > > <linux-kernel at vger.kernel.org>; Dhaval Sharma <dhaval at rivosinc.com>;
> > > Brune, Maximilian <maximilian.brune at 9elements.com>; Yunhui Cui
> > > <cuiyunhui at bytedance.com>; Dong, Guo <guo.dong at intel.com>; Tom Rini
> > > <trini at konsulko.com>; ron minnich <rminnich at gmail.com>; Guo, Gua
> > > <gua.guo at intel.com>; linux- acpi at vger.kernel.org; U-Boot Mailing
> > > List <u-boot at lists.denx.de>
> > > Subject: Re: [PATCH v7 2/2] schemas: Add some common reserved-memory
> > > usages
> > >
> > > On Fri, 22 Dec 2023 at 20:52, Chiu, Chasel <chasel.chiu at intel.com> wrote:
> > > >
> > > >
> > > > Please see my reply below inline.
> > > >
> > > > Thanks,
> > > > Chasel
> > > >
> > > ...
> > > > > > > The gEfiMemoryTypeInformationGuid HOB typically carries
> > > > > > > platform defaults, and the actual memory type information is
> > > > > > > kept in a non-volatile EFI variable, which gets updated when
> > > > > > > the memory usage changes. Is this different for UefiPayloadPkg?
> > > > > > >
> > > > > > > (For those among the cc'ees less versed in EFI/EDK2: when
> > > > > > > you get the 'config changed -rebooting' message from the
> > > > > > > boot firmware, it typically means that this memory type
> > > > > > > table has changed, and a reboot is necessary.)
> > > > > > >
> > > > > > > So the platform init needs to read this variable, or get the
> > > > > > > information in a different way. I assume it is the payload,
> > > > > > > not the platform init that updates the variable when
> > > > > > > necessary. This means the information flows from payload(n)
> > > > > > > to platform init(n+1), where n is a monotonic index tracking
> > > > > > > consecutive boots of the
> > > system.
> > > > > > >
> > > > > > > Can you explain how the DT fits into this? How are the
> > > > > > > runtime-code and runtime-data memory reservation nodes under
> > > > > > > /reserved-memory used to implement this information exchange
> > > > > > > between platform init and payload? And how do the HOB and
> > > > > > > the EFI
> > > variable fit into this picture?
> > > > > >
> > > > > >
> > > > > > 1. With some offline discussion, we would move
> > > > > > gEfiMemoryTypeInformationGuid usage to FDT->upl-custom node.
> > > > > > This is because it is edk2 implementation choice and non-edk2
> > > > > > PlatformInit or Payload may not have such memory optimization
> > > > > > implementation. (not a generic usage/requirement for
> > > > > > PlatformInit and Payload)
> > > > > >
> > > > > > The edk2 example flow will be like below:
> > > > > >
> > > > > > PlatformInit to GetVariable of gEfiMemoryTypeInformationGuid
> > > > > > and create Hob-
> > > > > >
> > > > > > PlatformInit to initialize FDT->upl-custom node to report
> > > > > gEfiMemoryTypeInformationGuid HOB information ->
> > > > > > UefiPayload entry to re-create
> > > > > > gEfiMemoryTypeInformationGuid HOB basing
> > > > > on FDT input (instead of the default MemoryType inside
> > > > > UefiPayload)
> > > > > ->
> > > > > > UefiPayload DxeMain/Gcd will consume
> > > > > > gEfiMemoryTypeInformationGuid
> > > > > Hob for memory type information ->
> > > > > > UefiPayload to initialize UEFI environment (mainly DXE dispatcher) -
> >
> > > > > > (additional FV binary appended to common UefiPayload
> > > > > > binary)
> > > > > PlatformPayload to provide VariableService which is platform
> > > > > specific ->
> > > > > > UefiPayload UefiBootManager will SetVariable if
> > > > > > memory type change
> > > > > needed and request a warm reset ->
> > > > > > Back to PlatformInit ...
> > > > > >
> > > > >
> > > > > OK so the upl-custom node can do whatever it needs to. I imagine
> > > > > these will include the memory descriptor attribute field, and
> > > > > other parts that may be missing from the /reserved-memory DT node
> specification?
> > > >
> > > >
> > > > Yes, if needed by edk2 specific implementation, not generic
> > > > enough, we may
> > > consider to use upl-custom node to pass those data.
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > 2. Now the proposed reserved-memory node usages will be for
> > > > > > PlatformInit to
> > > > > provide data which may be used by Payload or OS. This is not
> > > > > edk2 specific and any PlatformInit/Payload could have same support.
> > > > > > Note: all of below are optional and PlatformInit may choose to
> > > > > > implement some
> > > > > of them or not.
> > > > > >
> > > > > > - acpi
> > > > > > If PlatformInit created some ACPI tables, this will report a
> > > > > > memory region which
> > > > > contains all the tables to Payload and Payload may base on this
> > > > > to add some more tables if required.
> > > > > >
> > > > > > - acpi-nvs
> > > > > > If PlatformInit has created some ACPI tables which having ACPI
> > > > > > NVS memory
> > > > > dependency, this will be that nvs region.
> > > > > >
> > > > >
> > > > > These make sense.
> > > > >
> > > > > > - boot-code
> > > > > > When PlatformInit having some FW boot phase code that could be
> > > > > > freed for OS to use when payload transferring control to UEFI
> > > > > > OS
> > > > > >
> > > > > > - boot-data
> > > > > > When PlatformInit having some FW boot phase data that could be
> > > > > > freed for OS
> > > > > to use when payload transferring control to UEFI OS.
> > > > > >
> > > > > > - runtime-code
> > > > > > PlatformInit may provide some services code that can be used
> > > > > > for Payload to
> > > > > initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > >
> > > > > > - runtime-data
> > > > > > PlatformInit may provide some services data that can be used
> > > > > > for Payload to
> > > > > Initialize UEFI Runtime Services for supporting UEFI OS.
> > > > > >
> > > > >
> > > > > A UEFI OS must consume this information from the UEFI memory
> > > > > map, not from the /reserved-memory nodes. So these nodes must
> > > > > either not be visible to the OS at all, or carry an annotation
> > > > > that the OS must ignore
> > > them.
> > > > >
> > > > > Would it be possible to include a restriction in the DT schema
> > > > > that these are only valid in the firmware boot phase?
> > > >
> > > >
> > > > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#ef
> > > > i-bo ot-services-exitbootservices Per UEFI specification, UEFI OS
> > > > will always call UEFI GetMemoryMap function to retrieve memory
> > > > map, so FDT
> > > node present or not does not matter to UEFI OS. We probably could
> > > have annotation in UPL specification to emphasize this.
> > > > I'm not familiar with Linux FDT boot, but if non-UEFI OS does not
> > > > call UEFI
> > > GetMemoryMap() and does not know what is runtime-code/data, boot-
> > > code/data, it might just treat such reserved-memory nodes as
> > > 'regular' reserved memory nodes, and that's still ok because
> > > non-UEFI OS will not call to any runtime service or re-purpose boot-code/data
> memory regions.
> > > >
> > >
> > > You are saying the same thing but in a different way. A UEFI OS must
> > > only rely on GetMemoryMap(), and not on the /reserved-memory node to
> > > obtain this information. But this requirement needs to be stated
> > > somewhere: the UEFI spec does not reason about other sources of EFI
> > > memory information at all, and this DT schema does not mention any of this
> either.
> > >
> > > > Would you provide a real OS case which will be impacted by this
> > > > reserved-
> > > memory schema so we can discuss basing on real case?
> > > >
> > >
> > > Funny, that is what I have been trying to get from you :-)
> > >
> > > The problem I am anticipating here is that the information in
> > > /reserved-memory may be out of sync with the EFI memory map. It
> > > needs to be made clear that the EFI memory map is the only source of
> > > truth when the OS is involved, and this /reserved-memory mechanism
> > > should only be used by other firmware stages. But the schema does
> > > not mention this at all. The schema also does not mention that the
> > > information in /reserved-memory is not actually sufficient to
> > > reconstruct the EFI memory map that the firmware payload expects
> > > (which is why the upl- custom-node exists too)
> >
> >
> >
> > Does below solve your concerns if we mention those in schema
> > description? (please feel free to add more if you have) . boot-code/boot-data
> and runtime-code/runtime-data usages are following UEFI specification
> > . before ExitBootServices:
> https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory-
> type-usage-before-exitbootservices
> > . after ExitBootServices:
> > https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#memory
> > -type-usage-after-exitbootservices
> > . These usages do not intend to construct full UEFI memory map, it is only for
> PlatformInit to pass pre-installed tables or services to Payload for supporting UEFI
> OS boot.
> > . These usages are optional
> > . Typically UEFI OS boot will always call GetMemoryMap() to retrieve
> > memory map following UEFI spec, no matter DT nodes present or not
> > (https://uefi.org/specs/UEFI/2.10/07_Services_Boot_Services.html#efi-b
> > oot-services-exitbootservices) . Typically Non-UEFI OS boot will treat
> > those boot* or runtime* reserved-memory as 'regular' reserved memory if
> present.
> >
>
> This already helps quite a lot, thanks.
>
> But why should a non-UEFI OS be required to keep boot* or runtime* regions
> reserved? The firmware stage that boots the OS knows whether it is performing
> an UEFI boot or a non-UEFI boot, and it should only present the information that
> goes along with that. The OS should never have to worry about reconciling two
> sources of truth.
>
> And to Rob's point about boot / runtime being ill-defined: I would argue that
> 'runtime' quite clearly implies 'under the OS', and so UEFI
> runtime* reservations are assumed to always be relevant to UEFI OSes.
>
> I think there is a fundamental difference of opinion here, where the position of
> the firmware developers is that the DT should be the same across all boot stages,
> while my position reasoning from the OS side is that the OS should be able to
> observe only the abstractions that are part of the contract between firmware and
> OS.
I agree that boot* and runtime* can be utilized by non-UEFI OS too, we are just reusing existing definitions from UEFI spec.
. boot-code/boot-data: firmware stage code/data that can be freed after firmware stage ending so OS will have more usable memory.
. runtime-code/runtime-data: firmware stage code/data that are intended to be utilized by OS stage.
Non-UEFI OS still can implement/support boot* or runtime* memory if they want, and the runtime service can be 'non-UEFI' runtime service too as long as OS/FW aligning each other.
Or non-UEFI OS can simply treat them as "usable memory" if they do not call to any runtime services from those memory regions. (in this case runtime* memory can be repurposed just like boot* memory)
That will be OS choices and we may add some example OS handling to schema description too.
While we are working on UPL specific DT, we got agreement that 2 separate DT are unnecessary, we better align/merge with existing OS DT and OS could utilize those additional UPL DT information too if they want.
This also simplifies/unifies PlatformInit as same DT could support different OS loader Payloads.
More information about the U-Boot
mailing list