OvmfPkg/AmdSevDxe/AmdSevDxe.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
The hard-coded attributes for the re-added memory space should instead
forward the replaced descriptor's capabilities, plus the
EFI_MEMORY_CPU_CRYPTO attribute.
Tested on Linux with efi=debug. Prior to this change, an 8GiB VM running
a kernel without unaccepted memory support shows this entry
efi: mem94: [Conventional| | |CC| | | | | | | | | | | ]
range=[0x0000000100000000-0x000000023fffffff] (5120MB)
This does not have the cache capabilities one would expect for system
memory, UC|WC|WT|WB.
After this change, the same entry becomes
efi: mem94: [Conventional| | |CC| | | | | | | |WB|WT|WC|UC]
range=[0x0000000100000000-0x000000023fffffff] (5120MB)
This has all the expected attributes.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 6391d1f775..59d5ff759f 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -23,6 +23,10 @@
#include <Pi/PrePiDxeCis.h>
#include <Protocol/SevMemoryAcceptance.h>
#include <Protocol/MemoryAccept.h>
+#include <Uefi/UefiSpec.h>
+
+// Present, initialized, tested bits defined in MdeModulePkg/Core/Dxe/DxeMain.h
+#define EFI_MEMORY_INTERNAL_MASK 0x0700000000000000ULL
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A', 'M', 'D', 'E'),
@@ -78,6 +82,7 @@ AcceptAllMemory (
UINTN NumEntries;
UINTN Index;
EFI_STATUS Status;
+ UINT64 Capabilities;
DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
@@ -112,11 +117,14 @@ AcceptAllMemory (
break;
}
+ Capabilities = EFI_MEMORY_CPU_CRYPTO | Desc->Capabilities;
Status = gDS->AddMemorySpace (
EfiGcdMemoryTypeSystemMemory,
Desc->BaseAddress,
Desc->Length,
- EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP
+ // Allocable system memory resource capabilities as masked
+ // in MdeModulePkg/Core/Dxe/Mem/Page.c:PromoteMemoryResource
+ Capabilities & ~(EFI_MEMORY_INTERNAL_MASK | EFI_MEMORY_RUNTIME)
);
if (EFI_ERROR (Status)) {
break;
--
2.39.1.456.gfc5497dd1b-goog
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99363): https://edk2.groups.io/g/devel/message/99363
Mute This Topic: https://groups.io/mt/96659595/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> > efi: mem94: [Conventional| | |CC| | | | | | | | | | | ] > range=[0x0000000100000000-0x000000023fffffff] (5120MB) > > This does not have the cache capabilities one would expect for system > memory, UC|WC|WT|WB. > > After this change, the same entry becomes > > efi: mem94: [Conventional| | |CC| | | | | | | |WB|WT|WC|UC] > range=[0x0000000100000000-0x000000023fffffff] (5120MB) > > This has all the expected attributes. > This change is made given a request from Ard. The CC capability is not applied to other system memory ranges that probably should also have that capability, given that it's encrypted and accepted. I haven't considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to conventional memory, given the acceptance happens before DXE initializes. Perhaps CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more of a question to Ard and Thomas. -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99365): https://edk2.groups.io/g/devel/message/99365 Mute This Topic: https://groups.io/mt/96659595/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Tue, 31 Jan 2023 at 20:18, Dionna Amalie Glaze <dionnaglaze@google.com> wrote: > > > > > efi: mem94: [Conventional| | |CC| | | | | | | | | | | ] > > range=[0x0000000100000000-0x000000023fffffff] (5120MB) > > > > This does not have the cache capabilities one would expect for system > > memory, UC|WC|WT|WB. > > > > After this change, the same entry becomes > > > > efi: mem94: [Conventional| | |CC| | | | | | | |WB|WT|WC|UC] > > range=[0x0000000100000000-0x000000023fffffff] (5120MB) > > > > This has all the expected attributes. > > > > This change is made given a request from Ard. The CC capability is not > applied to other system memory ranges that probably should also have > that capability, given that it's encrypted and accepted. I haven't > considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to > conventional memory, given the acceptance happens before DXE > initializes. Perhaps > CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more > of a question to Ard and Thomas. > It's not clear to me whether the CC attribute applies to the host or the guest. From the guest PoV, there is really no distinction, whereas on the host, I could imagine that only CC capable memory can be used for handing out to VMs. The important thing for this conversion is that - the type changes to ConventionalMemory - EFI_MEMORY_WB gets set (along with the other ones set in the capability), as otherwise, Linux will disregard this memory and not use it at all. The EFI_MEMORY_CPU_CRYPTO flag is currently only referenced by Linux in code that prints the memory map for diagnostic purposes, and not used anywhere else. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99405): https://edk2.groups.io/g/devel/message/99405 Mute This Topic: https://groups.io/mt/96659595/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> > > > This change is made given a request from Ard. The CC capability is not > > applied to other system memory ranges that probably should also have > > that capability, given that it's encrypted and accepted. I haven't > > considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to > > conventional memory, given the acceptance happens before DXE > > initializes. Perhaps > > CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more > > of a question to Ard and Thomas. > > > > It's not clear to me whether the CC attribute applies to the host or > the guest. From the guest PoV, there is really no distinction, whereas > on the host, I could imagine that only CC capable memory can be used > for handing out to VMs. > That's a good point. The UEFI spec language is hard to interpret here. Min or Jiewen, do you have more context on the EFI_MEMORY_CPU_CRYPTO attribute? -- -Dionna Glaze, PhD (she/her) -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99501): https://edk2.groups.io/g/devel/message/99501 Mute This Topic: https://groups.io/mt/96659595/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Thu, 2 Feb 2023 at 21:42, Dionna Amalie Glaze <dionnaglaze@google.com> wrote: > > > > > > > This change is made given a request from Ard. The CC capability is not > > > applied to other system memory ranges that probably should also have > > > that capability, given that it's encrypted and accepted. I haven't > > > considered carefully where EFI_MEMORY_CPU_CRYPTO should be added to > > > conventional memory, given the acceptance happens before DXE > > > initializes. Perhaps > > > CoreConvertResourceDescriptorHobAttributesToCapabilities? This is more > > > of a question to Ard and Thomas. > > > > > > > It's not clear to me whether the CC attribute applies to the host or > > the guest. From the guest PoV, there is really no distinction, whereas > > on the host, I could imagine that only CC capable memory can be used > > for handing out to VMs. > > > > That's a good point. The UEFI spec language is hard to interpret here. > Min or Jiewen, do you have more context on the EFI_MEMORY_CPU_CRYPTO > attribute? > To keep things moving, I've queued this up (as #4040) with the EFI_MEMORY_CPU_CRYPTO flag dropped. I don't think we need to add it here, given that EDK2 nor Linux ever set or test this flag anywhere else, but if this changes, we can add it back. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100196): https://edk2.groups.io/g/devel/message/100196 Mute This Topic: https://groups.io/mt/96659595/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.