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

["Patrick Rudolph" <patrick.rudolph@9elements.com>]

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