Re: UefiCpuPkg CpuDxe GDT init question?

["Yao, Jiewen" <jiewen.yao@intel.com>]

Understood.
Cool. Thanks for the info.

If you have some clue on the SMM, please let us know.

Thank you
Yao Jiewen

> -----Original Message-----
> From: afish@apple.com [mailto:afish@apple.com]
> Sent: Tuesday, March 12, 2019 12:30 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org; Ni, Ray
> <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2] UefiCpuPkg CpuDxe GDT init question?
> 
> 
> 
> > On Mar 11, 2019, at 9:04 AM, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > Thanks Andrew.
> > + More CPU people in our team.
> >
> > Do you want to post your patch there for reference? :-)
> >
> > BTW: Would you please share how to trigger such case at your side?
> > Did you report above 4GB memory to be tested?
> 
> Jiewen,
> 
> I'm working on a chipset that is not open source and I or'ed in
> EFI_RESOURCE_ATTRIBUTE_TESTED to the above 4GB allocation in
> BuildResourceDescriptorHob(). The DXE Core then picked this area for the
> heap, as I seem to remember the 1st loop favors the PHIT, and the 2nd loop
> favors the largest area of free memory.
> 
> > As such all drivers are loaded to above 4GB?
> 
> DXE Core is loaded low, and all the drivers loaded by DXE are in > 4GB
> memory.
> 
> > Do you also allocate BIN to above 4GB?
> >
> 
> No I just marked the above 4G memory as tested.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thank you
> > Yao Jiewen
> >
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> >> Andrew Fish via edk2-devel
> >> Sent: Monday, March 11, 2019 11:59 PM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>
> >> Cc: edk2-devel <edk2-devel@lists.01.org>; Laszlo Ersek
> <lersek@redhat.com>
> >> Subject: Re: [edk2] UefiCpuPkg CpuDxe GDT init question?
> >>
> >> Jiewen,
> >>
> >> These three fixes got me past the CpuDxe driver:
> >> https://bugzilla.tianocore.org/show_bug.cgi?id=1613
> >>
> >> Now I getting page faults loading SMM....
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>> On Mar 8, 2019, at 7:10 PM, Andrew Fish <afish@apple.com> wrote:
> >>>
> >>>
> >>>
> >>>> On Mar 8, 2019, at 7:08 AM, Laszlo Ersek <lersek@redhat.com
> >> <mailto:lersek@redhat.com>> wrote:
> >>>>
> >>>> On 03/08/19 15:13, Yao, Jiewen wrote:
> >>>>> I guess the historic reason is that AP and BSP share same GDT before.
> As
> >> such, the GDT need to be below 4G, to let AP switch from real mode to
> >> protected mode.
> >>>>> We don't get issue, because Runtime memory is in BIN, and most
> >> platform allocates BIN under 4G.
> >>>>>
> >>>>> Some thought:
> >>>>> 1) I am think we not sure if AP is using same GDT as BSP today. If yes,
> we
> >> need GDT under 4G, by using MaxAddress. If no, there should be no
> >> restriction for BSP GDT. The (UINT32) case should be removed for BSP.
> But
> >> we still AP GDT below 4G, to support wake from INIT-SIPI-SIPI.
> >>>
> >>> Jiewen,
> >>>
> >>> It looks like there are several places that assume memory is < 4 GB in the
> >> CpuDxe driver.
> >>>
> >>> I noticed SetCodeSelector () is using a far jump to change the CS at that
> is
> >> limited < 4 GB. I had to hack around it via:
> >>>  popq    %rax
> >>>  pushq   %rcx
> >>>  pushq   %rax
> >>>  lretq
> >>>
> >>> There is some other crash later on.
> >>>
> >>>
> >>>>> 2) I am not sure why we need runtime memory. Do we need touch
> GDT
> >> at UEFI runtime?
> >>>>
> >>>> I could be confusing things *very badly*, but I vaguely remember that
> >>>> APs could be woken up spuriously later, and they must be able to
> execute
> >>>> code "enough" to go back to sleep.
> >>>>
> >>>> The following commits look relevant:
> >>>>
> >>>> - 7615702169b8 ("UefiCpuPkg/MpInitLib: Add AsmRelocateApLoop()
> >> assembly
> >>>> code", 2016-08-17)
> >>>>
> >>>> - 4d3314f69488 ("UefiCpuPkg/MpInitLib: Place APs in safe loop before
> >>>> hand-off to OS", 2016-08-17)
> >>>>
> >>>> - bf2786dc7900 ("UefiCpuPkg/DxeMpLib: Allocate new safe stack <
> 4GB",
> >>>> 2016-11-28)
> >>>>
> >>>
> >>> If I'm remembering correctly there are optional idle states for the AP. I
> >> think the real mode hlt loop might have used too much power on an UP
> OS.
> >>>
> >>> It is also not clear to me that we don't need the GDT when in real mode.
> In
> >> big-real mode you go to protected mode set the GDT and then it can get
> >> used for big-real so it seems like  the GDT is in play even in real mode.
> >>>
> >>> Thanks,
> >>>
> >>> Andrew Fish
> >>>
> >>>
> >>>> Laszlo
> >>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> Thank you
> >>>>> Yao Jiewen
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org
> >> <mailto:edk2-devel-bounces@lists.01.org>] On Behalf Of
> >>>>>> Laszlo Ersek
> >>>>>> Sent: Friday, March 8, 2019 12:00 AM
> >>>>>> To: Andrew Fish <afish@apple.com <mailto:afish@apple.com>>;
> >> edk2-devel <edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>>
> >>>>>> Subject: Re: [edk2] UefiCpuPkg CpuDxe GDT init question?
> >>>>>>
> >>>>>> Hi Andrew,
> >>>>>>
> >>>>>> On 03/07/19 23:37, Andrew Fish via edk2-devel wrote:
> >>>>>>> I'm trying to understand why gdtPtr.Base is casting to (UINT32)?
> >>>>>>> 1) gdtPtr.Base is a a UINTN
> >>>>>>> 2) It is legal for AllocateRuntimePool() to return an address > 4GB
> >>>>>>>
> >>>>>>> It seems like the code should just cast to (UINTN)?
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/CpuG
> >>
> <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/Cpu
> >> G>
> >>>>>> dt.c#L151
> >>>>>>
> >>>>>> I think you are right.
> >>>>>>
> >>>>>> I'm missing the background on this too. I tried to see if any
> >>>>>> justification was given in a git commit message, but according to "git
> >>>>>> blame", this code dates back to the original addition of the driver,
> >>>>>> namely commit a47463f28382 ("Add CPU DXE driver for IA32 & X64
> >>>>>> processor
> >>>>>> architectures.", 2009-05-27). The commit message is unhelpful (for
> >> 3119
> >>>>>> lines added).
> >>>>>>
> >>>>>> Thanks
> >>>>>> Laszlo
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> VOID
> >>>>>>> InitGlobalDescriptorTable (
> >>>>>>> VOID
> >>>>>>> )
> >>>>>>> {
> >>>>>>> GDT_ENTRIES *gdt;
> >>>>>>> IA32_DESCRIPTOR gdtPtr;
> >>>>>>>
> >>>>>>> //
> >>>>>>> // Allocate Runtime Data for the GDT
> >>>>>>> //
> >>>>>>> gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);
> >>>>>>> ASSERT (gdt != NULL);
> >>>>>>> gdt = ALIGN_POINTER (gdt, 8);
> >>>>>>>
> >>>>>>> //
> >>>>>>> // Initialize all GDT entries
> >>>>>>> //
> >>>>>>> CopyMem (gdt, &GdtTemplate, sizeof (GdtTemplate));
> >>>>>>>
> >>>>>>> //
> >>>>>>> // Write GDT register
> >>>>>>> //
> >>>>>>> gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;
> >>>>>>> gdtPtr.Limit = (UINT16) (sizeof (GdtTemplate) - 1);
> >>>>>>> AsmWriteGdtr (&gdtPtr);
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>> Andrew Fish
> >>>>>>> _______________________________________________
> >>>>>>> edk2-devel mailing list
> >>>>>>> edk2-devel@lists.01.org
> >>>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>>>>>>
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> edk2-devel mailing list
> >>>>>> edk2-devel@lists.01.org
> >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> >>
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel