Re: [edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing 5-level page table

2019-07-25 Thread Laszlo Ersek
On 07/24/19 09:03, Ni, Ray wrote:
> 
> 
>> -Original Message-
>> From: Laszlo Ersek 
>> Sent: Tuesday, July 23, 2019 5:23 PM
>> To: devel@edk2.groups.io; Ni, Ray 
>> Cc: Dong, Eric 
>> Subject: Re: [edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing
>> 5-level page table
>>
>> On 07/22/19 10:15, Ni, Ray wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
>>>
>>> Signed-off-by: Ray Ni 
>>> Cc: Eric Dong 
>>> Cc: Laszlo Ersek 
>>> ---
>>>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 22 --
>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> index c369b44f12..8e959eb2b7 100644
>>> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> @@ -1,7 +1,7 @@
>>>  /** @file
>>>Page table management support.
>>>
>>> -  Copyright (c) 2017, Intel Corporation. All rights reserved.
>>> +  Copyright (c) 2017 - 2019, Intel Corporation. All rights
>>> + reserved.
>>>Copyright (c) 2017, AMD Incorporated. All rights reserved.
>>>
>>>SPDX-License-Identifier: BSD-2-Clause-Patent @@ -276,25 +276,43 @@
>>> GetPageTableEntry (
>>>UINTN Index2;
>>>UINTN Index3;
>>>UINTN Index4;
>>> +  UINTN Index5;
>>>UINT64*L1PageTable;
>>>UINT64*L2PageTable;
>>>UINT64*L3PageTable;
>>>UINT64*L4PageTable;
>>> +  UINT64*L5PageTable;
>>>UINT64AddressEncMask;
>>> +  IA32_CR4  Cr4;
>>> +  BOOLEAN   Enable5LevelPaging;
>>>
>>>ASSERT (PagingContext != NULL);
>>>
>>> +  Index5 = ((UINTN)RShiftU64 (Address, 48)) &
>> PAGING_PAE_INDEX_MASK;
>>>Index4 = ((UINTN)RShiftU64 (Address, 39)) &
>> PAGING_PAE_INDEX_MASK;
>>>Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
>>>Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
>>>Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
>>>
>>> +  Cr4.UintN = AsmReadCr4 ();
>>> +  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
>>> +
>>>// Make sure AddressEncMask is contained to smallest supported address
>> field.
>>>//
>>>AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask)
>> &
>>> PAGING_1G_ADDRESS_MASK_64;
>>>
>>>if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
>>> -L4PageTable = (UINT64 *)(UINTN)PagingContext-
>>> ContextData.X64.PageTableBase;
>>> +if (Enable5LevelPaging) {
>>> +  L5PageTable = (UINT64 *)(UINTN)PagingContext-
>>> ContextData.X64.PageTableBase;
>>> +  if (L5PageTable[Index5] == 0) {
>>> +*PageAttribute = PageNone;
>>> +return NULL;
>>> +  }
>>> +
>>> +  L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] &
>> ~AddressEncMask & PAGING_4K_ADDRESS_MASK_64);
>>> +} else {
>>> +  L4PageTable = (UINT64 *)(UINTN)PagingContext-
>>> ContextData.X64.PageTableBase;
>>> +}
>>>  if (L4PageTable[Index4] == 0) {
>>>*PageAttribute = PageNone;
>>>return NULL;
>>>
>>
>> The patch seems to be a no-op if Enable5LevelPaging is FALSE, so that's good.
>>
>> Questions:
>>
>> (1) Same question as under patch #1: is the CR4 check reliable on AMD
>> processors too?
> 
> Replied in 1/4 thread.
> 
>>
>> (2) Should we read CR4 (or call CPUID) every time this function is invoked?
>> Can we perform the check at CpuDxe startup, and cache the result?
> It's a good suggestion. Will address it in V2.
> 
> 
>>
>> (3) Should we consider the PCD (from patch #3) before accessing the
>> hardware (CR4 / CPUID alike)?
> I want to avoid touching PCD in this CPU driver.
> All the information is taken in DxeIpl and pass to CPU driver through 
> Cr4.LA57.

Makes sense, thanks.
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44402): https://edk2.groups.io/g/devel/message/44402
Mute This Topic: https://groups.io/mt/32556533/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing 5-level page table

2019-07-24 Thread Ni, Ray


> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, July 23, 2019 5:23 PM
> To: devel@edk2.groups.io; Ni, Ray 
> Cc: Dong, Eric 
> Subject: Re: [edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing
> 5-level page table
> 
> On 07/22/19 10:15, Ni, Ray wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> >
> > Signed-off-by: Ray Ni 
> > Cc: Eric Dong 
> > Cc: Laszlo Ersek 
> > ---
> >  UefiCpuPkg/CpuDxe/CpuPageTable.c | 22 --
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > index c369b44f12..8e959eb2b7 100644
> > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >Page table management support.
> >
> > -  Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2017 - 2019, Intel Corporation. All rights
> > + reserved.
> >Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >
> >SPDX-License-Identifier: BSD-2-Clause-Patent @@ -276,25 +276,43 @@
> > GetPageTableEntry (
> >UINTN Index2;
> >UINTN Index3;
> >UINTN Index4;
> > +  UINTN Index5;
> >UINT64*L1PageTable;
> >UINT64*L2PageTable;
> >UINT64*L3PageTable;
> >UINT64*L4PageTable;
> > +  UINT64*L5PageTable;
> >UINT64AddressEncMask;
> > +  IA32_CR4  Cr4;
> > +  BOOLEAN   Enable5LevelPaging;
> >
> >ASSERT (PagingContext != NULL);
> >
> > +  Index5 = ((UINTN)RShiftU64 (Address, 48)) &
> PAGING_PAE_INDEX_MASK;
> >Index4 = ((UINTN)RShiftU64 (Address, 39)) &
> PAGING_PAE_INDEX_MASK;
> >Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
> >Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
> >Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
> >
> > +  Cr4.UintN = AsmReadCr4 ();
> > +  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
> > +
> >// Make sure AddressEncMask is contained to smallest supported address
> field.
> >//
> >AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask)
> &
> > PAGING_1G_ADDRESS_MASK_64;
> >
> >if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
> > -L4PageTable = (UINT64 *)(UINTN)PagingContext-
> >ContextData.X64.PageTableBase;
> > +if (Enable5LevelPaging) {
> > +  L5PageTable = (UINT64 *)(UINTN)PagingContext-
> >ContextData.X64.PageTableBase;
> > +  if (L5PageTable[Index5] == 0) {
> > +*PageAttribute = PageNone;
> > +return NULL;
> > +  }
> > +
> > +  L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] &
> ~AddressEncMask & PAGING_4K_ADDRESS_MASK_64);
> > +} else {
> > +  L4PageTable = (UINT64 *)(UINTN)PagingContext-
> >ContextData.X64.PageTableBase;
> > +}
> >  if (L4PageTable[Index4] == 0) {
> >*PageAttribute = PageNone;
> >return NULL;
> >
> 
> The patch seems to be a no-op if Enable5LevelPaging is FALSE, so that's good.
> 
> Questions:
> 
> (1) Same question as under patch #1: is the CR4 check reliable on AMD
> processors too?

Replied in 1/4 thread.

> 
> (2) Should we read CR4 (or call CPUID) every time this function is invoked?
> Can we perform the check at CpuDxe startup, and cache the result?
It's a good suggestion. Will address it in V2.


> 
> (3) Should we consider the PCD (from patch #3) before accessing the
> hardware (CR4 / CPUID alike)?
I want to avoid touching PCD in this CPU driver.
All the information is taken in DxeIpl and pass to CPU driver through Cr4.LA57.

> 
> Thanks
> Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44288): https://edk2.groups.io/g/devel/message/44288
Mute This Topic: https://groups.io/mt/32556533/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



Re: [edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing 5-level page table

2019-07-23 Thread Laszlo Ersek
On 07/22/19 10:15, Ni, Ray wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> 
> Signed-off-by: Ray Ni 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 22 --
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c 
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index c369b44f12..8e959eb2b7 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -1,7 +1,7 @@
>  /** @file
>Page table management support.
>  
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
>Copyright (c) 2017, AMD Incorporated. All rights reserved.
>  
>SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -276,25 +276,43 @@ GetPageTableEntry (
>UINTN Index2;
>UINTN Index3;
>UINTN Index4;
> +  UINTN Index5;
>UINT64*L1PageTable;
>UINT64*L2PageTable;
>UINT64*L3PageTable;
>UINT64*L4PageTable;
> +  UINT64*L5PageTable;
>UINT64AddressEncMask;
> +  IA32_CR4  Cr4;
> +  BOOLEAN   Enable5LevelPaging;
>  
>ASSERT (PagingContext != NULL);
>  
> +  Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK;
>Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
>Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
>Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
>Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
>  
> +  Cr4.UintN = AsmReadCr4 ();
> +  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
> +
>// Make sure AddressEncMask is contained to smallest supported address 
> field.
>//
>AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) & 
> PAGING_1G_ADDRESS_MASK_64;
>  
>if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
> -L4PageTable = (UINT64 
> *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
> +if (Enable5LevelPaging) {
> +  L5PageTable = (UINT64 
> *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
> +  if (L5PageTable[Index5] == 0) {
> +*PageAttribute = PageNone;
> +return NULL;
> +  }
> +
> +  L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~AddressEncMask 
> & PAGING_4K_ADDRESS_MASK_64);
> +} else {
> +  L4PageTable = (UINT64 
> *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
> +}
>  if (L4PageTable[Index4] == 0) {
>*PageAttribute = PageNone;
>return NULL;
> 

The patch seems to be a no-op if Enable5LevelPaging is FALSE, so that's
good.

Questions:

(1) Same question as under patch #1: is the CR4 check reliable on AMD
processors too?

(2) Should we read CR4 (or call CPUID) every time this function is
invoked? Can we perform the check at CpuDxe startup, and cache the result?

(3) Should we consider the PCD (from patch #3) before accessing the
hardware (CR4 / CPUID alike)?

Thanks
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44235): https://edk2.groups.io/g/devel/message/44235
Mute This Topic: https://groups.io/mt/32556533/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



[edk2-devel] [PATCH 2/4] UefiCpuPkg/CpuDxe: Support parsing 5-level page table

2019-07-22 Thread Ni, Ray
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008

Signed-off-by: Ray Ni 
Cc: Eric Dong 
Cc: Laszlo Ersek 
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index c369b44f12..8e959eb2b7 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -1,7 +1,7 @@
 /** @file
   Page table management support.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
   Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -276,25 +276,43 @@ GetPageTableEntry (
   UINTN Index2;
   UINTN Index3;
   UINTN Index4;
+  UINTN Index5;
   UINT64*L1PageTable;
   UINT64*L2PageTable;
   UINT64*L3PageTable;
   UINT64*L4PageTable;
+  UINT64*L5PageTable;
   UINT64AddressEncMask;
+  IA32_CR4  Cr4;
+  BOOLEAN   Enable5LevelPaging;
 
   ASSERT (PagingContext != NULL);
 
+  Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK;
   Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
   Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
   Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
   Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
 
+  Cr4.UintN = AsmReadCr4 ();
+  Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
+
   // Make sure AddressEncMask is contained to smallest supported address field.
   //
   AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) & 
PAGING_1G_ADDRESS_MASK_64;
 
   if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
-L4PageTable = (UINT64 
*)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+if (Enable5LevelPaging) {
+  L5PageTable = (UINT64 
*)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+  if (L5PageTable[Index5] == 0) {
+*PageAttribute = PageNone;
+return NULL;
+  }
+
+  L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~AddressEncMask & 
PAGING_4K_ADDRESS_MASK_64);
+} else {
+  L4PageTable = (UINT64 
*)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+}
 if (L4PageTable[Index4] == 0) {
   *PageAttribute = PageNone;
   return NULL;
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44116): https://edk2.groups.io/g/devel/message/44116
Mute This Topic: https://groups.io/mt/32556533/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-