> -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: Wednesday, January 3, 2024 8:01 AM > To: Ard Biesheuvel <ardb@xxxxxxxxxx> > Cc: Chiu, Chasel <chasel.chiu@xxxxxxxxx>; Simon Glass <sjg@xxxxxxxxxxxx>; > devicetree@xxxxxxxxxxxxxxx; Mark Rutland <mark.rutland@xxxxxxx>; 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, Dec 22, 2023 at 5:48 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > On Thu, 21 Dec 2023 at 17:50, Chiu, Chasel <chasel.chiu@xxxxxxxxx> wrote: > > > > > > > > > Hi Ard, > > > > > > Please see my reply below inline and let me know your thoughts. > > > > > > Thanks, > > > Chasel > > > > > > > > > > -----Original Message----- > > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx> > > > > Sent: Thursday, December 21, 2023 6:31 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 Tue, 28 Nov 2023 at 21:31, Chiu, Chasel <chasel.chiu@xxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Ard Biesheuvel <ardb@xxxxxxxxxx> > > > > > > Sent: Tuesday, November 28, 2023 10:08 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 > > > > > > > > > > > > You are referring to a 2000 line patch so it is not 100% clear where to > look tbh. > > > > > > > > > > > > > > > > > > On Tue, 21 Nov 2023 at 19:37, Chiu, Chasel <chasel.chiu@xxxxxxxxx> > wrote: > > > > > > > > > > > > > > > > > > > > > In PR, UefiPayloadPkg/Library/FdtParserLib/FdtParserLib.c, > > > > > > > line > > > > > > > 268 is for > > > > > > related example code. > > > > > > > > > > > > > > > > > > > That refers to a 'memory-allocation' node, right? How does > > > > > > that relate to the 'reserved-memory' node? > > > > > > > > > > > > And crucially, how does this clarify in which way > > > > > > "runtime-code" and > > > > > > "runtime- data" reservations are being used? > > > > > > > > > > > > Since the very beginning of this discussion, I have been > > > > > > asking repeatedly for examples that describe the wider context > > > > > > in which these > > > > reservations are used. > > > > > > The "runtime" into runtime-code and runtime-data means that > > > > > > these regions have a special significance to the operating > > > > > > system, not just to the next bootloader stage. So I want to > > > > > > understand exactly why it is necessary to describe these > > > > > > regions in a way where the operating system might be expected > > > > > > to interpret this information and act > > > > upon it. > > > > > > > > > > > > > > > > > > > > > I think runtime code and data today are mainly for supporting > > > > > UEFI runtime > > > > services - some BIOS functions for OS to utilize, OS may follow > > > > below ACPI spec to treat them as reserved range: > > > > > https://uefi.org/specs/ACPI/6.5/15_System_Address_Map_Interfaces > > > > > .html# uefi-memory-types-and-mapping-to-acpi-address-range-types > > > > > > > > > > Like I mentioned earlier, that PR is still in early phase and > > > > > has not reflected all > > > > the required changes yet, but the idea is to build > > > > gEfiMemoryTypeInformationGuid HOB from FDT reserved-memory nodes. > > > > > UEFI generic Payload has DxeMain integrated, however Memory > > > > > Types are > > > > platform-specific, for example, some platforms may need bigger > > > > runtime memory for their implementation, that's why we want such > > > > FDT reserved-memory node to tell DxeMain. > > > > > > > > > > > > > > The Payload flow will be like this: > > > > > Payload creates built-in default MemoryTypes table -> > > > > > FDT reserved-memory node to override if required (this also > > > > > ensures the > > > > same memory map cross boots so ACPI S4 works) -> > > > > > Build gEfiMemoryTypeInformationGuid HOB by "platfom specific" > > > > MemoryTypes Table -> > > > > > DxeMain/GCD to consume this MemoryTypes table and setup > > > > > memory > > > > service -> > > > > > Install memory types table to UEFI system table.Configuration > table... > > > > > > > > > > Note: if Payload built-in default MemoryTypes table works fine > > > > > for the platform, then FDT reserved-memory node does not need to > > > > > provide such > > > > 'usage' compatible strings. (optional) This FDT node could allow > > > > flexibility/compatibility without rebuilding Payload binary. > > > > > > > > > > Not sure if I answered all your questions, please highlight > > > > > which area you need > > > > more information. > > > > > > > > > > > > > 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? > > > > > > > > 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. > > > > > I'll say it again. "boot" and "runtime" on their own could mean about anything, > but the usage here is clearly tied to UEFI (or the EDK2 > implementation) and its meaning of boot and runtime. So the naming needs to > reflect that. > How about renaming to below? uefiboot-code uefiboot-data uefiruntime-code uefiruntime-data > > > 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. > > The kernel will process /reserved-memory for UEFI boot, so the expectation is > anything in the EFI memory map is not present there. An annotation to ignore > some nodes would require going back in time or accepting 2 sources of truth on > existing OS. > I thought UEFI boot will rely on UEFI GetMemoryMap() for knowing current memory map, and the DT memory/reserved-memory nodes should have been included/converted by GetMemoryMap() function. How kernel handling DT nodes vs GetMemoryMap() in UEFI boot case? > > Would it be possible to include a restriction in the DT schema that > > these are only valid in the firmware boot phase? > > The only way ATM is including a schema or not when running validation on a DT > for a particular boot phase. Include the schema in the project that wants to use > these nodes and don't include it in cases that don't use it. I don't see a reason > why this needs to be in dtschema. > > Rob
