> > Shouldn't this have a v2 in the subject (same goes for patch 2/4)? >
Yes. I'm upset that didn't happen. I used --subject="PATCHv2" in git send-email, and editing the cover letter showed that all the other emails would have PATCHv2 in the subject. Then I said to send the emails and saw "PATCH " get sent out anyway :( > I didn't see an answer as to why you couldn't use the > MemEncryptSevSnpPreValidateSystemRam() function in > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c to > accomplish this without introducing a whole new interface to the > MemEncryptSevLib library. > We could do the acceptance in the DXE implementation of MemEncryptSevSnpPreValidateSystemRam. I wasn't 100% on whether the same name function name should be used in two very different roles. I'll change it for v3. > Also, to better see the paths in the diffstat, I recommend using: > --diff-options "--stat=1000 --stat-graph-width=20" > Will do, thanks. > Thanks, > Tom > > > Cc: Gerd Hoffmann <kra...@redhat.com> > > Cc: James Bottomley <j...@linux.ibm.com> > > Cc: Jiewen Yao <jiewen....@intel.com> > > Cc: Tom Lendacky <thomas.lenda...@amd.com> > > > > Signed-off-by: Sophia Wolf <phiaw...@google.com> > > --- > > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 34 ++++++++++++++++++ > > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++ > > OvmfPkg/Include/Library/MemEncryptSevLib.h | 14 ++++++++ > > .../Ia32/MemEncryptSevLib.c | 17 +++++++++ > > .../X64/DxeSnpSystemRamValidate.c | 35 +++++++++++++++++++ > > .../X64/PeiSnpSystemRamValidate.c | 17 +++++++++ > > .../X64/SecSnpSystemRamValidate.c | 18 ++++++++++ > > 7 files changed, 138 insertions(+) > > > > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > > index 662d3c4ccb..6e3a1fc7d7 100644 > > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c > > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c > > @@ -20,6 +20,7 @@ > > #include <Library/UefiBootServicesTableLib.h> > > #include <Guid/ConfidentialComputingSevSnpBlob.h> > > #include <Library/PcdLib.h> > > +#include <Protocol/MemoryAccept.h> > > > > STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = { > > SIGNATURE_32 ('A', 'M', 'D', 'E'), > > @@ -31,6 +32,29 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION > > mSnpBootDxeTable = { > > FixedPcdGet32 (PcdOvmfCpuidSize), > > }; > > > > +STATIC EFI_HANDLE mAmdSevDxeHandle = NULL; > > + > > +STATIC > > +EFI_STATUS > > +EFIAPI > > +AmdSevMemoryAccept ( > > + IN EFI_MEMORY_ACCEPT_PROTOCOL *This, > > + IN EFI_PHYSICAL_ADDRESS StartAddress, > > + IN UINTN Size > > +) > > +{ > > + MemEncryptSnpAcceptPages ( > > + StartAddress, > > + EFI_SIZE_TO_PAGES (Size) > > + ); > > + > > + return EFI_SUCCESS; > > +} > > + > > +STATIC EFI_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = { > > + AmdSevMemoryAccept > > +}; > > + > > EFI_STATUS > > EFIAPI > > AmdSevDxeEntryPoint ( > > @@ -147,6 +171,16 @@ AmdSevDxeEntryPoint ( > > } > > } > > > > + Status = gBS->InstallProtocolInterface ( > > + &mAmdSevDxeHandle, > > + &gEfiMemoryAcceptProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mMemoryAcceptProtocol > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "Install EfiMemoryAcceptProtocol failed.\n")); > > + } > > + > > // > > // If its SEV-SNP active guest then install the > > CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB. > > // It contains the location for both the Secrets and CPUID page. > > diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > > b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > > index 9acf860cf2..5ddddabc32 100644 > > --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > > +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf > > @@ -47,6 +47,9 @@ > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase > > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize > > > > +[Protocols] > > + gEfiMemoryAcceptProtocolGuid > > + > > [Guids] > > gConfidentialComputingSevSnpBlobGuid > > > > diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h > > b/OvmfPkg/Include/Library/MemEncryptSevLib.h > > index 4fa9c0d700..05ec10471d 100644 > > --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h > > +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h > > @@ -228,4 +228,18 @@ MemEncryptSevSnpPreValidateSystemRam ( > > IN UINTN NumPages > > ); > > > > +/** > > + Accept pages system RAM when SEV-SNP is enabled in the guest VM. > > + > > + @param[in] BaseAddress Base address > > + @param[in] NumPages Number of pages starting from the > > base address > > + > > +**/ > > +VOID > > +EFIAPI > > +MemEncryptSnpAcceptPages ( > > + IN PHYSICAL_ADDRESS BaseAddress, > > + IN UINTN NumPages > > + ); > > + > > #endif // _MEM_ENCRYPT_SEV_LIB_H_ > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > > b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > > index f92299fc77..f0747d792e 100644 > > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c > > @@ -153,3 +153,20 @@ MemEncryptSevSnpPreValidateSystemRam ( > > { > > ASSERT (FALSE); > > } > > + > > +/** > > + Accept pages system RAM when SEV-SNP is enabled in the guest VM. > > + > > + @param[in] BaseAddress Base address > > + @param[in] NumPages Number of pages starting from the > > base address > > + > > +**/ > > +VOID > > +EFIAPI > > +MemEncryptSnpAcceptPages ( > > + IN PHYSICAL_ADDRESS BaseAddress, > > + IN UINTN NumPages > > + ) > > +{ > > + ASSERT (FALSE); > > +} > > diff --git > > a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > > b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > > index d3a95e4913..7693e0ca66 100644 > > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c > > @@ -14,6 +14,7 @@ > > #include <Library/MemEncryptSevLib.h> > > > > #include "SnpPageStateChange.h" > > +#include "VirtualMemory.h" > > > > /** > > Pre-validate the system RAM when SEV-SNP is enabled in the guest VM. > > @@ -38,3 +39,37 @@ MemEncryptSevSnpPreValidateSystemRam ( > > // > > ASSERT (FALSE); > > } > > + > > +/** > > + Accept pages system RAM when SEV-SNP is enabled in the guest VM. > > + > > + @param[in] BaseAddress Base address > > + @param[in] NumPages Number of pages starting from the > > base address > > + > > +**/ > > +VOID > > +EFIAPI > > +MemEncryptSnpAcceptPages ( > > + IN PHYSICAL_ADDRESS BaseAddress, > > + IN UINTN NumPages > > + ) > > +{ > > + EFI_STATUS Status; > > + > > + if (!MemEncryptSevSnpIsEnabled ()) { > > + return; > > + } > > + if (BaseAddress >= SIZE_4GB) { > > + Status = InternalMemEncryptSevCreateIdentityMap1G ( > > + 0, > > + BaseAddress, > > + EFI_PAGES_TO_SIZE (NumPages) > > + ); > > + if (EFI_ERROR (Status)) { > > + ASSERT (FALSE); > > + CpuDeadLoop (); > > + } > > + } > > + > > + InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE); > > +} > > diff --git > > a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c > > b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c > > index 4970165444..1c52bfe691 100644 > > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c > > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c > > @@ -126,3 +126,20 @@ MemEncryptSevSnpPreValidateSystemRam ( > > BaseAddress = EndAddress; > > } > > } > > + > > +/** > > + Accept pages system RAM when SEV-SNP is enabled in the guest VM. > > + > > + @param[in] BaseAddress Base address > > + @param[in] NumPages Number of pages starting from the > > base address > > + > > +**/ > > +VOID > > +EFIAPI > > +MemEncryptSnpAcceptPages ( > > + IN PHYSICAL_ADDRESS BaseAddress, > > + IN UINTN NumPages > > + ) > > +{ > > + ASSERT (FALSE); > > +} > > diff --git > > a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c > > b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c > > index 7797febb8a..edfebf6ef4 100644 > > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c > > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c > > @@ -10,6 +10,7 @@ > > > > #include <Uefi/UefiBaseType.h> > > #include <Library/BaseLib.h> > > +#include <Library/DebugLib.h> > > #include <Library/MemEncryptSevLib.h> > > > > #include "SnpPageStateChange.h" > > @@ -80,3 +81,20 @@ MemEncryptSevSnpPreValidateSystemRam ( > > > > InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE); > > } > > + > > +/** > > + Accept pages system RAM when SEV-SNP is enabled in the guest VM. > > + > > + @param[in] BaseAddress Base address > > + @param[in] NumPages Number of pages starting from the > > base address > > + > > +**/ > > +VOID > > +EFIAPI > > +MemEncryptSnpAcceptPages ( > > + IN PHYSICAL_ADDRESS BaseAddress, > > + IN UINTN NumPages > > + ) > > +{ > > + ASSERT(FALSE); > > +} -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#94346): https://edk2.groups.io/g/devel/message/94346 Mute This Topic: https://groups.io/mt/93879404/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-