Re: [PATCH] UefiPayloadPkg/UefiPayloadEntry: Remove 4GB memory WA


Patrick Rudolph
 

Hi Guo,
I tested it and it works fine with your patch. The crash is gone.

BTW: Are there are other possible side effects with placing memory
above 4GB? Like 32bit UEFI drivers or Option ROMs malfunctioning?

Regards,
Patrick

On Mon, Feb 22, 2021 at 6:10 PM Dong, Guo <guo.dong@intel.com> wrote:


Hi Patrick,
Thank you for the test and root-cause the issues.
Could you try if this patch works with "gdtPtr.Base = (UINTN)(VOID*) gdt;"?
I didn't see this issue when I tested SBL with UEFI payload on QEMU and a real platform.
I guess there is something that could make AllocateRuntimePool to allocate memory below 4GB while CpuDxe is dispatched above 4GB.

Thanks,
Guo

-----Original Message-----
From: Ma, Maurice <maurice.ma@intel.com>
Sent: Monday, February 22, 2021 9:50 AM
To: Patrick Rudolph <patrick.rudolph@9elements.com>; Dong, Guo
<guo.dong@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
<ray.ni@intel.com>
Cc: devel@edk2.groups.io; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry: Remove
4GB memory WA

Hi, Ray and Eric,

Is there any reason why the GDT base was typecast to UINT32 in CpuDxe driver ?
In x64 long mode, the GDT base is actually 64bit. Typecasting will zero out the
high 32bit address.
To me the correct code seems to be something like:
gdtPtr.Base = (UINTN)(VOID*) gdt;

Thanks
Maurice
-----Original Message-----
From: Patrick Rudolph <patrick.rudolph@9elements.com>
Sent: Monday, February 22, 2021 7:43
To: Dong, Guo <guo.dong@intel.com>
Cc: devel@edk2.groups.io; Ma, Maurice <maurice.ma@intel.com>; You,
Benjamin <benjamin.you@intel.com>
Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry:
Remove 4GB memory WA

Hi Guo,
I tested on 078400ee15e7b250e4dfafd840c2e0c19835e16b and run it in
QEMU.
The problem seems to be here, as gdt is allocated > 4GiB:
gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;

Regards,
Patrick

On Mon, Feb 22, 2021 at 3:59 PM Dong, Guo <guo.dong@intel.com> wrote:


Hi Patrick,
Please make sure you are using latest master when testing this patch.
That issue should be fix be this patch:
UefiCpuPkg/CpuDxe: Fix boot error (commit:
ebfe2d3eb5ac7fd92d74011edb31303a181920c7)
And there is similar fix in another place as below:
UefiCpuPkg/MpInitLib: Fix a hang in above 4GB case (commit:
edd74ad3ad79b855f76d9cf60a96c405cb3e863b)

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Patrick Rudolph
Sent: Monday, February 22, 2021 7:04 AM
To: devel@edk2.groups.io; Ma, Maurice <maurice.ma@intel.com>
Cc: Dong, Guo <guo.dong@intel.com>; You, Benjamin
<benjamin.you@intel.com>
Subject: Re: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry:
Remove 4GB memory WA

This patch breaks booting on master.
In CpuDxe.efi / InitGlobalDescriptorTable as the GDT pointer is
casted to 32bits.

Regards,
Patrick

On Fri, Feb 19, 2021 at 3:12 AM Ma, Maurice <maurice.ma@intel.com>
wrote:

Reviewed-by: Maurice Ma <maurice.ma@intel.com>

Regards
Maurice

-----Original Message-----
From: Dong, Guo <guo.dong@intel.com>
Sent: Sunday, February 14, 2021 21:13
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin
<benjamin.you@intel.com>
Subject: [edk2-devel] [PATCH] UefiPayloadPkg/UefiPayloadEntry:
Remove 4GB memory WA

Previous it would hang in CpuDxe if DXE drivers are dispatched above
4GB.
Now remove the work around since the fixed in CpuDxe are merged.

Signed-off-by: Guo Dong <guo.dong@intel.com>
---
UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index 805f5448d9..c403b0a80a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -40,11 +40,6 @@ MemInfoCallback (
EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE;

- if (Base >= BASE_4GB ) {
- // Remove tested attribute to avoid DXE core to dispatch driver to
memory above 4GB
- Attribue &= ~EFI_RESOURCE_ATTRIBUTE_TESTED;
- }
-
BuildResourceDescriptorHob (Type, Attribue,
(EFI_PHYSICAL_ADDRESS)Base, Size);
DEBUG ((DEBUG_INFO , "buildhob: base = 0x%lx, size = 0x%lx,
type = 0x%x\n", Base, Size, Type));

--
2.16.2.windows.1






Join devel@edk2.groups.io to automatically receive all group messages.