> -----Original Message----- > From: Ard Biesheuvel <ardb@xxxxxxxxxx> > Sent: Thursday, January 4, 2024 12:43 AM > To: Chiu, Chasel <chasel.chiu@xxxxxxxxx> > Cc: Simon Glass <sjg@xxxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; Mark Rutland > <mark.rutland@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Tan, Lean Sheng > <sheng.tan@xxxxxxxxxxxxx>; lkml <linux-kernel@xxxxxxxxxxxxxxx>; Dhaval > Sharma <dhaval@xxxxxxxxxxxx>; Brune, Maximilian > <maximilian.brune@xxxxxxxxxxxxx>; Yunhui Cui <cuiyunhui@xxxxxxxxxxxxx>; > Dong, Guo <guo.dong@xxxxxxxxx>; Tom Rini <trini@xxxxxxxxxxxx>; ron minnich > <rminnich@xxxxxxxxx>; Guo, Gua <gua.guo@xxxxxxxxx>; linux- > acpi@xxxxxxxxxxxxxxx; U-Boot Mailing List <u-boot@xxxxxxxxxxxxx> > 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@xxxxxxxxx> wrote: > > > > > > > > > -----Original Message----- > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx> > > > Sent: Wednesday, January 3, 2024 7:22 AM > > > To: Chiu, Chasel <chasel.chiu@xxxxxxxxx> > > > Cc: Simon Glass <sjg@xxxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; Mark > > > Rutland <mark.rutland@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Tan, > > > Lean Sheng <sheng.tan@xxxxxxxxxxxxx>; lkml > > > <linux-kernel@xxxxxxxxxxxxxxx>; Dhaval Sharma <dhaval@xxxxxxxxxxxx>; > > > Brune, Maximilian <maximilian.brune@xxxxxxxxxxxxx>; Yunhui Cui > > > <cuiyunhui@xxxxxxxxxxxxx>; Dong, Guo <guo.dong@xxxxxxxxx>; Tom Rini > > > <trini@xxxxxxxxxxxx>; ron minnich <rminnich@xxxxxxxxx>; Guo, Gua > > > <gua.guo@xxxxxxxxx>; linux- acpi@xxxxxxxxxxxxxxx; U-Boot Mailing > > > List <u-boot@xxxxxxxxxxxxx> > > > 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@xxxxxxxxx> 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.
