From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by mx.groups.io with SMTP id smtpd.web12.30416.1664207794477968497 for ; Mon, 26 Sep 2022 08:56:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@google.com header.s=20210112 header.b=gOzRqRTU; spf=pass (domain: google.com, ip: 209.85.218.52, mailfrom: dionnaglaze@google.com) Received: by mail-ej1-f52.google.com with SMTP id r18so15011951eja.11 for ; Mon, 26 Sep 2022 08:56:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=IYLQDOi/+s7OrvvSuQHF8+OKcInQqx8l//ia0P6STN4=; b=gOzRqRTU/ETaIjUMewNtYxiK3eEH5p02gREFni09ajSnxO54kUAhR3adYiqJq/g/Lj IyUoxQYuYJJYNJGwaJIIdy+d7ZwUfjmrRmMWo0eBmUIu66N5Grp4KiIgX8+xeEdEo5Jv 33dfQmHND5UVeW920LOopsGggrjvMJJU/9gWsk0x78sf/VD5/CQBCCP3cKnTm477gFSs VaSSu151lRVOcvn118VBsASZrn4M1Tvo5pfUQMkYpGYsM5pXWmXbWrQjqCBbOt/2fUqE 0XHPlAacEYYLQydSkJvlOn9u4wBMBC/H9aH+0nktpA3TTZldCqC1OAZ4Bd689NblggzC 85jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=IYLQDOi/+s7OrvvSuQHF8+OKcInQqx8l//ia0P6STN4=; b=SX6Q5s1nPA4gOoESiSeg4SDqJfhoeOJGFA3kHcdiN0EYSRiqzKx+WVpqHaLvpy47YR z5bFAASHUYy8A4HErTYelXfz5ZUV1GidppiBeXwCqn62D6hC9G/sNXrEJYwMhoo2EogL /6Kuc6938Ww9/zXnJh9h+aGN3VqX9J/kFvknvP+uE10+fxhyndJkvX+A2mxi2G4RFiO7 NFBBd+JVRDvWa+UgOtpTM6m2yMoJ5WI8DxQA/WrLkZLQPgtTmuX+oIZg5p5aX6oxeqFa N9TWPZAhuLmbpjDm+adyWlEO28gaLsly/Eju+9pV06KcYY1kGYLvpIB8N69Qwp6SdYg5 n1Fw== X-Gm-Message-State: ACrzQf3kNo+qmJ0afR46JQRaENo9KR5caR5YgEYuprqOuSo1HjBDTHJD vwIaWuMXhtWLFM5ZLh3JeqQ4FBPYnY5N0BTTC2394Q== X-Google-Smtp-Source: AMsMyM6rtmMY3nPO72lNrKfsTNZA+BxZ4CXxhbPJGRZk+q0oX3jLBgMEldZWGV+efLNwOWvcsJ0pTwn1AY1LnSXfjew= X-Received: by 2002:a17:907:8691:b0:783:645d:a4aa with SMTP id qa17-20020a170907869100b00783645da4aamr6048909ejc.473.1664207792798; Mon, 26 Sep 2022 08:56:32 -0700 (PDT) MIME-Version: 1.0 References: <20220923203431.1428535-1-dionnaglaze@google.com> <20220923203431.1428535-2-dionnaglaze@google.com> In-Reply-To: From: "Dionna Glaze" Date: Mon, 26 Sep 2022 08:56:21 -0700 Message-ID: Subject: Re: [PATCH 1/4] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe To: Tom Lendacky Cc: devel@edk2.groups.io, Gerd Hoffmann , James Bottomley , Jiewen Yao , Sophia Wolf Content-Type: text/plain; charset="UTF-8" > > 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)