On Fri, May 13, 2022 at 08:32:38AM -0500, Tom Lendacky wrote:
> On 5/13/22 08:22, Michael Roth wrote:
> > 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.
> >
> > No functional changes (for currently-supported environments, at least).
> >
> > Signed-off-by: Michael Roth <michael.r...@amd.com>
>
> Minor nit comment below that can be ignored if desired.
>
> Reviewed-by: Tom Lendacky <thomas.lenda...@amd.com>
>
> > ---
> > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 2 ++
> > OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h | 6 ++++--
> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > index 662d3c4ccb..ee6d2528d9 100644
> > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> > @@ -27,8 +27,10 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
> > mSnpBootDxeTable = {
> > 0,
> > (UINT64)(UINTN)FixedPcdGet32 (PcdOvmfSnpSecretsBase),
> > FixedPcdGet32 (PcdOvmfSnpSecretsSize),
> > + 0,
> > (UINT64)(UINTN)FixedPcdGet32 (PcdOvmfCpuidBase),
> > FixedPcdGet32 (PcdOvmfCpuidSize),
> > + 0,
> > };
> > EFI_STATUS
> > diff --git a/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
> > b/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
> > index b328310fd0..83620e31b8 100644
> > --- a/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
> > +++ b/OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
> > @@ -18,14 +18,16 @@
> > { 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42 }, \
> > }
> > -typedef struct {
> > +typedef PACKED struct {
> > UINT32 Header;
> > UINT16 Version;
> > - UINT16 Reserved1;
> > + UINT16 Reserved;
>
> Not to be picky, but I would have left this as Reserved1 and then made the
> below entries Reserved2 and Reserved3.
Hi Tom,
I updated those to match how the reserved fields are numbered in the
kernel since it seemed like it could cause confusion otherwise. I should
have noted that in the commit log though as it's a somewhat unrelated
change.
Thanks!
-Mike
>
> Thanks,
> Tom
>
> > UINT64 SecretsPhysicalAddress;
> > UINT32 SecretsSize;
> > + UINT32 Reserved1;
> > UINT64 CpuidPhysicalAddress;
> > UINT32 CpuidLSize;
> > + UINT32 Reserved2;
> > } CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION;
> > extern EFI_GUID gConfidentialComputingSevSnpBlobGuid;
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89727): https://edk2.groups.io/g/devel/message/89727
Mute This Topic: https://groups.io/mt/91080662/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
