Fixes for SEV-SNP CC blob and CPUID table handling #4313
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The SEV-SNP Confidential Computing blob contains metadata that should remain accessible for the life of the guest. Allocate it as EfiACPIReclaimMemory to ensure the memory isn't overwritten by the guest operating system later. Reported-by: Dov Murik <dovmurik@linux.ibm.com> Suggested-by: Dov Murik <dovmurik@linux.ibm.com> Reviewed-by: Dov Murik <dovmurik@linux.ibm.com> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
The Confidential Computing blob defined here is intended to match the definition defined by linux guest kernel. Previously, both definitions relied on natural alignment, but that relies on both OVMF and kernel being compiled as 64-bit. While there aren't currently any plans to enable SNP support for 32-bit compilations, the kernel definition has since been updated to use explicit padding/reserved fields to avoid this dependency. Update OVMF to match that definition. While at it, also fix up the Reserved fields to match the numbering used in the kernel. No functional changes (for currently-supported environments, at least). Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
CPUID leaf 0xD sub-leafs 0x0 and 0x1 contain cumulative sizes for the enabled XSave areas. Those sizes are calculated by tallying up all the other sub-leafs that contain per-area size information for XSave areas that are currently enabled in XCr0/XSS. The current check has the logic inverted. Fix that. This doesn't seem to cause problems currently, but could in the future if OVMF made more extensive use of XSave areas. It was noticed while implementing SNP-related tests for KVM Unit Tests, which re-uses the OVMF #VC handler in some cases. Reported-by: Pavan Kumar Paluri <papaluri@amd.com> Cc: Pavan Kumar Paluri <papaluri@amd.com> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
Currently OVMF tries to rely on the base size advertised via the CPUID table entries corresponding to leaf 0xD, sub-leafs 0x0/0x1. This will generally work for KVM guests, but might not for other SEV-SNP hypervisor implementations. Make the handling more robust by simply using the base area size documented by the APM. Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> Acked-by: Jiewen Yao <jiewen.yao@intel.com> Acked-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
From: Michael Roth michael.roth@amd.com
Date: 4/26/2023 4:32:54 AM
Subject: [PATCH v2 0/4] Fixes for SEV-SNP CC blob and CPUID table handling
(Mainly a resend of v1, but rolled in Gerd's Acked-by's, addressed
new coding style check in the CI, and updated Cc list)
Here are a number of fixes related to OVMF handling of the SEV-SNP
Confidential Computing blob and CPUID table.
Patch #1 is a fix for recently-reported issue that can cause
significant problems with some SEV-SNP guest operating systems.
Please consider applying this patch directly if the other
patches in this series are held up for any reason.
Patches 2-4 are minor changes for things that aren't currently
triggered in practice, but make OVMF's SEV-SNP implementation more
robust for different build/hypervisor environments in the future.
Patch #2 was submitted previously, but refreshed here to apply
cleanly on top of Patch #1, with no other functional changes since
the initial review.
v2:
test cases
Michael Roth (4):
OvmfPkg/AmdSevDxe: Allocate SEV-SNP CC blob as EfiACPIReclaimMemory
OvmfPkg/AmdSevDxe: Update ConfidentialComputing blob struct definition
OvmfPkg/CcExitLib: Fix SEV-SNP XSave area size calculation
OvmfPkg/CcExitLib: Use documented XSave area base size for SEV-SNP
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++--------------
OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h | 6 ++++--
OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 13 +++++--------
3 files changed, 59 insertions(+), 24 deletions(-)