Message-ID: <BN9PR11MB54837EEB391CC2A8FA6C0BF5E695A@BN9PR11MB5483.namprd11.prod.outlook.com>
Date: Thu, 21 Dec 2023 16:50:44 +0000
From: "Chiu, Chasel" <chasel.chiu@...el.com>
To: Ard Biesheuvel <ardb@...nel.org>
CC: Simon Glass <sjg@...omium.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, Mark Rutland <mark.rutland@....com>, "Rob
 Herring" <robh@...nel.org>, "Tan, Lean Sheng" <sheng.tan@...ements.com>, lkml
	<linux-kernel@...r.kernel.org>, Dhaval Sharma <dhaval@...osinc.com>, "Brune,
 Maximilian" <maximilian.brune@...ements.com>, Yunhui Cui
	<cuiyunhui@...edance.com>, "Dong, Guo" <guo.dong@...el.com>, Tom Rini
	<trini@...sulko.com>, ron minnich <rminnich@...il.com>, "Guo, Gua"
	<gua.guo@...el.com>, "linux-acpi@...r.kernel.org"
	<linux-acpi@...r.kernel.org>, U-Boot Mailing List <u-boot@...ts.denx.de>
Subject: RE: [PATCH v7 2/2] schemas: Add some common reserved-memory usages


Hi Ard,

Please see my reply below inline and let me know your thoughts.

Thanks,
Chasel


> -----Original Message-----
> From: Ard Biesheuvel <ardb@...nel.org>
> Sent: Thursday, December 21, 2023 6:31 AM
> To: Chiu, Chasel <chasel.chiu@...el.com>
> Cc: Simon Glass <sjg@...omium.org>; devicetree@...r.kernel.org; Mark Rutland
> <mark.rutland@....com>; Rob Herring <robh@...nel.org>; Tan, Lean Sheng
> <sheng.tan@...ements.com>; lkml <linux-kernel@...r.kernel.org>; Dhaval
> Sharma <dhaval@...osinc.com>; Brune, Maximilian
> <maximilian.brune@...ements.com>; Yunhui Cui <cuiyunhui@...edance.com>;
> Dong, Guo <guo.dong@...el.com>; Tom Rini <trini@...sulko.com>; ron minnich
> <rminnich@...il.com>; Guo, Gua <gua.guo@...el.com>; linux-
> acpi@...r.kernel.org; U-Boot Mailing List <u-boot@...ts.denx.de>
> 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@...el.com> wrote:
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb@...nel.org>
> > > Sent: Tuesday, November 28, 2023 10:08 AM
> > > To: Chiu, Chasel <chasel.chiu@...el.com>
> > > Cc: Simon Glass <sjg@...omium.org>; devicetree@...r.kernel.org; Mark
> > > Rutland <mark.rutland@....com>; Rob Herring <robh@...nel.org>; Tan,
> > > Lean Sheng <sheng.tan@...ements.com>; lkml
> > > <linux-kernel@...r.kernel.org>; Dhaval Sharma <dhaval@...osinc.com>;
> > > Brune, Maximilian <maximilian.brune@...ements.com>; Yunhui Cui
> > > <cuiyunhui@...edance.com>; Dong, Guo <guo.dong@...el.com>; Tom Rini
> > > <trini@...sulko.com>; ron minnich <rminnich@...il.com>; Guo, Gua
> > > <gua.guo@...el.com>; linux- acpi@...r.kernel.org; U-Boot Mailing
> > > List <u-boot@...ts.denx.de>
> > > 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@...el.com> 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 ...


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.

      - 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.

Powered by blists - more mailing lists