In order to allow the VMM (such as QEMU) to add a page with hashes of
kernel/initrd/cmdline for measured direct boot on SNP, this page must
not be part of the SNP metadata list reported to the VMM.
Check if that page is defined; if it is, skip it in the metadata list.
In such case, VMM should fill the page with the hashes content, or
explicitly update it as a zero page (if kernel hashes are not used).
Note that for SNP, the launch secret part of the page (lower 3KB) are
not relevant and will stay zero. The last 1KB is used for the hashes.
This should have no effect on OvmfPkgX64 targets (which don't define
PcdSevLaunchSecretBase).
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
OvmfPkg/ResetVector/ResetVector.nasmb | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 94fbb0a87b37..16f3daf49d82 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -75,7 +75,19 @@
;
%define SNP_SEC_MEM_BASE_DESC_2 (GHCB_BASE + 0x1000)
%define SNP_SEC_MEM_SIZE_DESC_2 (SEV_SNP_SECRETS_BASE - SNP_SEC_MEM_BASE_DESC_2)
-%define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE)
+%if (FixedPcdGet32 (PcdSevLaunchSecretBase) > 0)
+ ; There's a reserved page for SEV secrets and hashes; the VMM will fill and
+ ; validate the page, or mark it as a zero page.
+ %define EXPECTED_END_OF_LAUNCH_SECRET_PAGE (FixedPcdGet32 (PcdSevLaunchSecretBase) + \
+ FixedPcdGet32 (PcdSevLaunchSecretSize) + \
+ FixedPcdGet32 (PcdQemuHashTableSize))
+ %if (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) != EXPECTED_END_OF_LAUNCH_SECRET_PAGE)
+ %error "PcdOvmfSecPeiTempRamBase must start directly after the SEV Launch Secret page"
+ %endif
+ %define SNP_SEC_MEM_BASE_DESC_3 (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase))
+%else
+ %define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE)
+%endif
%define SNP_SEC_MEM_SIZE_DESC_3 (FixedPcdGet32 (PcdOvmfPeiMemFvBase) - SNP_SEC_MEM_BASE_DESC_3)
%ifdef ARCH_X64
--
2.25.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100364): https://edk2.groups.io/g/devel/message/100364
Mute This Topic: https://groups.io/mt/97082683/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-On 2/20/23 02:49, Dov Murik wrote: > In order to allow the VMM (such as QEMU) to add a page with hashes of > kernel/initrd/cmdline for measured direct boot on SNP, this page must > not be part of the SNP metadata list reported to the VMM. > > Check if that page is defined; if it is, skip it in the metadata list. > In such case, VMM should fill the page with the hashes content, or > explicitly update it as a zero page (if kernel hashes are not used). Would it be better to define a new section type (similar to what I did in the SVSM PoC)? This way, it remains listed in the metadata and allows the VMM to detect it and decide how to handle it. Thanks, Tom > > Note that for SNP, the launch secret part of the page (lower 3KB) are > not relevant and will stay zero. The last 1KB is used for the hashes. > > This should have no effect on OvmfPkgX64 targets (which don't define > PcdSevLaunchSecretBase). > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > --- > OvmfPkg/ResetVector/ResetVector.nasmb | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb > index 94fbb0a87b37..16f3daf49d82 100644 > --- a/OvmfPkg/ResetVector/ResetVector.nasmb > +++ b/OvmfPkg/ResetVector/ResetVector.nasmb > @@ -75,7 +75,19 @@ > ; > %define SNP_SEC_MEM_BASE_DESC_2 (GHCB_BASE + 0x1000) > %define SNP_SEC_MEM_SIZE_DESC_2 (SEV_SNP_SECRETS_BASE - SNP_SEC_MEM_BASE_DESC_2) > -%define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE) > +%if (FixedPcdGet32 (PcdSevLaunchSecretBase) > 0) > + ; There's a reserved page for SEV secrets and hashes; the VMM will fill and > + ; validate the page, or mark it as a zero page. > + %define EXPECTED_END_OF_LAUNCH_SECRET_PAGE (FixedPcdGet32 (PcdSevLaunchSecretBase) + \ > + FixedPcdGet32 (PcdSevLaunchSecretSize) + \ > + FixedPcdGet32 (PcdQemuHashTableSize)) > + %if (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) != EXPECTED_END_OF_LAUNCH_SECRET_PAGE) > + %error "PcdOvmfSecPeiTempRamBase must start directly after the SEV Launch Secret page" > + %endif > + %define SNP_SEC_MEM_BASE_DESC_3 (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase)) > +%else > + %define SNP_SEC_MEM_BASE_DESC_3 (CPUID_BASE + CPUID_SIZE) > +%endif > %define SNP_SEC_MEM_SIZE_DESC_3 (FixedPcdGet32 (PcdOvmfPeiMemFvBase) - SNP_SEC_MEM_BASE_DESC_3) > > %ifdef ARCH_X64 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100370): https://edk2.groups.io/g/devel/message/100370 Mute This Topic: https://groups.io/mt/97082683/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Mon, Feb 20, 2023 at 08:44:23AM -0600, Tom Lendacky wrote: > On 2/20/23 02:49, Dov Murik wrote: > > In order to allow the VMM (such as QEMU) to add a page with hashes of > > kernel/initrd/cmdline for measured direct boot on SNP, this page must > > not be part of the SNP metadata list reported to the VMM. > > > > Check if that page is defined; if it is, skip it in the metadata list. > > In such case, VMM should fill the page with the hashes content, or > > explicitly update it as a zero page (if kernel hashes are not used). > > Would it be better to define a new section type (similar to what I did in > the SVSM PoC)? This way, it remains listed in the metadata and allows the > VMM to detect it and decide how to handle it. Explicitly describing things sounds better to me too. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100410): https://edk2.groups.io/g/devel/message/100410 Mute This Topic: https://groups.io/mt/97082683/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 21/02/2023 11:38, Gerd Hoffmann wrote: > On Mon, Feb 20, 2023 at 08:44:23AM -0600, Tom Lendacky wrote: >> On 2/20/23 02:49, Dov Murik wrote: >>> In order to allow the VMM (such as QEMU) to add a page with hashes of >>> kernel/initrd/cmdline for measured direct boot on SNP, this page must >>> not be part of the SNP metadata list reported to the VMM. >>> >>> Check if that page is defined; if it is, skip it in the metadata list. >>> In such case, VMM should fill the page with the hashes content, or >>> explicitly update it as a zero page (if kernel hashes are not used). >> >> Would it be better to define a new section type (similar to what I did in >> the SVSM PoC)? This way, it remains listed in the metadata and allows the >> VMM to detect it and decide how to handle it. > > Explicitly describing things sounds better to me too. > Thanks for the feedback Tom and Gerd. I can define a new section type OVMF_SECTION_TYPE_KERNEL_HASHES. In the AmdSev target it'll cover the single MEMFD page at 00F000 (after the CPUID page). Now there's a question for the QEMU side -- should QEMU then fill the page and encrypt it (launch_update type=NORMAL)? (currently the whole hashes table creation and encryption is done elsewhere there) And on regular OvmfX64 builds - should that area should be with type OVMF_SECTION_TYPE_SNP_SEC_MEM which is accepted as a type=ZERO page ? Playing with this idea, the metadata list will add: ; Kernel hashes section for measured direct boot %define OVMF_SECTION_TYPE_KERNEL_HASHES 0x5 ... ; Kernel hashes for measured direct boot, or zero page if ; there are no kernel hashes / SEV secrets SevSnpKernelHashes: DD SEV_SNP_KERNEL_HASHES_BASE DD SEV_SNP_KERNEL_HASHES_SIZE DD SEV_SNP_KERNEL_HASHES_TYPE and the base/size/type of that region are defined in an %if statement in ResetVector.nasmb: %if (FixedPcdGet32 (PcdSevLaunchSecretBase) > 0) ; There's a reserved page for SEV secrets and hashes; the VMM will fill and ; validate the page %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_KERNEL_HASHES %define SEV_SNP_KERNEL_HASHES_BASE (FixedPcdGet32 (PcdSevLaunchSecretBase)) %else ; No SEV secrets and hashes page; the VMM will validate it as another zero page %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_SNP_SEC_MEM %define SEV_SNP_KERNEL_HASHES_BASE (CPUID_BASE + CPUID_SIZE) %endif %define SEV_SNP_KERNEL_HASHES_SIZE (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) - SEV_SNP_KERNEL_HASHES_BASE) (I still need to figure out the point about QEMU above.) Is that what you had in mind? -Dov -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100456): https://edk2.groups.io/g/devel/message/100456 Mute This Topic: https://groups.io/mt/97082683/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 23/02/2023 16:58, Dov Murik wrote: > > > On 21/02/2023 11:38, Gerd Hoffmann wrote: >> On Mon, Feb 20, 2023 at 08:44:23AM -0600, Tom Lendacky wrote: >>> On 2/20/23 02:49, Dov Murik wrote: >>>> In order to allow the VMM (such as QEMU) to add a page with hashes of >>>> kernel/initrd/cmdline for measured direct boot on SNP, this page must >>>> not be part of the SNP metadata list reported to the VMM. >>>> >>>> Check if that page is defined; if it is, skip it in the metadata list. >>>> In such case, VMM should fill the page with the hashes content, or >>>> explicitly update it as a zero page (if kernel hashes are not used). >>> >>> Would it be better to define a new section type (similar to what I did in >>> the SVSM PoC)? This way, it remains listed in the metadata and allows the >>> VMM to detect it and decide how to handle it. >> >> Explicitly describing things sounds better to me too. >> > > Thanks for the feedback Tom and Gerd. > > > I can define a new section type OVMF_SECTION_TYPE_KERNEL_HASHES. In the AmdSev > target it'll cover the single MEMFD page at 00F000 (after the CPUID page). > Now there's a question for the QEMU side -- should QEMU then fill the page > and encrypt it (launch_update type=NORMAL)? (currently the whole hashes table > creation and encryption is done elsewhere there) > > And on regular OvmfX64 builds - should that area should be with type > OVMF_SECTION_TYPE_SNP_SEC_MEM which is accepted as a type=ZERO page ? > > > Playing with this idea, the metadata list will add: > > > ; Kernel hashes section for measured direct boot > %define OVMF_SECTION_TYPE_KERNEL_HASHES 0x5 > > ... > > ; Kernel hashes for measured direct boot, or zero page if > ; there are no kernel hashes / SEV secrets > SevSnpKernelHashes: > DD SEV_SNP_KERNEL_HASHES_BASE > DD SEV_SNP_KERNEL_HASHES_SIZE > DD SEV_SNP_KERNEL_HASHES_TYPE > Or maybe this metadata section ^^^^^ should be added only if the Pcd for secrets+hashes page is defined? -Dov > > > and the base/size/type of that region are defined in an > %if statement in ResetVector.nasmb: > > > %if (FixedPcdGet32 (PcdSevLaunchSecretBase) > 0) > ; There's a reserved page for SEV secrets and hashes; the VMM will fill and > ; validate the page > %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_KERNEL_HASHES > %define SEV_SNP_KERNEL_HASHES_BASE (FixedPcdGet32 (PcdSevLaunchSecretBase)) > %else > ; No SEV secrets and hashes page; the VMM will validate it as another zero page > %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_SNP_SEC_MEM > %define SEV_SNP_KERNEL_HASHES_BASE (CPUID_BASE + CPUID_SIZE) > %endif > %define SEV_SNP_KERNEL_HASHES_SIZE (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) - SEV_SNP_KERNEL_HASHES_BASE) > > > (I still need to figure out the point about QEMU above.) > > > Is that what you had in mind? > > -Dov -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100457): https://edk2.groups.io/g/devel/message/100457 Mute This Topic: https://groups.io/mt/97082683/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2/23/23 09:04, Dov Murik wrote: > > > On 23/02/2023 16:58, Dov Murik wrote: >> >> >> On 21/02/2023 11:38, Gerd Hoffmann wrote: >>> On Mon, Feb 20, 2023 at 08:44:23AM -0600, Tom Lendacky wrote: >>>> On 2/20/23 02:49, Dov Murik wrote: >>>>> In order to allow the VMM (such as QEMU) to add a page with hashes of >>>>> kernel/initrd/cmdline for measured direct boot on SNP, this page must >>>>> not be part of the SNP metadata list reported to the VMM. >>>>> >>>>> Check if that page is defined; if it is, skip it in the metadata list. >>>>> In such case, VMM should fill the page with the hashes content, or >>>>> explicitly update it as a zero page (if kernel hashes are not used). >>>> >>>> Would it be better to define a new section type (similar to what I did in >>>> the SVSM PoC)? This way, it remains listed in the metadata and allows the >>>> VMM to detect it and decide how to handle it. >>> >>> Explicitly describing things sounds better to me too. >>> >> >> Thanks for the feedback Tom and Gerd. >> >> >> I can define a new section type OVMF_SECTION_TYPE_KERNEL_HASHES. In the AmdSev >> target it'll cover the single MEMFD page at 00F000 (after the CPUID page). >> Now there's a question for the QEMU side -- should QEMU then fill the page >> and encrypt it (launch_update type=NORMAL)? (currently the whole hashes table >> creation and encryption is done elsewhere there) Yes, I think that is the way to go. Allocate a page in Qemu, zero it out, fill in the hash values at the proper location and then do a launch update for type NORMAL page. You can use the section type to identify the data you need to retrieve and encrypt. >> >> And on regular OvmfX64 builds - should that area should be with type >> OVMF_SECTION_TYPE_SNP_SEC_MEM which is accepted as a type=ZERO page ? >> >> >> Playing with this idea, the metadata list will add: >> >> >> ; Kernel hashes section for measured direct boot >> %define OVMF_SECTION_TYPE_KERNEL_HASHES 0x5 >> >> ... >> >> ; Kernel hashes for measured direct boot, or zero page if >> ; there are no kernel hashes / SEV secrets >> SevSnpKernelHashes: >> DD SEV_SNP_KERNEL_HASHES_BASE >> DD SEV_SNP_KERNEL_HASHES_SIZE >> DD SEV_SNP_KERNEL_HASHES_TYPE >> > > Or maybe this metadata section ^^^^^ should be added only if the Pcd for > secrets+hashes page is defined? That would be optimal if you could do that. Thanks, Tom > > -Dov > >> >> >> and the base/size/type of that region are defined in an >> %if statement in ResetVector.nasmb: >> >> >> %if (FixedPcdGet32 (PcdSevLaunchSecretBase) > 0) >> ; There's a reserved page for SEV secrets and hashes; the VMM will fill and >> ; validate the page >> %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_KERNEL_HASHES >> %define SEV_SNP_KERNEL_HASHES_BASE (FixedPcdGet32 (PcdSevLaunchSecretBase)) >> %else >> ; No SEV secrets and hashes page; the VMM will validate it as another zero page >> %define SEV_SNP_KERNEL_HASHES_TYPE OVMF_SECTION_TYPE_SNP_SEC_MEM >> %define SEV_SNP_KERNEL_HASHES_BASE (CPUID_BASE + CPUID_SIZE) >> %endif >> %define SEV_SNP_KERNEL_HASHES_SIZE (FixedPcdGet32 (PcdOvmfSecPeiTempRamBase) - SEV_SNP_KERNEL_HASHES_BASE) >> >> >> (I still need to figure out the point about QEMU above.) >> >> >> Is that what you had in mind? >> >> -Dov -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#100520): https://edk2.groups.io/g/devel/message/100520 Mute This Topic: https://groups.io/mt/97082683/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.