Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 11 commits into from

Conversation

lersek
Copy link
Member

@lersek lersek commented Nov 5, 2020

https://bugzilla.tianocore.org/show_bug.cgi?id=3008
https://edk2.groups.io/g/devel/message/66741
http://mid.mail-archive.com/cover.1603981082.git.thomas.lendacky@amd.com

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(-)

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

Use OFFSET_OF () and sizeof () to calculate the GHCB register field
offsets instead of hardcoding the values in the GHCB_REGISTER enum.
Define only fields that are used per the GHCB specification, which will
result in removing some fields and adding others.

Also, remove the DR7 field from the GHCB_SAVE_AREA structure since it is
not used/defined in the GHCB specification and then rename the reserved
fields as appropriate.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <11164899a8f43cfb79beb98e113141c14f0c2240.1603981082.git.thomas.lendacky@amd.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

In upcoming patches, the setting of the bits in the GHCB ValidBitmap will
be performed in multiple places. In order to reduce code duplication, add
an interface, VmgSetOffsetValid(), to VmgExitLib library to perform this
function. Also, to keep management of the ValidBitmap within the library,
add an inteface, VmgIsOffsetValid(), to return whether the bit in the
ValidBitmap is set for a specified offset.

The new VmgSetOffsetValid() function is a VOID function and will be an
empty function in the VmgExitLibNull implementation of the VmgExitLib
library.

The new VmgIsOffsetValid() function returns a BOOLEAN to indicate if the
offset is valid. This will always return FALSE in the VmgExitLibNull
implementation of the VmgExitLib library.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <360a3882884716ebb60276220c18e33e127a83f9.1603981082.git.thomas.lendacky@amd.com>
Acked-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

The VmgExitLib library added two new interfaces, VmgSetOffsetValid() and
VmgIsOffsetValid(), that must now be implemented in the OvmfPkg version
of the library.

Implement VmgSetOffsetValid() and VmgIsOffsetValid() and update existing
code, that is directly accessing ValidBitmap, to use the new interfaces.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <0e7695dce56e749e80d5df1e484cc4ac8011717c.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bits for the
software exit information fields when performing a VMGEXIT (SwExitCode,
SwExitInfo1, SwExitInfo2).

Fixes: 61bacc0
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <dd1863c2ca7cc61c710614da0044b4fa7c0e09d2.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bit for the scratch
area field (SwScratch).

Fixes: 0020157
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <fe9958edf6c66f7979cb9fa3df2789d22221c79c.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bit for the scratch
area field (SwScratch).

Fixes: c45f678
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c00f8534a2aed7805d5c2f79e6466ff8ac73c4a0.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bits for the
software exit information fields when performing a VMGEXIT (SwExitCode,
SwExitInfo1, SwExitInfo2).

Fixes: 20da7ca
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <55a090da23698f4988861461b31b5ee81b4667c9.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bit for the scratch
area field (SwScratch).

Fixes: 437eb3f
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <54463dadda03ef4ff6fae89ec5e9a1c20a663f31.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

The original SEV-ES support missed updating the QemuFlashEraseBlock()
function to successfully erase blocks. Update QemuFlashEraseBlock() to
call the QemuFlashPtrWrite() to be able to successfully perform the
commands under SEV-ES.

Fixes: 437eb3f
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c2e86d6faf1a9e564253259fcfc6b73509eb103a.1603981082.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008

The QemuFlashPtrWrite() flash services runtime uses the GHCB and VmgExit()
directly to perform the flash write when running as an SEV-ES guest. If an
interrupt arrives between VmgInit() and VmgExit(), the Dr7 read in the
interrupt handler will generate a #VC, which can overwrite information in
the GHCB that QemuFlashPtrWrite() has set. This has been seen with the
timer interrupt firing and the CpuExceptionHandlerLib library code,
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/
  Xcode5ExceptionHandlerAsm.nasm and
  ExceptionHandlerAsm.nasm
reading the Dr7 register while QemuFlashPtrWrite() is using the GHCB. In
general, it is necessary to protect the GHCB whenever it is used, not just
in QemuFlashPtrWrite().

Disable interrupts around the usage of the GHCB by modifying the VmgInit()
and VmgDone() interfaces:
- VmgInit() will take an extra parameter that is a pointer to a BOOLEAN
  that will hold the interrupt state at the time of invocation. VmgInit()
  will get and save this interrupt state before updating the GHCB.
- VmgDone() will take an extra parameter that is used to indicate whether
  interrupts are to be (re)enabled. Before exiting, VmgDone() will enable
  interrupts if that is requested.

Fixes: 437eb3f
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <0f3f11bec870bc4f10843beb53c6a919ac549ef4.1603981082.git.thomas.lendacky@amd.com>
Acked-by: Eric Dong <eric.dong@intel.com>
…number

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

Set the SEV-ES reset stack address for an AP based on the processor number
instead of the APIC ID in case the APIC IDs are not zero-based and densely
packed/enumerated. This will ensure an AP reset stack address does not get
set outside of the AP reset stack memory allocation.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c078bcfcc6ff450e4e804410ca1bfcc6228ad2d6.1603981082.git.thomas.lendacky@amd.com>
@lersek lersek added the push Auto push patch series in PR if all checks pass label Nov 5, 2020
@mergify
Copy link

mergify bot commented Nov 5, 2020

PR can not be merged due to an Ubuntu GCC5 failure. Please resolve and resubmit

@mergify
Copy link

mergify bot commented Nov 5, 2020

PR can not be merged due to a Windows VS2019 failure. Please resolve and resubmit

@lersek
Copy link
Member Author

lersek commented Nov 10, 2020

Superseded by #1110, closing.

@lersek lersek closed this Nov 10, 2020
@lersek lersek deleted the tom_sev_es_fixes_v3_push branch December 18, 2020 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants