From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f53.google.com (mail-ed1-f53.google.com [209.85.208.53]) by mx.groups.io with SMTP id smtpd.web10.64325.1680507130013092632 for ; Mon, 03 Apr 2023 00:32:10 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@starlabs-systems.20210112.gappssmtp.com header.s=20210112 header.b=68vkev9P; spf=pass (domain: starlabs.systems, ip: 209.85.208.53, mailfrom: sean@starlabs.systems) Received: by mail-ed1-f53.google.com with SMTP id ek18so113432771edb.6 for ; Mon, 03 Apr 2023 00:32:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=starlabs-systems.20210112.gappssmtp.com; s=20210112; t=1680507127; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=bQjxnbysvR4mwWyVRdWs0ijW4fZX6tqpmLPihLHFwnw=; b=68vkev9PhJZz6sBkQuXfTJ1I7jjdOOtpH8mG46//8n/mvHCPMefNFlsVX9d6z3APZX RjDmHvsE0Ebln0EaIfJ8gdztz93j1N41pIesj75I1jz8DTx21hRx1zukqQAQ0zHP/fpO 3vHwTB86AZgRs9xbayWYZa0AKJKusgOdag5XuxnqmlF4huADTstvrvHR/D4msEj8kHqb knxGcPh37Z0VEMPjPusrpWA6GXVqZ7T3Sw1IniU0Xagso5eQAQI2rxk40wKwL147IIT3 h1nQsUcb5jU3DdpRmcap3W3dbduW5wxHTt9dcCwE1HKS4NmM4PpjSaQun6A33QDzG4DR fZsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680507127; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=bQjxnbysvR4mwWyVRdWs0ijW4fZX6tqpmLPihLHFwnw=; b=hzaS0VZKqYiVXTwg5+0Tjt2JbRzPa9QLGarLqtrxKbrvM4zwJ0vYO5fY4DEAza0Yx9 DmEzmBf+6/JMWUttAEAoKTyKAJSoRM37+CK1I9XkgawqJzpqPsj3agr+h/7mhjoTKuKk sBynWMPPSM9fmuC5UQEAQByNtwin+foPHRCz/x101ipWtLP1wEX3Wa5/hgj6KeO1Tvzh a8Elnd7clNccT3Llq03WTC2n8W0o9bmN0ysiYZFPhfSK7A0tFE6dF5qe19WO/qcPsq/W YBYsr6TqBKfqE2ftAnTJbpcR9ODDmFPlXfoci+uMinvAEVG8lQ3SJzzDPV4ArkttKnRq 6D1Q== X-Gm-Message-State: AAQBX9e56W2LgtgtWBo6nvCgewONg4mmrGSoROGY0mqDAj1kCVnhZSHc j5vuqnw35JE5nZgrN6v16hEJDDug0tXHkM88T/v+ X-Google-Smtp-Source: AKy350aZ+yT1OwAraJRr7QJmOXmzP4XckaRO5PuRM5sH2SDWOvYH/ZWFrpwvvJIVZblpeLqq+cM9wkuenUDhwQooUfI= X-Received: by 2002:a17:907:720e:b0:947:46e0:9e51 with SMTP id dr14-20020a170907720e00b0094746e09e51mr6282575ejc.11.1680507127628; Mon, 03 Apr 2023 00:32:07 -0700 (PDT) MIME-Version: 1.0 References: <629e75402eb21750d9e536aac6c6b30cd346ae47.1680306925.git.benjamin.doron00@gmail.com> In-Reply-To: <629e75402eb21750d9e536aac6c6b30cd346ae47.1680306925.git.benjamin.doron00@gmail.com> From: "Sean Rhodes" Date: Mon, 3 Apr 2023 08:31:56 +0100 Message-ID: Subject: Re: [edk2-devel][PATCH v1 1/2] UefiPayloadPkg: Always build MemoryTypeInformation HOB for DXE GCD To: Benjamin Doron Cc: devel@edk2.groups.io, Guo Dong , Ray Ni , James Lu , Gua Guo Content-Type: multipart/alternative; boundary="00000000000062c24e05f8698ed6" --00000000000062c24e05f8698ed6 Content-Type: text/plain; charset="UTF-8" Reviewed-by: Sean Rhodes On Sat, 1 Apr 2023 at 00:58, Benjamin Doron wrote: > MemoryType information assists GCD with defragmenting the memory map. > When the DXE core starts, GCD adds memory descriptors for the resource > descriptors HOBs. This allocates heap space which can be reused later > as the bins by memory type. It seems memory allocation prefers low > ranges. > > It seems "below 4G" is an artifact of this heap reuse. However, the > memory type information determines the DXE core's > `MinimalMemorySizeNeeded`, determining which system memory descriptor > HOB may be used by DXE. Furthermore, it's important that the memory > type information be correct, for an S4 memory map. > > Therefore, follow other bootloaders, such as [MinPlatform][1], and do > this unconditionally. As of [edk2-stable202011][2], it was. > > [1]: > https://github.com/tianocore/edk2-platforms/blob/b6f96743891be51541184bf61dd2970c008e2c43/Platform/Intel/MinPlatformPkg/PlatformInit/PlatformInitPei/PlatformInitPreMem.c#L164-L201 > [2]: > https://github.com/tianocore/edk2/blob/edk2-stable202011/UefiPayloadPkg/BlSupportPei/BlSupportPei.c#L462-L466 > > Cc: Guo Dong > Cc: Ray Ni > Cc: Sean Rhodes > Cc: James Lu > Cc: Gua Guo > Signed-off-by: Benjamin Doron > --- > .../UefiPayloadEntry/UefiPayloadEntry.c | 36 +++++-------------- > .../UefiPayloadEntry/UefiPayloadEntry.inf | 2 -- > UefiPayloadPkg/UefiPayloadPkg.dec | 3 -- > UefiPayloadPkg/UefiPayloadPkg.dsc | 2 -- > 4 files changed, 8 insertions(+), 35 deletions(-) > > diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > index 780348eadfa8..030a5baed914 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > @@ -19,30 +19,6 @@ EFI_MEMORY_TYPE_INFORMATION > mDefaultMemoryTypeInformation[] = { > { EfiMaxMemoryType, 0 > } > }; > > -/** > - Function to reserve memory below 4GB for EDKII Modules. > - > - This causes the DXE to dispatch everything under 4GB and allows > Operating > - System's that require EFI_LOADED_IMAGE to be under 4GB to start. > - e.g. Xen hypervisor used in Qubes. > -**/ > -VOID > -ForceModulesBelow4G ( > - VOID > - ) > -{ > - DEBUG ((DEBUG_INFO, "Building hob to restrict memory resorces to below > 4G.\n")); > - > - // > - // Create Memory Type Information HOB > - // > - BuildGuidDataHob ( > - &gEfiMemoryTypeInformationGuid, > - mDefaultMemoryTypeInformation, > - sizeof (mDefaultMemoryTypeInformation) > - ); > -} > - > /** > Callback function to build resource descriptor HOB > > @@ -472,10 +448,14 @@ _ModuleEntryPoint ( > // Build other HOBs required by DXE > BuildGenericHob (); > > - // Create a HOB to make resources for EDKII modules below 4G > - if (!FixedPcdGetBool (PcdDispatchModuleAbove4GMemory)) { > - ForceModulesBelow4G (); > - } > + // > + // Create Memory Type Information HOB > + // > + BuildGuidDataHob ( > + &gEfiMemoryTypeInformationGuid, > + mDefaultMemoryTypeInformation, > + sizeof (mDefaultMemoryTypeInformation) > + ); > > // Load the DXE Core > Status = LoadDxeCore (&DxeCoreEntryPoint); > diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf > b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf > index d47e8e76cf4c..e2af8a4b7c1b 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf > +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf > @@ -96,5 +96,3 @@ > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## > SOMETIMES_CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## > SOMETIMES_CONSUMES > > - gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory > - > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec > b/UefiPayloadPkg/UefiPayloadPkg.dec > index 7d61d6eeae6c..2ed73513700d 100644 > --- a/UefiPayloadPkg/UefiPayloadPkg.dec > +++ b/UefiPayloadPkg/UefiPayloadPkg.dec > @@ -82,9 +82,6 @@ > gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000|UINT32|0x > > gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x80, > 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 > }|VOID*|0x00000018 > > -# Above 4G Memory > > -gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|TRUE|BOOLEAN|0x00000019 > - > # Boot Manager Key > > gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|FALSE|BOOLEAN|0x00000020 > > diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc > b/UefiPayloadPkg/UefiPayloadPkg.dsc > index bca5d3f335bc..9847f189fff5 100644 > --- a/UefiPayloadPkg/UefiPayloadPkg.dsc > +++ b/UefiPayloadPkg/UefiPayloadPkg.dsc > @@ -34,7 +34,6 @@ > DEFINE SECURITY_STUB_ENABLE = TRUE > DEFINE SMM_SUPPORT = FALSE > DEFINE PLATFORM_BOOT_TIMEOUT = 3 > - DEFINE ABOVE_4G_MEMORY = TRUE > DEFINE BOOT_MANAGER_ESCAPE = FALSE > DEFINE ATA_ENABLE = TRUE > DEFINE SD_ENABLE = TRUE > @@ -442,7 +441,6 @@ > !endif > > gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(SD_MMC_TIMEOUT) > > - > gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|$(ABOVE_4G_MEMORY) > > gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANAGER_ESCAPE) > > gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1800000 > -- > 2.39.1 > > --00000000000062c24e05f8698ed6 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Reviewed-by: Sean Rhodes <sean@starlabs.systems>

On Sat, 1 Apr 2023 at 00:58, Benjamin Doron <benjamin.doron00@gmail.com> wrote:
MemoryType information assis= ts GCD with defragmenting the memory map.
When the DXE core starts, GCD adds memory descriptors for the resource
descriptors HOBs. This allocates heap space which can be reused later
as the bins by memory type. It seems memory allocation prefers low
ranges.

It seems "below 4G" is an artifact of this heap reuse. However, t= he
memory type information determines the DXE core's
`MinimalMemorySizeNeeded`, determining which system memory descriptor
HOB may be used by DXE. Furthermore, it's important that the memory
type information be correct, for an S4 memory map.

Therefore, follow other bootloaders, such as [MinPlatform][1], and do
this unconditionally. As of [edk2-stable202011][2], it was.

[1]: https://github.com/tianocore/edk2-platforms/blob/b6f96743891be5154= 1184bf61dd2970c008e2c43/Platform/Intel/MinPlatformPkg/PlatformInit/Platform= InitPei/PlatformInitPreMem.c#L164-L201
[2]: https://github.com/tianocore/edk2/blob/edk2-stable202011/Uefi= PayloadPkg/BlSupportPei/BlSupportPei.c#L462-L466

Cc: Guo Dong <gu= o.dong@intel.com>
Cc: Ray Ni <ray.ni= @intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <ja= mes.lu@intel.com>
Cc: Gua Guo <gua.= guo@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
=C2=A0.../UefiPayloadEntry/UefiPayloadEntry.c=C2=A0 =C2=A0 =C2=A0 =C2=A0| 3= 6 +++++--------------
=C2=A0.../UefiPayloadEntry/UefiPayloadEntry.inf=C2=A0 =C2=A0 =C2=A0|=C2=A0 = 2 --
=C2=A0UefiPayloadPkg/UefiPayloadPkg.dec=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0|=C2=A0 3 --
=C2=A0UefiPayloadPkg/UefiPayloadPkg.dsc=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0|=C2=A0 2 --
=C2=A04 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPaylo= adPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 780348eadfa8..030a5baed914 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -19,30 +19,6 @@ EFI_MEMORY_TYPE_INFORMATION=C2=A0 mDefaultMemoryTypeInfo= rmation[] =3D {
=C2=A0 =C2=A0{ EfiMaxMemoryType,=C2=A0 =C2=A0 =C2=A0 =C2=A00=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0}
=C2=A0};

-/**
-=C2=A0 =C2=A0Function to reserve memory below 4GB for EDKII Modules.
-
-=C2=A0 =C2=A0This causes the DXE to dispatch everything under 4GB and allo= ws Operating
-=C2=A0 =C2=A0System's that require EFI_LOADED_IMAGE to be under 4GB to= start.
-=C2=A0 =C2=A0e.g. Xen hypervisor used in Qubes.
-**/
-VOID
-ForceModulesBelow4G (
-=C2=A0 VOID
-=C2=A0 )
-{
-=C2=A0 DEBUG ((DEBUG_INFO, "Building hob to restrict memory resorces = to below 4G.\n"));
-
-=C2=A0 //
-=C2=A0 // Create Memory Type Information HOB
-=C2=A0 //
-=C2=A0 BuildGuidDataHob (
-=C2=A0 =C2=A0 &gEfiMemoryTypeInformationGuid,
-=C2=A0 =C2=A0 mDefaultMemoryTypeInformation,
-=C2=A0 =C2=A0 sizeof (mDefaultMemoryTypeInformation)
-=C2=A0 =C2=A0 );
-}
-
=C2=A0/**
=C2=A0 =C2=A0 Callback function to build resource descriptor HOB

@@ -472,10 +448,14 @@ _ModuleEntryPoint (
=C2=A0 =C2=A0// Build other HOBs required by DXE
=C2=A0 =C2=A0BuildGenericHob ();

-=C2=A0 // Create a HOB to make resources for EDKII modules below 4G
-=C2=A0 if (!FixedPcdGetBool (PcdDispatchModuleAbove4GMemory)) {
-=C2=A0 =C2=A0 ForceModulesBelow4G ();
-=C2=A0 }
+=C2=A0 //
+=C2=A0 // Create Memory Type Information HOB
+=C2=A0 //
+=C2=A0 BuildGuidDataHob (
+=C2=A0 =C2=A0 &gEfiMemoryTypeInformationGuid,
+=C2=A0 =C2=A0 mDefaultMemoryTypeInformation,
+=C2=A0 =C2=A0 sizeof (mDefaultMemoryTypeInformation)
+=C2=A0 =C2=A0 );

=C2=A0 =C2=A0// Load the DXE Core
=C2=A0 =C2=A0Status =3D LoadDxeCore (&DxeCoreEntryPoint);
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf b/UefiPay= loadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
index d47e8e76cf4c..e2af8a4b7c1b 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.inf
@@ -96,5 +96,3 @@
=C2=A0 =C2=A0gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy = ## SOMETIMES_CONSUMES
=C2=A0 =C2=A0gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy=C2=A0 = =C2=A0 =C2=A0 =C2=A0## SOMETIMES_CONSUMES

-=C2=A0 gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory
-
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dec b/UefiPayloadPkg/UefiPayload= Pkg.dec
index 7d61d6eeae6c..2ed73513700d 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dec
+++ b/UefiPayloadPkg/UefiPayloadPkg.dec
@@ -82,9 +82,6 @@ gUefiPayloadPkgTokenSpaceGuid.PcdSystemMemoryUefiRegionSi= ze|0x04000000|UINT32|0x

=C2=A0gUefiPayloadPkgTokenSpaceGuid.PcdPcdDriverFile|{ 0x57, 0x72, 0xcf, 0x= 80, 0xab, 0x87, 0xf9, 0x47, 0xa3, 0xfe, 0xD5, 0x0B, 0x76, 0xd8, 0x95, 0x41 = }|VOID*|0x00000018

-# Above 4G Memory
-gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|TRUE|BOOLEAN|= 0x00000019
-
=C2=A0# Boot Manager Key
=C2=A0gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|FALSE|BOOLEAN|0x00= 000020

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayload= Pkg.dsc
index bca5d3f335bc..9847f189fff5 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -34,7 +34,6 @@
=C2=A0 =C2=A0DEFINE SECURITY_STUB_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =3D TRUE
=C2=A0 =C2=A0DEFINE SMM_SUPPORT=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =3D FALSE
=C2=A0 =C2=A0DEFINE PLATFORM_BOOT_TIMEOUT=C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D 3<= br> -=C2=A0 DEFINE ABOVE_4G_MEMORY=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =3D TRUE
=C2=A0 =C2=A0DEFINE BOOT_MANAGER_ESCAPE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =3D FALSE
=C2=A0 =C2=A0DEFINE ATA_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0=3D TRUE
=C2=A0 =C2=A0DEFINE SD_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =3D TRUE
@@ -442,7 +441,6 @@
=C2=A0!endif
=C2=A0 =C2=A0gEfiMdeModulePkgTokenSpaceGuid.PcdSdMmcGenericTimeoutValue|$(S= D_MMC_TIMEOUT)

-=C2=A0 gUefiPayloadPkgTokenSpaceGuid.PcdDispatchModuleAbove4GMemory|$(ABOV= E_4G_MEMORY)
=C2=A0 =C2=A0gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape|$(BOOT_MANA= GER_ESCAPE)

=C2=A0 =C2=A0gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1800000=
--
2.39.1

--00000000000062c24e05f8698ed6--