From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by mx.groups.io with SMTP id smtpd.web10.1408.1664216062983678251 for ; Mon, 26 Sep 2022 11:14:23 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@bsdio.com header.s=fm3 header.b=nprOKC8K; spf=pass (domain: bsdio.com, ip: 66.111.4.28, mailfrom: rebecca@bsdio.com) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id F1D055C0197; Mon, 26 Sep 2022 14:14:21 -0400 (EDT) Received: from imap43 ([10.202.2.93]) by compute5.internal (MEProxy); Mon, 26 Sep 2022 14:14:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdio.com; h=cc :cc:content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1664216061; x=1664302461; bh=RV5lHjz+pR qyOXWBGqT6LVhULnBN7MpS11gUe8s2NBU=; b=nprOKC8K1vQlG+bVOT8O5qwtAd Po+UzhV2D/ZQDhiQdZXQWcsPFi0Czuf7OlrdPLtAmI9dZxTP5F4vHYY6Dx5mnb7X 4F6XJkmUA6dxX4H8V5jLIZhUcF0UKP0pnh00BSP4gbJXIdzucqgpbdz+snKgkAFD TjDtIPvn2xnacKo+ve7UHZDCtNc1QwtfY2AKiHwbm7OCpZV204U3tfm5CYaoct+1 COZ7Xfqfe+p2opzmanPVQ8a+C9VmF5JoT5/v8z2u3XEAZQAEmCKGgH+9g3DfmGXX C+fbZ9ceEVeF5ZPMfUU7xNqDCGLArfDZ/nsrnyT93mDPpA+dc5qds/xmHTjQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; t=1664216061; x=1664302461; bh=RV5lHjz+pRqyOXWBGqT6LVhULnBN 7MpS11gUe8s2NBU=; b=bAWXsN5qs3oXoHAOhIwwsVZdm8HugI+u4dCGYZmaux/3 Nji8aY4oKKzkOzWt5ssDE7jZZCpeFgrl5XZo8CRMhse74+XN0uQxkY918jLw5Z6A sEPZ76uutiw3jLkYPHDi6Dj/UDUzgNXHupPreDLEUQIqrvLIijDFQxZgLpVNEUwF GHNMQK1/k2QpFni1eXrbYQqUHslRByBjlTsgFtRyAZSv6vDNsaYm3WZScLI3nhcE UN4Tgomh4uuEekirlopaUKZGhnbBtNLZMbNMSqtNxh5/sAt2MWCJi2CxXPf5bbIF 7oSfonEmuZR30lHXGrXW6ZhLJBtDhAFpfz8I0God+g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeegvddguddvvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefofgggkfgjfhffhffvvefutgesthdtredtreertdenucfhrhhomhepfdft vggsvggttggrucevrhgrnhdfuceorhgvsggvtggtrgessghsughiohdrtghomheqnecugg ftrfgrthhtvghrnhepvdekieeivdeijeekkefgteehteeiieefgfevveeufeekheekieff vdekgeeifefgnecuffhomhgrihhnpehgrhhouhhpshdrihhonecuvehluhhsthgvrhfuih iivgeptdenucfrrghrrghmpehmrghilhhfrhhomheprhgvsggvtggtrgessghsughiohdr tghomh X-ME-Proxy: Feedback-ID: i5b994698:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id A2CD42D40071; Mon, 26 Sep 2022 14:14:21 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.7.0-alpha0-968-g04df58079d-fm-20220921.001-g04df5807 Mime-Version: 1.0 Message-Id: In-Reply-To: References: <20220923203431.1428535-1-dionnaglaze@google.com> <20220923203431.1428535-2-dionnaglaze@google.com> Date: Mon, 26 Sep 2022 11:14:19 -0700 From: "Rebecca Cran" To: devel@edk2.groups.io, dionnaglaze@google.com, "Tom Lendacky" Cc: "Gerd Hoffmann" , "James Bottomley" , "Jiewen Yao" , "Sophia Wolf" Subject: Re: [edk2-devel] [PATCH 1/4] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Content-Type: text/plain You should use -v2 -v3 -v4 etc. to specify the patch version. On Mon, Sep 26, 2022, at 8:56 AM, Dionna Glaze via groups.io wrote: >> >> 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 >> > Cc: James Bottomley >> > Cc: Jiewen Yao >> > Cc: Tom Lendacky >> > >> > Signed-off-by: Sophia Wolf >> > --- >> > 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 >> > #include >> > #include >> > +#include >> > >> > 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 >> > >> > #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 >> > #include >> > +#include >> > #include >> > >> > #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) > > >