* UefiCpuPkg CpuDxe GDT init question?
@ 2019-03-07 22:37 Andrew Fish
2019-03-08 5:02 ` Andrew Fish
2019-03-08 7:59 ` Laszlo Ersek
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Fish @ 2019-03-07 22:37 UTC (permalink / raw)
To: edk2-devel
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/CpuGdt.c#L151
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-07 22:37 UefiCpuPkg CpuDxe GDT init question? Andrew Fish
@ 2019-03-08 5:02 ` Andrew Fish
2019-03-08 7:59 ` Laszlo Ersek
1 sibling, 0 replies; 10+ messages in thread
From: Andrew Fish @ 2019-03-08 5:02 UTC (permalink / raw)
To: edk2-devel
Actually it looks like the the CpuDxe driver is coded to only run if it it is loaded under 4 GB? Is that following the spec? Is that intentional?
I noticed that SetCodeSelector is coded to use a far jump and that is a 32-bit absolute value? Note [rsp+4]
https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/X64/CpuAsm.nasm#L28
ASM_PFX(SetCodeSelector):
sub rsp, 0x10
lea rax, [setCodeSelectorLongJump]
mov [rsp], rax
mov [rsp+4], cx
jmp dword far [rsp]
setCodeSelectorLongJump:
add rsp, 0x10
ret
Thanks,
Andrew Fish
> On Mar 7, 2019, at 2:37 PM, Andrew Fish <afish@apple.com> 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/CpuGdt.c#L151 <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuDxe/CpuGdt.c#L151>
>
>
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-07 22:37 UefiCpuPkg CpuDxe GDT init question? Andrew Fish
2019-03-08 5:02 ` Andrew Fish
@ 2019-03-08 7:59 ` Laszlo Ersek
2019-03-08 14:13 ` Yao, Jiewen
1 sibling, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2019-03-08 7:59 UTC (permalink / raw)
To: Andrew Fish, edk2-devel
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/CpuGdt.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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-08 7:59 ` Laszlo Ersek
@ 2019-03-08 14:13 ` Yao, Jiewen
2019-03-08 15:08 ` Laszlo Ersek
0 siblings, 1 reply; 10+ messages in thread
From: Yao, Jiewen @ 2019-03-08 14:13 UTC (permalink / raw)
To: Laszlo Ersek, Andrew Fish, edk2-devel
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.
2) I am not sure why we need runtime memory. Do we need touch GDT at UEFI runtime?
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [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>; edk2-devel <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
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-08 14:13 ` Yao, Jiewen
@ 2019-03-08 15:08 ` Laszlo Ersek
2019-03-09 3:10 ` Andrew Fish
0 siblings, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2019-03-08 15:08 UTC (permalink / raw)
To: Yao, Jiewen, Andrew Fish, edk2-devel
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.
> 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)
Laszlo
>
>
>
> Thank you
> Yao Jiewen
>
>
>> -----Original Message-----
>> From: edk2-devel [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>; edk2-devel <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
>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-08 15:08 ` Laszlo Ersek
@ 2019-03-09 3:10 ` Andrew Fish
2019-03-11 15:59 ` Andrew Fish
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-03-09 3:10 UTC (permalink / raw)
To: Yao, Jiewen; +Cc: edk2-devel, Laszlo Ersek
> On Mar 8, 2019, at 7:08 AM, Laszlo Ersek <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] On Behalf Of
>>> Laszlo Ersek
>>> Sent: Friday, March 8, 2019 12:00 AM
>>> To: Andrew Fish <afish@apple.com>; edk2-devel <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
>>> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-09 3:10 ` Andrew Fish
@ 2019-03-11 15:59 ` Andrew Fish
2019-03-11 16:04 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-03-11 15:59 UTC (permalink / raw)
To: Yao, Jiewen; +Cc: edk2-devel, Laszlo Ersek
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/CpuG>
>>>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-11 15:59 ` Andrew Fish
@ 2019-03-11 16:04 ` Yao, Jiewen
2019-03-11 16:30 ` Andrew Fish
0 siblings, 1 reply; 10+ messages in thread
From: Yao, Jiewen @ 2019-03-11 16:04 UTC (permalink / raw)
To: Andrew Fish; +Cc: Laszlo Ersek, edk2-devel@lists.01.org, Ni, Ray, Dong, Eric
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? As such all drivers are loaded to above 4GB?
Do you also allocate BIN to above 4GB?
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-11 16:04 ` Yao, Jiewen
@ 2019-03-11 16:30 ` Andrew Fish
2019-03-11 21:04 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-03-11 16:30 UTC (permalink / raw)
To: Yao, Jiewen; +Cc: Laszlo Ersek, edk2-devel@lists.01.org, Ni, Ray, Dong, Eric
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: UefiCpuPkg CpuDxe GDT init question?
2019-03-11 16:30 ` Andrew Fish
@ 2019-03-11 21:04 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2019-03-11 21:04 UTC (permalink / raw)
To: afish@apple.com
Cc: Laszlo Ersek, edk2-devel@lists.01.org, Ni, Ray, Dong, Eric,
Yao, Jiewen
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-03-11 21:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-07 22:37 UefiCpuPkg CpuDxe GDT init question? Andrew Fish
2019-03-08 5:02 ` Andrew Fish
2019-03-08 7:59 ` Laszlo Ersek
2019-03-08 14:13 ` Yao, Jiewen
2019-03-08 15:08 ` Laszlo Ersek
2019-03-09 3:10 ` Andrew Fish
2019-03-11 15:59 ` Andrew Fish
2019-03-11 16:04 ` Yao, Jiewen
2019-03-11 16:30 ` Andrew Fish
2019-03-11 21:04 ` Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox