[edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup

Lendacky, Thomas posted 11 patches 3 years, 6 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 ++++++++++++-
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 ++++++--------------
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
10 files changed, 258 insertions(+), 141 deletions(-)
[edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Lendacky, Thomas 3 years, 6 months ago
From: Tom Lendacky <thomas.lendacky@amd.com>

This patch series provides some fixes, updates and cleanup to the SEV-ES
guest support:

- Update the calculation of the qword offset of fields within the GHCB
  by removing the hardcoding of the offsets and using the OFFSET_OF ()
  and sizeof () functions to calculate the values. Remove unused values
  and add values that will be used in later patches.

- Set the SwExitCode, SwExitInfo1, SwExitInfo2 and SwScratch valid bits
  in the GHCB ValidBitmap area when these fields are for a VMGEXIT. This
  is done by adding two new interfaces to the VmgExitLib library to set
  and test the bits of the GHCB ValidBitmap. This reduces code duplication
  and keeps access to the ValidBitmap field within the VmgExitLib library.

- Update the Qemu flash drive services support to add SEV-ES support for
  erasing blocks.

- Disable interrupts when using the GHCB.

- Use the processor number for setting the AP stack pointer instead of the
  APIC ID by calling GetProcessorNumber().

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

---

These patches are based on commit:
6ad819c1abe3 ("FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API")

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>

Changes since v2:
- Don't rename the GHCB_REGISTER enum type.

Changes since v1:
- For the GHCB savearea changes, create a new reserved area name instead
  of "renumbering" the reserved areas.
- Rework the ValidBitmap set/test support to be part of the VmgExitLib
  library. Create two new interfaces for setting and testing bits in the
  GHCB ValidBitmap field and adjust all existing code and the new code in
  this series to use these interfaces for the ValidBitmap updates/checks.
- Don't disable interrupts for just the Qemu flash services support, but
  rather, cover all users of the GHCB by disabling interrupts in VmgInit()
  and restoring them in VmgDone(). This requires changes to those
  interaces.

Tom Lendacky (11):
  MdePkg: Clean up GHCB field offsets and save area
  UefiCpuPkg/VmgExitLib: Add interfaces to set/read GHCB ValidBitmap
    bits
  OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces
  OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT
  OvmfPkg/VmgExitLib: Set the SwScratch valid bit for IOIO events
  OvmfPkg/VmgExitLib: Set the SwScratch valid bit for MMIO events
  UefiCpuPkg/MpInitLib: Set the SW exit fields when performing VMGEXIT
  OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Set the SwScratch valid bit
  OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Fix erase blocks for SEV-ES
  UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
  UefiCpuPkg/MpInitLib: For SEV-ES guest, set stack based on processor
    number

 MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
 UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
 OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 ++++++++++++-
 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 ++++++--------------
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
 10 files changed, 258 insertions(+), 141 deletions(-)

-- 
2.28.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66741): https://edk2.groups.io/g/devel/message/66741
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Laszlo Ersek 3 years, 5 months ago
On 10/29/20 15:17, Tom Lendacky wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> This patch series provides some fixes, updates and cleanup to the SEV-ES
> guest support:
> 
> - Update the calculation of the qword offset of fields within the GHCB
>   by removing the hardcoding of the offsets and using the OFFSET_OF ()
>   and sizeof () functions to calculate the values. Remove unused values
>   and add values that will be used in later patches.
> 
> - Set the SwExitCode, SwExitInfo1, SwExitInfo2 and SwScratch valid bits
>   in the GHCB ValidBitmap area when these fields are for a VMGEXIT. This
>   is done by adding two new interfaces to the VmgExitLib library to set
>   and test the bits of the GHCB ValidBitmap. This reduces code duplication
>   and keeps access to the ValidBitmap field within the VmgExitLib library.
> 
> - Update the Qemu flash drive services support to add SEV-ES support for
>   erasing blocks.
> 
> - Disable interrupts when using the GHCB.
> 
> - Use the processor number for setting the AP stack pointer instead of the
>   APIC ID by calling GetProcessorNumber().
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
> 
> ---
> 
> These patches are based on commit:
> 6ad819c1abe3 ("FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API")
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> 
> Changes since v2:
> - Don't rename the GHCB_REGISTER enum type.

I've got this queued for review. I'll need some time for getting to it,
as I've just returned after some absence, and everything seems to have
collapsed on my head (as usual).

Laszlo

> 
> Changes since v1:
> - For the GHCB savearea changes, create a new reserved area name instead
>   of "renumbering" the reserved areas.
> - Rework the ValidBitmap set/test support to be part of the VmgExitLib
>   library. Create two new interfaces for setting and testing bits in the
>   GHCB ValidBitmap field and adjust all existing code and the new code in
>   this series to use these interfaces for the ValidBitmap updates/checks.
> - Don't disable interrupts for just the Qemu flash services support, but
>   rather, cover all users of the GHCB by disabling interrupts in VmgInit()
>   and restoring them in VmgDone(). This requires changes to those
>   interaces.
> 
> Tom Lendacky (11):
>   MdePkg: Clean up GHCB field offsets and save area
>   UefiCpuPkg/VmgExitLib: Add interfaces to set/read GHCB ValidBitmap
>     bits
>   OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces
>   OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT
>   OvmfPkg/VmgExitLib: Set the SwScratch valid bit for IOIO events
>   OvmfPkg/VmgExitLib: Set the SwScratch valid bit for MMIO events
>   UefiCpuPkg/MpInitLib: Set the SW exit fields when performing VMGEXIT
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Set the SwScratch valid bit
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Fix erase blocks for SEV-ES
>   UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
>   UefiCpuPkg/MpInitLib: For SEV-ES guest, set stack based on processor
>     number
> 
>  MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
>  UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
>  OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 ++++++++++++-
>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 ++++++--------------
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
>  UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
>  10 files changed, 258 insertions(+), 141 deletions(-)
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66866): https://edk2.groups.io/g/devel/message/66866
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Laszlo Ersek 3 years, 5 months ago
On 10/29/20 15:17, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> This patch series provides some fixes, updates and cleanup to the SEV-ES
> guest support:
> 
> - Update the calculation of the qword offset of fields within the GHCB
>   by removing the hardcoding of the offsets and using the OFFSET_OF ()
>   and sizeof () functions to calculate the values. Remove unused values
>   and add values that will be used in later patches.
> 
> - Set the SwExitCode, SwExitInfo1, SwExitInfo2 and SwScratch valid bits
>   in the GHCB ValidBitmap area when these fields are for a VMGEXIT. This
>   is done by adding two new interfaces to the VmgExitLib library to set
>   and test the bits of the GHCB ValidBitmap. This reduces code duplication
>   and keeps access to the ValidBitmap field within the VmgExitLib library.
> 
> - Update the Qemu flash drive services support to add SEV-ES support for
>   erasing blocks.
> 
> - Disable interrupts when using the GHCB.
> 
> - Use the processor number for setting the AP stack pointer instead of the
>   APIC ID by calling GetProcessorNumber().
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
> 
> ---
> 
> These patches are based on commit:
> 6ad819c1abe3 ("FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API")
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> 
> Changes since v2:
> - Don't rename the GHCB_REGISTER enum type.
> 
> Changes since v1:
> - For the GHCB savearea changes, create a new reserved area name instead
>   of "renumbering" the reserved areas.
> - Rework the ValidBitmap set/test support to be part of the VmgExitLib
>   library. Create two new interfaces for setting and testing bits in the
>   GHCB ValidBitmap field and adjust all existing code and the new code in
>   this series to use these interfaces for the ValidBitmap updates/checks.
> - Don't disable interrupts for just the Qemu flash services support, but
>   rather, cover all users of the GHCB by disabling interrupts in VmgInit()
>   and restoring them in VmgDone(). This requires changes to those
>   interaces.
> 
> Tom Lendacky (11):
>   MdePkg: Clean up GHCB field offsets and save area
>   UefiCpuPkg/VmgExitLib: Add interfaces to set/read GHCB ValidBitmap
>     bits
>   OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces
>   OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT
>   OvmfPkg/VmgExitLib: Set the SwScratch valid bit for IOIO events
>   OvmfPkg/VmgExitLib: Set the SwScratch valid bit for MMIO events
>   UefiCpuPkg/MpInitLib: Set the SW exit fields when performing VMGEXIT
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Set the SwScratch valid bit
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Fix erase blocks for SEV-ES
>   UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
>   UefiCpuPkg/MpInitLib: For SEV-ES guest, set stack based on processor
>     number
> 
>  MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
>  UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
>  OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 ++++++++++++-
>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 ++++++--------------
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
>  UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
>  10 files changed, 258 insertions(+), 141 deletions(-)
> 

I've submitted PR#1086 <https://github.com/tianocore/edk2/pull/1086>,
but CI seems slower than usual today, and I really need some sleep, so I
won't wait for CI. Tom, if the PR succeeds, please close TianoCore#3008,
noting the commit range, and please also follow up in this thread with
the commit range.

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67024): https://edk2.groups.io/g/devel/message/67024
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Lendacky, Thomas 3 years, 5 months ago
On 11/4/20 9:29 PM, Laszlo Ersek wrote:
> On 10/29/20 15:17, Lendacky, Thomas wrote:
>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>
>> This patch series provides some fixes, updates and cleanup to the SEV-ES
>> guest support:
>>
>> - Update the calculation of the qword offset of fields within the GHCB
>>    by removing the hardcoding of the offsets and using the OFFSET_OF ()
>>    and sizeof () functions to calculate the values. Remove unused values
>>    and add values that will be used in later patches.
>>
>> - Set the SwExitCode, SwExitInfo1, SwExitInfo2 and SwScratch valid bits
>>    in the GHCB ValidBitmap area when these fields are for a VMGEXIT. This
>>    is done by adding two new interfaces to the VmgExitLib library to set
>>    and test the bits of the GHCB ValidBitmap. This reduces code duplication
>>    and keeps access to the ValidBitmap field within the VmgExitLib library.
>>
>> - Update the Qemu flash drive services support to add SEV-ES support for
>>    erasing blocks.
>>
>> - Disable interrupts when using the GHCB.
>>
>> - Use the processor number for setting the AP stack pointer instead of the
>>    APIC ID by calling GetProcessorNumber().
>>
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3008&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C0697eb8b721342b8a39508d8813b1f3c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637401438219622636%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7Ox%2FhGWT9GKg9v5gIX2xRjIvKLiGBkeBzT7Via20cZk%3D&amp;reserved=0
>>
>> ---
>>
>> These patches are based on commit:
>> 6ad819c1abe3 ("FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API")
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>
>> Changes since v2:
>> - Don't rename the GHCB_REGISTER enum type.
>>
>> Changes since v1:
>> - For the GHCB savearea changes, create a new reserved area name instead
>>    of "renumbering" the reserved areas.
>> - Rework the ValidBitmap set/test support to be part of the VmgExitLib
>>    library. Create two new interfaces for setting and testing bits in the
>>    GHCB ValidBitmap field and adjust all existing code and the new code in
>>    this series to use these interfaces for the ValidBitmap updates/checks.
>> - Don't disable interrupts for just the Qemu flash services support, but
>>    rather, cover all users of the GHCB by disabling interrupts in VmgInit()
>>    and restoring them in VmgDone(). This requires changes to those
>>    interaces.
>>
>> Tom Lendacky (11):
>>    MdePkg: Clean up GHCB field offsets and save area
>>    UefiCpuPkg/VmgExitLib: Add interfaces to set/read GHCB ValidBitmap
>>      bits
>>    OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces
>>    OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT
>>    OvmfPkg/VmgExitLib: Set the SwScratch valid bit for IOIO events
>>    OvmfPkg/VmgExitLib: Set the SwScratch valid bit for MMIO events
>>    UefiCpuPkg/MpInitLib: Set the SW exit fields when performing VMGEXIT
>>    OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Set the SwScratch valid bit
>>    OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Fix erase blocks for SEV-ES
>>    UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
>>    UefiCpuPkg/MpInitLib: For SEV-ES guest, set stack based on processor
>>      number
>>
>>   MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
>>   UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
>>   OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 ++++++++++++-
>>   OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 ++++++--------------
>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
>>   UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
>>   UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
>>   UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
>>   10 files changed, 258 insertions(+), 141 deletions(-)
>>
> 
> I've submitted PR#1086 <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1086&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C0697eb8b721342b8a39508d8813b1f3c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637401438219622636%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NH2IfxbTqaHvVijHOGExWFU%2FECzxOVwgJyVajPZjaCw%3D&amp;reserved=0>,
> but CI seems slower than usual today, and I really need some sleep, so I
> won't wait for CI. Tom, if the PR succeeds, please close TianoCore#3008,
> noting the commit range, and please also follow up in this thread with
> the commit range.

Thanks, Laszlo!

It looks like it failed because it doesn't like the use of the "sizeof 
(UINT64)". I suppose I can change that to just hard code a value of 8. Let 
me know what you think.

Thanks,
Tom

> 
> Thanks!
> Laszlo
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67043): https://edk2.groups.io/g/devel/message/67043
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Lendacky, Thomas 3 years, 5 months ago
On 11/5/20 8:34 AM, Tom Lendacky wrote:
> On 11/4/20 9:29 PM, Laszlo Ersek wrote:
>> On 10/29/20 15:17, Lendacky, Thomas wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> This patch series provides some fixes, updates and cleanup to the SEV-ES
>>> guest support:
>>>
>>> - Update the calculation of the qword offset of fields within the GHCB
>>>    by removing the hardcoding of the offsets and using the OFFSET_OF ()
>>>    and sizeof () functions to calculate the values. Remove unused values
>>>    and add values that will be used in later patches.
>>>
>>> - Set the SwExitCode, SwExitInfo1, SwExitInfo2 and SwScratch valid bits
>>>    in the GHCB ValidBitmap area when these fields are for a VMGEXIT. This
>>>    is done by adding two new interfaces to the VmgExitLib library to set
>>>    and test the bits of the GHCB ValidBitmap. This reduces code 
>>> duplication
>>>    and keeps access to the ValidBitmap field within the VmgExitLib 
>>> library.
>>>
>>> - Update the Qemu flash drive services support to add SEV-ES support for
>>>    erasing blocks.
>>>
>>> - Disable interrupts when using the GHCB.
>>>
>>> - Use the processor number for setting the AP stack pointer instead of the
>>>    APIC ID by calling GetProcessorNumber().
>>>
>>> BZ: 
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3008&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C0697eb8b721342b8a39508d8813b1f3c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637401438219622636%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7Ox%2FhGWT9GKg9v5gIX2xRjIvKLiGBkeBzT7Via20cZk%3D&amp;reserved=0 
>>>
>>>
>>> ---
>>>
>>> These patches are based on commit:
>>> 6ad819c1abe3 ("FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to 
>>> Check/Set API")
>>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> Cc: Eric Dong <eric.dong@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>>
>>> Changes since v2:
>>> - Don't rename the GHCB_REGISTER enum type.
>>>
>>> Changes since v1:
>>> - For the GHCB savearea changes, create a new reserved area name instead
>>>    of "renumbering" the reserved areas.
>>> - Rework the ValidBitmap set/test support to be part of the VmgExitLib
>>>    library. Create two new interfaces for setting and testing bits in the
>>>    GHCB ValidBitmap field and adjust all existing code and the new code in
>>>    this series to use these interfaces for the ValidBitmap updates/checks.
>>> - Don't disable interrupts for just the Qemu flash services support, but
>>>    rather, cover all users of the GHCB by disabling interrupts in 
>>> VmgInit()
>>>    and restoring them in VmgDone(). This requires changes to those
>>>    interaces.
>>>
>>> Tom Lendacky (11):
>>>    MdePkg: Clean up GHCB field offsets and save area
>>>    UefiCpuPkg/VmgExitLib: Add interfaces to set/read GHCB ValidBitmap
>>>      bits
>>>    OvmfPkg/VmgExitLib: Implement new VmgExitLib interfaces
>>>    OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT
>>>    OvmfPkg/VmgExitLib: Set the SwScratch valid bit for IOIO events
>>>    OvmfPkg/VmgExitLib: Set the SwScratch valid bit for MMIO events
>>>    UefiCpuPkg/MpInitLib: Set the SW exit fields when performing VMGEXIT
>>>    OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Set the SwScratch valid bit
>>>    OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Fix erase blocks for SEV-ES
>>>    UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
>>>    UefiCpuPkg/MpInitLib: For SEV-ES guest, set stack based on processor
>>>      number
>>>
>>>   MdePkg/Include/Register/Amd/Ghcb.h                    |  40 +++---
>>>   UefiCpuPkg/Include/Library/VmgExitLib.h               |  51 +++++++-
>>>   OvmfPkg/Library/VmgExitLib/VmgExitLib.c               |  84 
>>> ++++++++++++-
>>>   OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         | 129 
>>> ++++++--------------
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c    |   4 +-
>>>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |   6 +-
>>>   UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |   5 +-
>>>   UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  14 ++-
>>>   UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    |  60 +++++++--
>>>   UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm         |   6 +
>>>   10 files changed, 258 insertions(+), 141 deletions(-)
>>>
>>
>> I've submitted PR#1086 
>> <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1086&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C0697eb8b721342b8a39508d8813b1f3c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637401438219622636%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NH2IfxbTqaHvVijHOGExWFU%2FECzxOVwgJyVajPZjaCw%3D&amp;reserved=0>, 
>>
>> but CI seems slower than usual today, and I really need some sleep, so I
>> won't wait for CI. Tom, if the PR succeeds, please close TianoCore#3008,
>> noting the commit range, and please also follow up in this thread with
>> the commit range.
> 
> Thanks, Laszlo!
> 
> It looks like it failed because it doesn't like the use of the "sizeof 
> (UINT64)". I suppose I can change that to just hard code a value of 8. Let 
> me know what you think.

I did verify that changing the "sizeof (UINT64)" to "8" in the first patch 
results in all CI tests passing. I can re-submit with that, if you feel 
that is the best course of action.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>> Thanks!
>> Laszlo
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67080): https://edk2.groups.io/g/devel/message/67080
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Laszlo Ersek 3 years, 5 months ago
On 11/06/20 07:29, Tom Lendacky wrote:
> On 11/5/20 8:34 AM, Tom Lendacky wrote:
>> On 11/4/20 9:29 PM, Laszlo Ersek wrote:

>>> I've submitted PR#1086 [...] but CI seems slower than usual today,
>>> and I really need some sleep, so I won't wait for CI. Tom, if the PR
>>> succeeds, please close TianoCore#3008, noting the commit range, and
>>> please also follow up in this thread with the commit range.
>>
>> Thanks, Laszlo!
>>
>> It looks like it failed because it doesn't like the use of the
>> "sizeof (UINT64)". I suppose I can change that to just hard code a
>> value of 8. Let me know what you think.
>
> I did verify that changing the "sizeof (UINT64)" to "8" in the first
> patch results in all CI tests passing. I can re-submit with that, if
> you feel that is the best course of action.


So this is the error (from ECC), if I understand correctly:

> 2020-11-05T03:25:05.2175761Z ERROR - EFI coding style error
> 2020-11-05T03:25:05.2176262Z ERROR - *Error code: 8005
> 2020-11-05T03:25:05.2177026Z ERROR - *Variable name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters 4. Global variable name must start with a 'g'
> 2020-11-05T03:25:05.2177773Z ERROR - *file: //home/vsts/work/1/s/MdePkg/Include/Register/Amd/Ghcb.h
> 2020-11-05T03:25:05.2180810Z ERROR - *Line number: 114
> 2020-11-05T03:25:05.2181426Z ERROR - *Member variable [GHCB_REGISTER.(UINT64)] NOT follow naming convention.

for such lines as:

>   GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),

I think this is a bug in CheckMemberVariableFormat(), in
"BaseTools/Source/Python/Ecc/c.py".

I don't exactly understand how that ECC method functions. However,
squashing the following update into the first (MdePkg) patch of this
series seems to work:

> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
> index 1fce64d5b7cd..3814a79e0619 100644
> --- a/MdePkg/Include/Register/Amd/Ghcb.h
> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
> @@ -111,17 +111,20 @@ typedef PACKED struct {
>    UINT32                 GhcbUsage;
>  } GHCB;
>
> +#define GHCB_SAVE_AREA_WORD_OFFSET(RegisterField) \
> +  (OFFSET_OF (GHCB, SaveArea.RegisterField) / sizeof (UINT64))
> +
>  typedef enum {
> -  GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
> -  GhcbRax          = OFFSET_OF (GHCB, SaveArea.Rax) / sizeof (UINT64),
> -  GhcbRbx          = OFFSET_OF (GHCB, SaveArea.Rbx) / sizeof (UINT64),
> -  GhcbRcx          = OFFSET_OF (GHCB, SaveArea.Rcx) / sizeof (UINT64),
> -  GhcbRdx          = OFFSET_OF (GHCB, SaveArea.Rdx) / sizeof (UINT64),
> -  GhcbXCr0         = OFFSET_OF (GHCB, SaveArea.XCr0) / sizeof (UINT64),
> -  GhcbSwExitCode   = OFFSET_OF (GHCB, SaveArea.SwExitCode) / sizeof (UINT64),
> -  GhcbSwExitInfo1  = OFFSET_OF (GHCB, SaveArea.SwExitInfo1) / sizeof (UINT64),
> -  GhcbSwExitInfo2  = OFFSET_OF (GHCB, SaveArea.SwExitInfo2) / sizeof (UINT64),
> -  GhcbSwScratch    = OFFSET_OF (GHCB, SaveArea.SwScratch) / sizeof (UINT64),
> +  GhcbCpl          = GHCB_SAVE_AREA_WORD_OFFSET (Cpl),
> +  GhcbRax          = GHCB_SAVE_AREA_WORD_OFFSET (Rax),
> +  GhcbRbx          = GHCB_SAVE_AREA_WORD_OFFSET (Rbx),
> +  GhcbRcx          = GHCB_SAVE_AREA_WORD_OFFSET (Rcx),
> +  GhcbRdx          = GHCB_SAVE_AREA_WORD_OFFSET (Rdx),
> +  GhcbXCr0         = GHCB_SAVE_AREA_WORD_OFFSET (XCr0),
> +  GhcbSwExitCode   = GHCB_SAVE_AREA_WORD_OFFSET (SwExitCode),
> +  GhcbSwExitInfo1  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo1),
> +  GhcbSwExitInfo2  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo2),
> +  GhcbSwScratch    = GHCB_SAVE_AREA_WORD_OFFSET (SwScratch),
>  } GHCB_REGISTER;
>
>  typedef union {

I build-tested this update with gcc, and also verified that it passes
ECC, by running CI locally. (I reproduced the bogus ECC error message
locally at first, of course, to see if the update makes a difference.)

> PROGRESS - --Running MdePkg: EccCheck Test NO-TARGET --
> PROGRESS - --->Test Success: EccCheck Test NO-TARGET

If it works for you as well (check with a personal build PR perhaps),
then I suggest posting v4 -- Liming should please re-check the udpated
MdePkg patch, and then we can merge the series.

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67097): https://edk2.groups.io/g/devel/message/67097
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Posted by Lendacky, Thomas 3 years, 5 months ago
On 11/6/20 10:46 AM, Laszlo Ersek wrote:
> On 11/06/20 07:29, Tom Lendacky wrote:
>> On 11/5/20 8:34 AM, Tom Lendacky wrote:
>>> On 11/4/20 9:29 PM, Laszlo Ersek wrote:
> 
>>>> I've submitted PR#1086 [...] but CI seems slower than usual today,
>>>> and I really need some sleep, so I won't wait for CI. Tom, if the PR
>>>> succeeds, please close TianoCore#3008, noting the commit range, and
>>>> please also follow up in this thread with the commit range.
>>>
>>> Thanks, Laszlo!
>>>
>>> It looks like it failed because it doesn't like the use of the
>>> "sizeof (UINT64)". I suppose I can change that to just hard code a
>>> value of 8. Let me know what you think.
>>
>> I did verify that changing the "sizeof (UINT64)" to "8" in the first
>> patch results in all CI tests passing. I can re-submit with that, if
>> you feel that is the best course of action.
> 
> 
> So this is the error (from ECC), if I understand correctly:
> 
>> 2020-11-05T03:25:05.2175761Z ERROR - EFI coding style error
>> 2020-11-05T03:25:05.2176262Z ERROR - *Error code: 8005
>> 2020-11-05T03:25:05.2177026Z ERROR - *Variable name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters 4. Global variable name must start with a 'g'
>> 2020-11-05T03:25:05.2177773Z ERROR - *file: //home/vsts/work/1/s/MdePkg/Include/Register/Amd/Ghcb.h
>> 2020-11-05T03:25:05.2180810Z ERROR - *Line number: 114
>> 2020-11-05T03:25:05.2181426Z ERROR - *Member variable [GHCB_REGISTER.(UINT64)] NOT follow naming convention.
> 
> for such lines as:
> 
>>    GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
> 
> I think this is a bug in CheckMemberVariableFormat(), in
> "BaseTools/Source/Python/Ecc/c.py".
> 
> I don't exactly understand how that ECC method functions. However,
> squashing the following update into the first (MdePkg) patch of this
> series seems to work:
> 
>> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
>> index 1fce64d5b7cd..3814a79e0619 100644
>> --- a/MdePkg/Include/Register/Amd/Ghcb.h
>> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
>> @@ -111,17 +111,20 @@ typedef PACKED struct {
>>     UINT32                 GhcbUsage;
>>   } GHCB;
>>
>> +#define GHCB_SAVE_AREA_WORD_OFFSET(RegisterField) \

I'll make one minor change here, s/WORD/QWORD/.

>> +  (OFFSET_OF (GHCB, SaveArea.RegisterField) / sizeof (UINT64))
>> +
>>   typedef enum {
>> -  GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
>> -  GhcbRax          = OFFSET_OF (GHCB, SaveArea.Rax) / sizeof (UINT64),
>> -  GhcbRbx          = OFFSET_OF (GHCB, SaveArea.Rbx) / sizeof (UINT64),
>> -  GhcbRcx          = OFFSET_OF (GHCB, SaveArea.Rcx) / sizeof (UINT64),
>> -  GhcbRdx          = OFFSET_OF (GHCB, SaveArea.Rdx) / sizeof (UINT64),
>> -  GhcbXCr0         = OFFSET_OF (GHCB, SaveArea.XCr0) / sizeof (UINT64),
>> -  GhcbSwExitCode   = OFFSET_OF (GHCB, SaveArea.SwExitCode) / sizeof (UINT64),
>> -  GhcbSwExitInfo1  = OFFSET_OF (GHCB, SaveArea.SwExitInfo1) / sizeof (UINT64),
>> -  GhcbSwExitInfo2  = OFFSET_OF (GHCB, SaveArea.SwExitInfo2) / sizeof (UINT64),
>> -  GhcbSwScratch    = OFFSET_OF (GHCB, SaveArea.SwScratch) / sizeof (UINT64),
>> +  GhcbCpl          = GHCB_SAVE_AREA_WORD_OFFSET (Cpl),
>> +  GhcbRax          = GHCB_SAVE_AREA_WORD_OFFSET (Rax),
>> +  GhcbRbx          = GHCB_SAVE_AREA_WORD_OFFSET (Rbx),
>> +  GhcbRcx          = GHCB_SAVE_AREA_WORD_OFFSET (Rcx),
>> +  GhcbRdx          = GHCB_SAVE_AREA_WORD_OFFSET (Rdx),
>> +  GhcbXCr0         = GHCB_SAVE_AREA_WORD_OFFSET (XCr0),
>> +  GhcbSwExitCode   = GHCB_SAVE_AREA_WORD_OFFSET (SwExitCode),
>> +  GhcbSwExitInfo1  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo1),
>> +  GhcbSwExitInfo2  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo2),
>> +  GhcbSwScratch    = GHCB_SAVE_AREA_WORD_OFFSET (SwScratch),
>>   } GHCB_REGISTER;
>>
>>   typedef union {
> 
> I build-tested this update with gcc, and also verified that it passes
> ECC, by running CI locally. (I reproduced the bogus ECC error message
> locally at first, of course, to see if the update makes a difference.)
> 
>> PROGRESS - --Running MdePkg: EccCheck Test NO-TARGET --
>> PROGRESS - --->Test Success: EccCheck Test NO-TARGET
> 
> If it works for you as well (check with a personal build PR perhaps),

Yup, that works, all tests passed.

> then I suggest posting v4 -- Liming should please re-check the udpated
> MdePkg patch, and then we can merge the series.

Ok, I'll submit a v4 and without Liming's Reviewed-by on the first patch.

Thanks,
Tom

> 
> Thanks!
> Laszlo
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67098): https://edk2.groups.io/g/devel/message/67098
Mute This Topic: https://groups.io/mt/77888107/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-