public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
       [not found] <17517763F19F09A0.27612@groups.io>
@ 2023-03-31  9:41 ` duntan
  2023-03-31 14:35   ` Lendacky, Thomas
  0 siblings, 1 reply; 5+ messages in thread
From: duntan @ 2023-03-31  9:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, Tom Lendacky; +Cc: Ni, Ray

Hi Tom,

Reccentlly I sent this patch set to change DxeIpl code to use CpuPageTableLib to create page table. I have done some test on Intel CPU to make sure that the page table created by DxeIpl before the change is the same as the page table created by DxeIpl after the change. But there was a remaining case that I didn't cover. The case is that PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and PcdGhcbSize are not zero(when memory encryption is enabled on AMD processors supporting the SEV feature). 
So could you please help do a test on AMD processor to make sure that the SEV feature still works good with this pacth set?

Thanks, 
Dun

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
Sent: Friday, March 31, 2023 5:34 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl

In this V2 patch set:
1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create page table by CpuPageTableLib', change the input parameter name from Is32BitPageTable to Is64BitPageTable and add a line of "MapAttribute.Bits.Present = 0" before set a range to non-present.
3.In this patch 'Refinement to the code to set PageTable as RO', add a line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.

Dun Tan (8):
  EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
  IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
  MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
  OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
  MdeModulePkg: Add UefiCpuPkg.dec to pass DependencyCheck
  MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
  MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
  MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO

 EmulatorPkg/EmulatorPkg.dsc                      |   3 ++-
 IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc          |   3 ++-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            |   3 ++-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |   4 +++-
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 112 ++++------------------------------------------------------------------------------------------------------------
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |   5 +++--
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 ++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 MdeModulePkg/MdeModulePkg.ci.yaml                |   5 +++--
 MdeModulePkg/MdeModulePkg.dsc                    |   3 ++-
 OvmfPkg/AmdSev/AmdSevX64.dsc                     |   2 +-
 OvmfPkg/Bhyve/BhyveX64.dsc                       |   3 ++-
 OvmfPkg/CloudHv/CloudHvX64.dsc                   |   2 +-
 OvmfPkg/Microvm/MicrovmX64.dsc                   |   2 +-
 OvmfPkg/OvmfPkgIa32.dsc                          |   3 ++-
 OvmfPkg/OvmfPkgIa32X64.dsc                       |   2 +-
 OvmfPkg/OvmfPkgX64.dsc                           |   2 +-
 OvmfPkg/OvmfXen.dsc                              |   2 +-
 18 files changed, 200 insertions(+), 849 deletions(-)

--
2.31.1.windows.1







^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
  2023-03-31  9:41 ` [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl duntan
@ 2023-03-31 14:35   ` Lendacky, Thomas
  2023-04-03 14:24     ` Lendacky, Thomas
  0 siblings, 1 reply; 5+ messages in thread
From: Lendacky, Thomas @ 2023-03-31 14:35 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Ni, Ray

On 3/31/23 04:41, Tan, Dun wrote:
> Hi Tom,
> 
> Reccentlly I sent this patch set to change DxeIpl code to use CpuPageTableLib to create page table. I have done some test on Intel CPU to make sure that the page table created by DxeIpl before the change is the same as the page table created by DxeIpl after the change. But there was a remaining case that I didn't cover. The case is that PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and PcdGhcbSize are not zero(when memory encryption is enabled on AMD processors supporting the SEV feature).
> So could you please help do a test on AMD processor to make sure that the SEV feature still works good with this pacth set?

Yes, I can test it.

Thanks,
Tom

> 
> Thanks,
> Dun
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Friday, March 31, 2023 5:34 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
> 
> In this V2 patch set:
> 1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create page table by CpuPageTableLib', change the input parameter name from Is32BitPageTable to Is64BitPageTable and add a line of "MapAttribute.Bits.Present = 0" before set a range to non-present.
> 3.In this patch 'Refinement to the code to set PageTable as RO', add a line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.
> 
> Dun Tan (8):
>    EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
>    IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
>    MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
>    OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
>    MdeModulePkg: Add UefiCpuPkg.dec to pass DependencyCheck
>    MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
>    MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
>    MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO
> 
>   EmulatorPkg/EmulatorPkg.dsc                      |   3 ++-
>   IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc          |   3 ++-
>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            |   3 ++-
>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |   4 +++-
>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 112 ++++------------------------------------------------------------------------------------------------------------
>   MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |   5 +++--
>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 ++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>   MdeModulePkg/MdeModulePkg.ci.yaml                |   5 +++--
>   MdeModulePkg/MdeModulePkg.dsc                    |   3 ++-
>   OvmfPkg/AmdSev/AmdSevX64.dsc                     |   2 +-
>   OvmfPkg/Bhyve/BhyveX64.dsc                       |   3 ++-
>   OvmfPkg/CloudHv/CloudHvX64.dsc                   |   2 +-
>   OvmfPkg/Microvm/MicrovmX64.dsc                   |   2 +-
>   OvmfPkg/OvmfPkgIa32.dsc                          |   3 ++-
>   OvmfPkg/OvmfPkgIa32X64.dsc                       |   2 +-
>   OvmfPkg/OvmfPkgX64.dsc                           |   2 +-
>   OvmfPkg/OvmfXen.dsc                              |   2 +-
>   18 files changed, 200 insertions(+), 849 deletions(-)
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
  2023-03-31 14:35   ` Lendacky, Thomas
@ 2023-04-03 14:24     ` Lendacky, Thomas
  2023-04-03 17:14       ` Lendacky, Thomas
  0 siblings, 1 reply; 5+ messages in thread
From: Lendacky, Thomas @ 2023-04-03 14:24 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Ni, Ray

On 3/31/23 09:35, Tom Lendacky wrote:
> On 3/31/23 04:41, Tan, Dun wrote:
>> Hi Tom,
>>
>> Reccentlly I sent this patch set to change DxeIpl code to use 
>> CpuPageTableLib to create page table. I have done some test on Intel CPU 
>> to make sure that the page table created by DxeIpl before the change is 
>> the same as the page table created by DxeIpl after the change. But there 
>> was a remaining case that I didn't cover. The case is that 
>> PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and PcdGhcbSize are not 
>> zero(when memory encryption is enabled on AMD processors supporting the 
>> SEV feature).
>> So could you please help do a test on AMD processor to make sure that 
>> the SEV feature still works good with this pacth set?
> 
> Yes, I can test it.

This is breaking the SEV-ES and SEV-SNP boots. I'll see if I can figure 
out what or where the breakage is, but this patchset can't be merged as is.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>> Thanks,
>> Dun
>>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
>> Sent: Friday, March 31, 2023 5:34 PM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] [Patch V2 0/8] Create page table by 
>> CpuPageTableLib in DxeIpl
>>
>> In this V2 patch set:
>> 1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create page 
>> table by CpuPageTableLib', change the input parameter name from 
>> Is32BitPageTable to Is64BitPageTable and add a line of 
>> "MapAttribute.Bits.Present = 0" before set a range to non-present.
>> 3.In this patch 'Refinement to the code to set PageTable as RO', add a 
>> line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.
>>
>> Dun Tan (8):
>>    EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
>>    IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
>>    MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
>>    OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
>>    MdeModulePkg: Add UefiCpuPkg.dec to pass DependencyCheck
>>    MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
>>    MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
>>    MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO
>>
>>   EmulatorPkg/EmulatorPkg.dsc                      |   3 ++-
>>   IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc          |   3 ++-
>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            |   3 ++-
>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |   4 +++-
>>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 112 
>> ++++------------------------------------------------------------------------------------------------------------
>>   MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |   5 +++--
>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711 
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 
>> ++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>   MdeModulePkg/MdeModulePkg.ci.yaml                |   5 +++--
>>   MdeModulePkg/MdeModulePkg.dsc                    |   3 ++-
>>   OvmfPkg/AmdSev/AmdSevX64.dsc                     |   2 +-
>>   OvmfPkg/Bhyve/BhyveX64.dsc                       |   3 ++-
>>   OvmfPkg/CloudHv/CloudHvX64.dsc                   |   2 +-
>>   OvmfPkg/Microvm/MicrovmX64.dsc                   |   2 +-
>>   OvmfPkg/OvmfPkgIa32.dsc                          |   3 ++-
>>   OvmfPkg/OvmfPkgIa32X64.dsc                       |   2 +-
>>   OvmfPkg/OvmfPkgX64.dsc                           |   2 +-
>>   OvmfPkg/OvmfXen.dsc                              |   2 +-
>>   18 files changed, 200 insertions(+), 849 deletions(-)
>>
>> -- 
>> 2.31.1.windows.1
>>
>>
>>
>> 
>>
>>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
  2023-04-03 14:24     ` Lendacky, Thomas
@ 2023-04-03 17:14       ` Lendacky, Thomas
  2023-04-04  1:23         ` duntan
  0 siblings, 1 reply; 5+ messages in thread
From: Lendacky, Thomas @ 2023-04-03 17:14 UTC (permalink / raw)
  To: Tan, Dun, devel@edk2.groups.io; +Cc: Ni, Ray

On 4/3/23 09:24, Tom Lendacky wrote:
> On 3/31/23 09:35, Tom Lendacky wrote:
>> On 3/31/23 04:41, Tan, Dun wrote:
>>> Hi Tom,
>>>
>>> Reccentlly I sent this patch set to change DxeIpl code to use 
>>> CpuPageTableLib to create page table. I have done some test on Intel 
>>> CPU to make sure that the page table created by DxeIpl before the 
>>> change is the same as the page table created by DxeIpl after the 
>>> change. But there was a remaining case that I didn't cover. The case is 
>>> that PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and PcdGhcbSize 
>>> are not zero(when memory encryption is enabled on AMD processors 
>>> supporting the SEV feature).
>>> So could you please help do a test on AMD processor to make sure that 
>>> the SEV feature still works good with this pacth set?
>>
>> Yes, I can test it.
> 
> This is breaking the SEV-ES and SEV-SNP boots. I'll see if I can figure 
> out what or where the breakage is, but this patchset can't be merged as is.

The following change to the patch series allows SEV-ES and SEV-SNP guests
to boot.

Thanks,
Tom


diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index a9edf4de32..a3f16c7cf9 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -416,6 +416,7 @@ CreateIdentityMappingPageTables (
    IA32_MAP_ATTRIBUTE                           MapAttribute;
    IA32_MAP_ATTRIBUTE                           MapMask;
    EFI_PHYSICAL_ADDRESS                         GhcbBase4K;
+  EFI_PHYSICAL_ADDRESS                         GhcbBaseEnd;
  
    //
    // Make sure AddressEncMask is contained to smallest supported address field
@@ -504,15 +505,21 @@ CreateIdentityMappingPageTables (
      //
      // The GHCB range consists of two pages per CPU, the GHCB and a
      // per-CPU variable page. The GHCB page needs to be mapped as an
-    // unencrypted page while the per-CPU variable page needs to be
-    // mapped encrypted. These pages alternate in assignment.
+    // unencrypted page while the per-CPU variable page needs to remain
+    // mapped as an encrypted page.
+    //
+    // Loop through the GHCB range, remapping the GHCB page unencrypted
+    // and skipping over the per-CPU variable page.
      //
      ASSERT (Is64BitPageTable == TRUE);
-    GhcbBase4K                           = ALIGN_VALUE (GhcbBase, SIZE_4KB);
-    MapAttribute.Uint64                  = GhcbBase4K;
-    MapMask.Uint64                       = 0;
-    MapMask.Bits.PageTableBaseAddressLow = 1;
-    CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB, &MapAttribute, &MapMask);
+    GhcbBase4K  = ALIGN_VALUE (GhcbBase, SIZE_4KB);
+    GhcbBaseEnd = ALIGN_VALUE (GhcbBase + GhcbSize, SIZE_4KB);
+    for (; GhcbBase4K < GhcbBaseEnd; GhcbBase4K += (SIZE_4KB * 2)) {
+      MapAttribute.Uint64                  = GhcbBase4K;
+      MapMask.Uint64                       = 0;
+      MapMask.Bits.PageTableBaseAddressLow = 1;
+      CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB, &MapAttribute, &MapMask);
+    }
    }
  
    if (PcdGetBool (PcdSetNxForStack)) {

> 
> Thanks,
> Tom
> 
>>
>> Thanks,
>> Tom
>>
>>>
>>> Thanks,
>>> Dun
>>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
>>> Sent: Friday, March 31, 2023 5:34 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] [Patch V2 0/8] Create page table by 
>>> CpuPageTableLib in DxeIpl
>>>
>>> In this V2 patch set:
>>> 1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create page 
>>> table by CpuPageTableLib', change the input parameter name from 
>>> Is32BitPageTable to Is64BitPageTable and add a line of 
>>> "MapAttribute.Bits.Present = 0" before set a range to non-present.
>>> 3.In this patch 'Refinement to the code to set PageTable as RO', add a 
>>> line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.
>>>
>>> Dun Tan (8):
>>>    EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
>>>    MdeModulePkg: Add UefiCpuPkg.dec to pass DependencyCheck
>>>    MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
>>>    MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
>>>    MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO
>>>
>>>   EmulatorPkg/EmulatorPkg.dsc                      |   3 ++-
>>>   IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc          |   3 ++-
>>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            |   3 ++-
>>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |   4 +++-
>>>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 112 
>>> ++++------------------------------------------------------------------------------------------------------------
>>>   MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |   5 +++--
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711 
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 
>>> ++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>>   MdeModulePkg/MdeModulePkg.ci.yaml                |   5 +++--
>>>   MdeModulePkg/MdeModulePkg.dsc                    |   3 ++-
>>>   OvmfPkg/AmdSev/AmdSevX64.dsc                     |   2 +-
>>>   OvmfPkg/Bhyve/BhyveX64.dsc                       |   3 ++-
>>>   OvmfPkg/CloudHv/CloudHvX64.dsc                   |   2 +-
>>>   OvmfPkg/Microvm/MicrovmX64.dsc                   |   2 +-
>>>   OvmfPkg/OvmfPkgIa32.dsc                          |   3 ++-
>>>   OvmfPkg/OvmfPkgIa32X64.dsc                       |   2 +-
>>>   OvmfPkg/OvmfPkgX64.dsc                           |   2 +-
>>>   OvmfPkg/OvmfXen.dsc                              |   2 +-
>>>   18 files changed, 200 insertions(+), 849 deletions(-)
>>>
>>> -- 
>>> 2.31.1.windows.1
>>>
>>>
>>>
>>> 
>>>
>>>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl
  2023-04-03 17:14       ` Lendacky, Thomas
@ 2023-04-04  1:23         ` duntan
  0 siblings, 0 replies; 5+ messages in thread
From: duntan @ 2023-04-04  1:23 UTC (permalink / raw)
  To: Tom Lendacky, devel@edk2.groups.io; +Cc: Ni, Ray

Thank you a lot! I misunderstood the condition for remapping GHCB page in previous code. I'll fix the issue in next version patch. 

Thanks,
Dun

-----Original Message-----
From: Tom Lendacky <thomas.lendacky@amd.com> 
Sent: Tuesday, April 4, 2023 1:14 AM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl

On 4/3/23 09:24, Tom Lendacky wrote:
> On 3/31/23 09:35, Tom Lendacky wrote:
>> On 3/31/23 04:41, Tan, Dun wrote:
>>> Hi Tom,
>>>
>>> Reccentlly I sent this patch set to change DxeIpl code to use 
>>> CpuPageTableLib to create page table. I have done some test on Intel 
>>> CPU to make sure that the page table created by DxeIpl before the 
>>> change is the same as the page table created by DxeIpl after the 
>>> change. But there was a remaining case that I didn't cover. The case 
>>> is that PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and 
>>> PcdGhcbSize are not zero(when memory encryption is enabled on AMD 
>>> processors supporting the SEV feature).
>>> So could you please help do a test on AMD processor to make sure 
>>> that the SEV feature still works good with this pacth set?
>>
>> Yes, I can test it.
> 
> This is breaking the SEV-ES and SEV-SNP boots. I'll see if I can 
> figure out what or where the breakage is, but this patchset can't be merged as is.

The following change to the patch series allows SEV-ES and SEV-SNP guests to boot.

Thanks,
Tom


diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index a9edf4de32..a3f16c7cf9 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -416,6 +416,7 @@ CreateIdentityMappingPageTables (
    IA32_MAP_ATTRIBUTE                           MapAttribute;
    IA32_MAP_ATTRIBUTE                           MapMask;
    EFI_PHYSICAL_ADDRESS                         GhcbBase4K;
+  EFI_PHYSICAL_ADDRESS                         GhcbBaseEnd;
  
    //
    // Make sure AddressEncMask is contained to smallest supported address field @@ -504,15 +505,21 @@ CreateIdentityMappingPageTables (
      //
      // The GHCB range consists of two pages per CPU, the GHCB and a
      // per-CPU variable page. The GHCB page needs to be mapped as an
-    // unencrypted page while the per-CPU variable page needs to be
-    // mapped encrypted. These pages alternate in assignment.
+    // unencrypted page while the per-CPU variable page needs to remain
+    // mapped as an encrypted page.
+    //
+    // Loop through the GHCB range, remapping the GHCB page unencrypted
+    // and skipping over the per-CPU variable page.
      //
      ASSERT (Is64BitPageTable == TRUE);
-    GhcbBase4K                           = ALIGN_VALUE (GhcbBase, SIZE_4KB);
-    MapAttribute.Uint64                  = GhcbBase4K;
-    MapMask.Uint64                       = 0;
-    MapMask.Bits.PageTableBaseAddressLow = 1;
-    CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB, &MapAttribute, &MapMask);
+    GhcbBase4K  = ALIGN_VALUE (GhcbBase, SIZE_4KB);
+    GhcbBaseEnd = ALIGN_VALUE (GhcbBase + GhcbSize, SIZE_4KB);
+    for (; GhcbBase4K < GhcbBaseEnd; GhcbBase4K += (SIZE_4KB * 2)) {
+      MapAttribute.Uint64                  = GhcbBase4K;
+      MapMask.Uint64                       = 0;
+      MapMask.Bits.PageTableBaseAddressLow = 1;
+      CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB, &MapAttribute, &MapMask);
+    }
    }
  
    if (PcdGetBool (PcdSetNxForStack)) {

> 
> Thanks,
> Tom
> 
>>
>> Thanks,
>> Tom
>>
>>>
>>> Thanks,
>>> Dun
>>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
>>> duntan
>>> Sent: Friday, March 31, 2023 5:34 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] [Patch V2 0/8] Create page table by 
>>> CpuPageTableLib in DxeIpl
>>>
>>> In this V2 patch set:
>>> 1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create 
>>> page table by CpuPageTableLib', change the input parameter name from 
>>> Is32BitPageTable to Is64BitPageTable and add a line of 
>>> "MapAttribute.Bits.Present = 0" before set a range to non-present.
>>> 3.In this patch 'Refinement to the code to set PageTable as RO', add 
>>> a line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.
>>>
>>> Dun Tan (8):
>>>    EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
>>>    OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
>>>    MdeModulePkg: Add UefiCpuPkg.dec to pass DependencyCheck
>>>    MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
>>>    MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
>>>    MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as 
>>> RO
>>>
>>>   EmulatorPkg/EmulatorPkg.dsc                      |   3 ++-
>>>   IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc          |   3 ++-
>>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.h            |   3 ++-
>>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |   4 +++-
>>>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  | 112
>>> ++++----------------------------------------------------------------
>>> ++++--------------------------------------------
>>>   MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |   5 +++--
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182
>>> ++++++++++----------------------------------------------------------
>>> ++++++++++----------------------------------------------------------
>>> ++++++++++--------------------------------------------------------
>>>   MdeModulePkg/MdeModulePkg.ci.yaml                |   5 +++--
>>>   MdeModulePkg/MdeModulePkg.dsc                    |   3 ++-
>>>   OvmfPkg/AmdSev/AmdSevX64.dsc                     |   2 +-
>>>   OvmfPkg/Bhyve/BhyveX64.dsc                       |   3 ++-
>>>   OvmfPkg/CloudHv/CloudHvX64.dsc                   |   2 +-
>>>   OvmfPkg/Microvm/MicrovmX64.dsc                   |   2 +-
>>>   OvmfPkg/OvmfPkgIa32.dsc                          |   3 ++-
>>>   OvmfPkg/OvmfPkgIa32X64.dsc                       |   2 +-
>>>   OvmfPkg/OvmfPkgX64.dsc                           |   2 +-
>>>   OvmfPkg/OvmfXen.dsc                              |   2 +-
>>>   18 files changed, 200 insertions(+), 849 deletions(-)
>>>
>>> --
>>> 2.31.1.windows.1
>>>
>>>
>>>
>>> 
>>>
>>>

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-04  1:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <17517763F19F09A0.27612@groups.io>
2023-03-31  9:41 ` [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib in DxeIpl duntan
2023-03-31 14:35   ` Lendacky, Thomas
2023-04-03 14:24     ` Lendacky, Thomas
2023-04-03 17:14       ` Lendacky, Thomas
2023-04-04  1:23         ` duntan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox