Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/28/23 03:41, Gerd Hoffmann wrote:
Hi,
I'd have to dig much deeper to see if there's a way to identify whether a
VARS file was specified on the Qemu command line. I *think* (please correct
me if I'm missing something) for SEV and SEV-ES it would be straight forward
to try and access the memory as shared and check the headers. If they're
valid, then a VARS file was specified on the command line and should remain
mapped shared. If they aren't valid, a VARS file wasn't specified and you
have either the full OVMF.fd file or just the OVMF_CODE.fd with memory
backing the VARS that, in either case, should be mapped private.
OVMF_CODE.fd + OVMF_VARS.fd is *identical* to just OVMF.fd, i.e. the
guest will see valid varstore headers in both cases.
It is identical except in how they are mapped. With a split OVMF_CODE.fd /
OVMF_VARS.fd, the OVMF_CODE.fd file is mapped private and the OVMF_VARS.fd
is mapped shared because the hypervisor is updating the contents of
OVMF_VARS.fd. With OVMF.fd, the whole file is mapped private because
updates to the variables are not retained, so the hypervisor isn't
updating the contents.
I'll give the patch below a try in the next day or two.
Thanks,
Tom
The split into code part and vars part allows to (a) easily update the
code without screwing up the vars, and (b) map both with different
properties, i.e. code read-only and vars read/write.
Does the patch below help?
take care,
Gerd
From 3971f9453ded3032f5918dc9d181ecc0b6f97862 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Fri, 28 Apr 2023 10:34:23 +0200
Subject: [PATCH 1/1] [testing] try setup mmio in QemuFlashBeforeProbe (dxe)
---
.../QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
index d57f7ca25ccf..3a6280ab9c3a 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
@@ -37,9 +37,18 @@ QemuFlashBeforeProbe (
IN UINTN FdBlockCount
)
{
- //
- // Do nothing
- //
+ EFI_STATUS Status;
+
+ if (MemEncryptSevIsEnabled ()) {
+Status = MemEncryptSevClearMmioPageEncMask (
+ 0,
+ BaseAddress,
+ EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
+ );
+if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_WARN, "%a: MemEncryptSevClearMmioPageEncMask: %r\n",
__func__, Status));
+}
+ }
}
/**
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103824): https://edk2.groups.io/g/devel/message/103824
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi,
> I'd have to dig much deeper to see if there's a way to identify whether a
> VARS file was specified on the Qemu command line. I *think* (please correct
> me if I'm missing something) for SEV and SEV-ES it would be straight forward
> to try and access the memory as shared and check the headers. If they're
> valid, then a VARS file was specified on the command line and should remain
> mapped shared. If they aren't valid, a VARS file wasn't specified and you
> have either the full OVMF.fd file or just the OVMF_CODE.fd with memory
> backing the VARS that, in either case, should be mapped private.
OVMF_CODE.fd + OVMF_VARS.fd is *identical* to just OVMF.fd, i.e. the
guest will see valid varstore headers in both cases.
The split into code part and vars part allows to (a) easily update the
code without screwing up the vars, and (b) map both with different
properties, i.e. code read-only and vars read/write.
Does the patch below help?
take care,
Gerd
>From 3971f9453ded3032f5918dc9d181ecc0b6f97862 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Fri, 28 Apr 2023 10:34:23 +0200
Subject: [PATCH 1/1] [testing] try setup mmio in QemuFlashBeforeProbe (dxe)
---
.../QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
index d57f7ca25ccf..3a6280ab9c3a 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
@@ -37,9 +37,18 @@ QemuFlashBeforeProbe (
IN UINTN FdBlockCount
)
{
- //
- // Do nothing
- //
+ EFI_STATUS Status;
+
+ if (MemEncryptSevIsEnabled ()) {
+Status = MemEncryptSevClearMmioPageEncMask (
+ 0,
+ BaseAddress,
+ EFI_SIZE_TO_PAGES (FdBlockSize * FdBlockCount)
+ );
+if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_WARN, "%a: MemEncryptSevClearMmioPageEncMask: %r\n",
__func__, Status));
+}
+ }
}
/**
--
2.40.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103764): https://edk2.groups.io/g/devel/message/103764
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/24/23 04:45, Gerd Hoffmann wrote: On Fri, Apr 21, 2023 at 03:49:27PM -0500, Tom Lendacky wrote: On 4/21/23 04:18, Gerd Hoffmann wrote: Hmm, good question. Can the guest figure what memory ranges are part of the launch measurement? I have a patch here (attached below) which refines flash detection and can detect whenever varstore flash is writable or not. I suspect that doesn't help much though as flash probing requires mappings already being correct. Sorry for the delay, but, yeah, doesn't help. SEV and SEV-ES assert and SEV-SNP terminates because of accessing a shared page (in the RMP) as a private page (we don't support the generated 0x404 error code in the #VC handler). Can you try this? https://github.com/kraxel/edk2/commits/devel/secure-boot-pcd It works for the split vars/code launch, but fails for the combined vars/code launch: EMU Variable FVB Started EMU Variable FVB: Using pre-reserved block at 7FE7C000 EMU Variable FVB: Basic FV headers were invalid EMU Variable FVB: SecureBoot: restore FV from ROM EMU Variable FVB: Basic FV headers were invalid ASSERT [EmuVariableFvbRuntimeDxe] /root/kernels/ovmf-gerd-build-X64/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c(781): ((BOOLEAN)(0==1)) So the mapping isn't correct at this point either. Log looks like this for me, using grep -Ei '(fvb|flash|amdsev)' Loading driver at 0x0003F022000 EntryPoint=0x0003F0245B5 AmdSevDxe.efi Loading driver at 0x0003F6E4000 EntryPoint=0x0003F6E7035 FvbServicesRuntimeDxe.efi QEMU Flash: Attempting flash detection at FFC00010 QemuFlashDetected => FD behaves as ROM QemuFlashDetected => No QEMU flash was not detected. Writable FVB is not being installed. Loading driver at 0x0003F6D3000 EntryPoint=0x0003F6D55B9 EmuVariableFvbRuntimeDxe.efi EMU Variable FVB Started EMU Variable FVB: Using pre-reserved block at 3FEF4000 EMU Variable FVB: Basic FV headers were invalid Installing FVB for EMU Variable support So AmdSevDxe is loaded first, next comes FvbServicesRuntimeDxe, finally EmuVariableFvbRuntimeDxe. So AmdSev should have (in theory, in practice obviously not ...) setup everything at that point I assume? I'd have to dig much deeper to see if there's a way to identify whether a VARS file was specified on the Qemu command line. I *think* (please correct me if I'm missing something) for SEV and SEV-ES it would be straight forward to try and access the memory as shared and check the headers. If they're valid, then a VARS file was specified on the command line and should remain mapped shared. If they aren't valid, a VARS file wasn't specified and you have either the full OVMF.fd file or just the OVMF_CODE.fd with memory backing the VARS that, in either case, should be mapped private. I think the problem may come in with SNP where if the mapping isn't correct (shared mapping against a page that has been validated or a private mapping against a page that hasn't been validated) you can end up with #NPFs or #VCs and having to figure out what or why you are getting those. Let me see what I can find... I'm off the next few days so it might be a bit. Thanks, Tom Failing that FvbServicesRuntimeDxe might do it as well, there actually is some code doing so to fixup things after calling SetMemorySpaceAttributes (see MarkIoMemoryRangeForRuntimeAccess). Maybe that should also be called before QemuFlashInitialize() so the SEV settings are correct when OVMF goes do flash detection? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103665): https://edk2.groups.io/g/devel/message/103665 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Fri, Apr 21, 2023 at 03:49:27PM -0500, Tom Lendacky wrote: > On 4/21/23 04:18, Gerd Hoffmann wrote: > > > > Hmm, good question. Can the guest figure what memory ranges are part > > > > of the launch measurement? > > > > > > > > I have a patch here (attached below) which refines flash detection and > > > > can detect whenever varstore flash is writable or not. I suspect that > > > > doesn't help much though as flash probing requires mappings already > > > > being correct. > > > > > > Sorry for the delay, but, yeah, doesn't help. SEV and SEV-ES assert and > > > SEV-SNP terminates because of accessing a shared page (in the RMP) as a > > > private page (we don't support the generated 0x404 error code in the #VC > > > handler). > > > > Can you try this? > > https://github.com/kraxel/edk2/commits/devel/secure-boot-pcd > > It works for the split vars/code launch, but fails for the combined > vars/code launch: > > EMU Variable FVB Started > EMU Variable FVB: Using pre-reserved block at 7FE7C000 > EMU Variable FVB: Basic FV headers were invalid > EMU Variable FVB: SecureBoot: restore FV from ROM > EMU Variable FVB: Basic FV headers were invalid > ASSERT [EmuVariableFvbRuntimeDxe] > /root/kernels/ovmf-gerd-build-X64/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c(781): > ((BOOLEAN)(0==1)) > > So the mapping isn't correct at this point either. Log looks like this for me, using grep -Ei '(fvb|flash|amdsev)' Loading driver at 0x0003F022000 EntryPoint=0x0003F0245B5 AmdSevDxe.efi Loading driver at 0x0003F6E4000 EntryPoint=0x0003F6E7035 FvbServicesRuntimeDxe.efi QEMU Flash: Attempting flash detection at FFC00010 QemuFlashDetected => FD behaves as ROM QemuFlashDetected => No QEMU flash was not detected. Writable FVB is not being installed. Loading driver at 0x0003F6D3000 EntryPoint=0x0003F6D55B9 EmuVariableFvbRuntimeDxe.efi EMU Variable FVB Started EMU Variable FVB: Using pre-reserved block at 3FEF4000 EMU Variable FVB: Basic FV headers were invalid Installing FVB for EMU Variable support So AmdSevDxe is loaded first, next comes FvbServicesRuntimeDxe, finally EmuVariableFvbRuntimeDxe. So AmdSev should have (in theory, in practice obviously not ...) setup everything at that point I assume? Failing that FvbServicesRuntimeDxe might do it as well, there actually is some code doing so to fixup things after calling SetMemorySpaceAttributes (see MarkIoMemoryRangeForRuntimeAccess). Maybe that should also be called before QemuFlashInitialize() so the SEV settings are correct when OVMF goes do flash detection? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103462): https://edk2.groups.io/g/devel/message/103462 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/21/23 04:18, Gerd Hoffmann wrote: Hmm, good question. Can the guest figure what memory ranges are part of the launch measurement? I have a patch here (attached below) which refines flash detection and can detect whenever varstore flash is writable or not. I suspect that doesn't help much though as flash probing requires mappings already being correct. Sorry for the delay, but, yeah, doesn't help. SEV and SEV-ES assert and SEV-SNP terminates because of accessing a shared page (in the RMP) as a private page (we don't support the generated 0x404 error code in the #VC handler). Can you try this? https://github.com/kraxel/edk2/commits/devel/secure-boot-pcd It works for the split vars/code launch, but fails for the combined vars/code launch: EMU Variable FVB Started EMU Variable FVB: Using pre-reserved block at 7FE7C000 EMU Variable FVB: Basic FV headers were invalid EMU Variable FVB: SecureBoot: restore FV from ROM EMU Variable FVB: Basic FV headers were invalid ASSERT [EmuVariableFvbRuntimeDxe] /root/kernels/ovmf-gerd-build-X64/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c(781): ((BOOLEAN)(0==1)) So the mapping isn't correct at this point either. Thanks, Tom It moves the varstore copy from platform init to emu variable driver, which should be late enough that sev setup should be complete. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103432): https://edk2.groups.io/g/devel/message/103432 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
> > Hmm, good question. Can the guest figure what memory ranges are part > > of the launch measurement? > > > > I have a patch here (attached below) which refines flash detection and > > can detect whenever varstore flash is writable or not. I suspect that > > doesn't help much though as flash probing requires mappings already > > being correct. > > Sorry for the delay, but, yeah, doesn't help. SEV and SEV-ES assert and > SEV-SNP terminates because of accessing a shared page (in the RMP) as a > private page (we don't support the generated 0x404 error code in the #VC > handler). Can you try this? https://github.com/kraxel/edk2/commits/devel/secure-boot-pcd It moves the varstore copy from platform init to emu variable driver, which should be late enough that sev setup should be complete. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103398): https://edk2.groups.io/g/devel/message/103398 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/14/23 05:20, Gerd Hoffmann wrote:
Hi,
-drive
if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF_CODE.fd,readonly=on
-drive if=pflash,format=raw,unit=1,file=./fedora.fd
In this case, only OVMF_CODE.fd will be encrypted.
The fedora.fd (OVMF_VARS.fd) will be unencrypted.
-drive
if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF.fd,readonly=on
In this case, OVMF.fd will be encrypted, which includes the now memory
backed variable store.
Can AmdSevInitialize() setup the mappings?
Is there a way to tell when OVMF.fd vs OVMF_VARS.fd/OVMF_CODE.fd is used?
Hmm, good question. Can the guest figure what memory ranges are part
of the launch measurement?
I have a patch here (attached below) which refines flash detection and
can detect whenever varstore flash is writable or not. I suspect that
doesn't help much though as flash probing requires mappings already
being correct.
Sorry for the delay, but, yeah, doesn't help. SEV and SEV-ES assert and
SEV-SNP terminates because of accessing a shared page (in the RMP) as a
private page (we don't support the generated 0x404 error code in the #VC
handler).
Thanks,
Tom
take care,
Gerd
commit fdab276a9f8a25f505b083b5e15180d093f515e3
Author: Gerd Hoffmann
Date: Tue Apr 4 11:25:37 2023 +0200
OvmfPkg/QemuFlashFvbServicesRuntimeDxe: refine flash detection
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
index 82b2b70441bf..c088d560f829 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
@@ -118,8 +118,17 @@ QemuFlashDetected (
*Ptr = OriginalUint8;
} else if (ProbeUint8 == CLEARED_ARRAY_STATUS) {
DEBUG ((DEBUG_INFO, "QemuFlashDetected => FD behaves as FLASH\n"));
- FlashDetected = TRUE;
- *Ptr = READ_ARRAY_CMD;
+ *Ptr = WRITE_BYTE_CMD;
+ *Ptr = OriginalUint8;
+ *Ptr = READ_STATUS_CMD;
+ ProbeUint8 = *Ptr;
+ if (ProbeUint8 & 0x10 /* programming error */) {
+DEBUG ((DEBUG_INFO, "QemuFlashDetected => FLASH is readonly\n"));
+ } else {
+DEBUG ((DEBUG_INFO, "QemuFlashDetected => FLASH is writable\n"));
+FlashDetected = TRUE;
+ }
+ *Ptr = READ_ARRAY_CMD;
}
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103309): https://edk2.groups.io/g/devel/message/103309
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi,
>-drive
> if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF_CODE.fd,readonly=on
>-drive if=pflash,format=raw,unit=1,file=./fedora.fd
> In this case, only OVMF_CODE.fd will be encrypted.
> The fedora.fd (OVMF_VARS.fd) will be unencrypted.
>-drive
> if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF.fd,readonly=on
> In this case, OVMF.fd will be encrypted, which includes the now memory
> backed variable store.
> > Can AmdSevInitialize() setup the mappings?
>
> Is there a way to tell when OVMF.fd vs OVMF_VARS.fd/OVMF_CODE.fd is used?
Hmm, good question. Can the guest figure what memory ranges are part
of the launch measurement?
I have a patch here (attached below) which refines flash detection and
can detect whenever varstore flash is writable or not. I suspect that
doesn't help much though as flash probing requires mappings already
being correct.
take care,
Gerd
commit fdab276a9f8a25f505b083b5e15180d093f515e3
Author: Gerd Hoffmann
Date: Tue Apr 4 11:25:37 2023 +0200
OvmfPkg/QemuFlashFvbServicesRuntimeDxe: refine flash detection
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
index 82b2b70441bf..c088d560f829 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlash.c
@@ -118,8 +118,17 @@ QemuFlashDetected (
*Ptr = OriginalUint8;
} else if (ProbeUint8 == CLEARED_ARRAY_STATUS) {
DEBUG ((DEBUG_INFO, "QemuFlashDetected => FD behaves as FLASH\n"));
- FlashDetected = TRUE;
- *Ptr = READ_ARRAY_CMD;
+ *Ptr = WRITE_BYTE_CMD;
+ *Ptr = OriginalUint8;
+ *Ptr = READ_STATUS_CMD;
+ ProbeUint8 = *Ptr;
+ if (ProbeUint8 & 0x10 /* programming error */) {
+DEBUG ((DEBUG_INFO, "QemuFlashDetected => FLASH is readonly\n"));
+ } else {
+DEBUG ((DEBUG_INFO, "QemuFlashDetected => FLASH is writable\n"));
+FlashDetected = TRUE;
+ }
+ *Ptr = READ_ARRAY_CMD;
}
}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102994): https://edk2.groups.io/g/devel/message/102994
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/13/23 01:05, Gerd Hoffmann wrote: Hi, Specifying both OVMF_CODE.fd and OVMF_VARS.fd generates an ASSERT. Both as pflash I assume? Which assert? Yes, both as pflash. I've never attempted to run an SEV guest using the -bios option. The assert is: ASSERT [PlatformPei] /root/kernels/ovmf-build-X64/OvmfPkg/Library/PlatformInitLib/Platform.c(930): ((BOOLEAN)(0==1)) Ok, so wrong encryption settings. Specifying just OVMF.fd boots successfully pflash or -bios or both? Just pflash. /me looks surprised. It should not make a difference whenever you use the separate OVMF_CODE.fd + OVMF_VARS.fd files or the combined OVMF.fd. What are the exact qemu command lines for both cases? For the OVMF_CODE.fd/OVMF_VARS.fd case: qemu-system-x86_64 -enable-kvm -cpu EPYC,host-phys-bits=true -smp 1 -machine type=q35,confidential-guest-support=sev0,vmport=off -m 1G -object sev-guest,id=sev0,policy=0,cbitpos=51,reduced-phys-bits=1 -drive if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF_CODE.fd,readonly=on -drive if=pflash,format=raw,unit=1,file=./fedora.fd -drive file=./fedora.img,if=none,id=disk0,format=raw -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true -device scsi-hd,drive=disk0 -nographic -monitor pty -monitor unix:monitor,server,nowait -gdb tcp::1234 -qmp tcp::,server,wait=off In this case, only OVMF_CODE.fd will be encrypted. The fedora.fd (OVMF_VARS.fd) will be unencrypted. For the OVMF.fd case: qemu-system-x86_64 -enable-kvm -cpu EPYC,host-phys-bits=true -smp 1 -machine type=q35,confidential-guest-support=sev0,vmport=off -m 1G -object sev-guest,id=sev0,policy=0,cbitpos=51,reduced-phys-bits=1 -drive if=pflash,format=raw,unit=0,file=/root/kernels/qemu-install/OVMF.fd,readonly=on -drive file=./fedora.img,if=none,id=disk0,format=raw -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true -device scsi-hd,drive=disk0 -nographic -monitor pty -monitor unix:monitor,server,nowait -gdb tcp::1234 -qmp tcp::,server,wait=off In this case, OVMF.fd will be encrypted, which includes the now memory backed variable store. I believe none of the mappings are setup properly at this point. I think just eliminating the call for an SEV guest is fine. Can AmdSevInitialize() setup the mappings? Is there a way to tell when OVMF.fd vs OVMF_VARS.fd/OVMF_CODE.fd is used? The reason being that the variable store of OVMF.fd must be accessed encrypted since the whole binary was used in the LAUNCH_UPDATE. But with the split mode, only the OVMF_CODE.fd was encrypted in the LAUNCH_UPDATE, so the variable store must be accessed unencrypted. If we can make that determination, then I think we can setup the mappings. Thanks, Tom take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102950): https://edk2.groups.io/g/devel/message/102950 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi, > > > Specifying both OVMF_CODE.fd and OVMF_VARS.fd generates an ASSERT. > > > > Both as pflash I assume? Which assert? > > Yes, both as pflash. I've never attempted to run an SEV guest using the > -bios option. > > The assert is: > ASSERT [PlatformPei] > /root/kernels/ovmf-build-X64/OvmfPkg/Library/PlatformInitLib/Platform.c(930): > ((BOOLEAN)(0==1)) Ok, so wrong encryption settings. > > > Specifying just OVMF.fd boots successfully > > > > pflash or -bios or both? > > Just pflash. /me looks surprised. It should not make a difference whenever you use the separate OVMF_CODE.fd + OVMF_VARS.fd files or the combined OVMF.fd. What are the exact qemu command lines for both cases? > I believe none of the mappings are setup properly at this point. I > think just eliminating the call for an SEV guest is fine. Can AmdSevInitialize() setup the mappings? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102922): https://edk2.groups.io/g/devel/message/102922 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/12/23 02:24, Gerd Hoffmann wrote: On Tue, Apr 11, 2023 at 01:03:28PM -0500, Tom Lendacky wrote: On 4/11/23 05:04, Gerd Hoffmann wrote: On Fri, Apr 07, 2023 at 12:00:46PM -0500, Tom Lendacky wrote: Thanks for the quick turn-around, but that patch didn't work for me. I've update the bugzilla. Can you try the patch below? That doesn't work either. Specifying both OVMF_CODE.fd and OVMF_VARS.fd generates an ASSERT. Both as pflash I assume? Which assert? Yes, both as pflash. I've never attempted to run an SEV guest using the -bios option. The assert is: ASSERT [PlatformPei] /root/kernels/ovmf-build-X64/OvmfPkg/Library/PlatformInitLib/Platform.c(930): ((BOOLEAN)(0==1)) That happens for SEV and SEV-ES. For SEV-SNP, it causes a VMRUN failure with a strange exit code - but I believe it is because of accessing a page marked as shared in the RMP, but accessed as private by the guest. Specifying just OVMF_CODE.fd causes VMRUN failure (triple fault) That's not a valid configuration anyway. Right, but it has worked in the past. IIUC, it effectively ends up creating a memory based variable store. An SEV guest triple faults. An SEV-ES and SEV-SNP guest asserts: Invalid MMIO opcode (AF) ASSERT [SecMain] /root/kernels/ovmf-build-X64/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c(507): ((BOOLEAN)(0==1)) Specifying just OVMF.fd boots successfully pflash or -bios or both? Just pflash. We don't support running OVMF under SEV using the -bios option. If I try to run an SEV guest with -bios OVMF.fd, both SEV and SEV-ES hang, while SEV-SNP returns an -EFAULT on a launch update. I believe none of the mappings are setup properly at this point. I think just eliminating the call for an SEV guest is fine. Thanks, Tom For which cases does the patch change behavior? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102890): https://edk2.groups.io/g/devel/message/102890 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Tue, Apr 11, 2023 at 01:03:28PM -0500, Tom Lendacky wrote: > On 4/11/23 05:04, Gerd Hoffmann wrote: > > On Fri, Apr 07, 2023 at 12:00:46PM -0500, Tom Lendacky wrote: > > > > > > Thanks for the quick turn-around, but that patch didn't work for me. I've > > > update the bugzilla. > > > > Can you try the patch below? > > That doesn't work either. > > Specifying both OVMF_CODE.fd and OVMF_VARS.fd generates an ASSERT. Both as pflash I assume? Which assert? > Specifying just OVMF_CODE.fd causes VMRUN failure (triple fault) That's not a valid configuration anyway. > Specifying just OVMF.fd boots successfully pflash or -bios or both? For which cases does the patch change behavior? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102870): https://edk2.groups.io/g/devel/message/102870 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/11/23 05:04, Gerd Hoffmann wrote:
On Fri, Apr 07, 2023 at 12:00:46PM -0500, Tom Lendacky wrote:
Thanks for the quick turn-around, but that patch didn't work for me. I've
update the bugzilla.
Can you try the patch below?
That doesn't work either.
Specifying both OVMF_CODE.fd and OVMF_VARS.fd generates an ASSERT.
Specifying just OVMF_CODE.fd causes VMRUN failure (triple fault)
Specifying just OVMF.fd boots successfully
Thanks,
Tom
thanks,
Gerd
From a9179864523d12c3dcc137f36f6ed1a2832ed22c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Tue, 11 Apr 2023 11:12:37 +0200
Subject: [PATCH 1/1] OvmfPkg: call ReserveEmuVariableNvStore after
AmdSevInitialize
Signed-off-by: Gerd Hoffmann
---
OvmfPkg/PlatformPei/Platform.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index c56247e294f2..1e70c1920830 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -378,10 +378,6 @@ InitializePlatform (
InitializeRamRegions (PlatformInfoHob);
if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) {
-if (!PlatformInfoHob->SmmSmramRequire) {
- ReserveEmuVariableNvStore ();
-}
-
PeiFvInitialization (PlatformInfoHob);
MemTypeInfoInitialization (PlatformInfoHob);
MemMapInitialization (PlatformInfoHob);
@@ -390,6 +386,12 @@ InitializePlatform (
InstallClearCacheCallback ();
AmdSevInitialize (PlatformInfoHob);
+
+ if ((PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) &&
+ (!PlatformInfoHob->SmmSmramRequire)) {
+ReserveEmuVariableNvStore ();
+ }
+
if (PlatformInfoHob->HostBridgeDevId == 0x) {
MiscInitializationForMicrovm (PlatformInfoHob);
} else {
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102848): https://edk2.groups.io/g/devel/message/102848
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Fri, Apr 07, 2023 at 12:00:46PM -0500, Tom Lendacky wrote:
>
> Thanks for the quick turn-around, but that patch didn't work for me. I've
> update the bugzilla.
Can you try the patch below?
thanks,
Gerd
>From a9179864523d12c3dcc137f36f6ed1a2832ed22c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Tue, 11 Apr 2023 11:12:37 +0200
Subject: [PATCH 1/1] OvmfPkg: call ReserveEmuVariableNvStore after
AmdSevInitialize
Signed-off-by: Gerd Hoffmann
---
OvmfPkg/PlatformPei/Platform.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index c56247e294f2..1e70c1920830 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -378,10 +378,6 @@ InitializePlatform (
InitializeRamRegions (PlatformInfoHob);
if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) {
-if (!PlatformInfoHob->SmmSmramRequire) {
- ReserveEmuVariableNvStore ();
-}
-
PeiFvInitialization (PlatformInfoHob);
MemTypeInfoInitialization (PlatformInfoHob);
MemMapInitialization (PlatformInfoHob);
@@ -390,6 +386,12 @@ InitializePlatform (
InstallClearCacheCallback ();
AmdSevInitialize (PlatformInfoHob);
+
+ if ((PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) &&
+ (!PlatformInfoHob->SmmSmramRequire)) {
+ReserveEmuVariableNvStore ();
+ }
+
if (PlatformInfoHob->HostBridgeDevId == 0x) {
MiscInitializationForMicrovm (PlatformInfoHob);
} else {
--
2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102822): https://edk2.groups.io/g/devel/message/102822
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/6/23 20:56, Xu, Min M wrote: On Friday, April 7, 2023 4:29 AM, Tom Lendacky wrote: On 4/5/23 20:42, Xu, Min M wrote: On April 3, 2023 7:21 PM, Gerd Hoffmann wrote: I agree that the efi variable store is not secure without smm. But after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work with SEV. System just hangs in "NvVarStore FV headers were invalid." Hi, Joeyli ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped and an error code is returned. So system will not hang. So another solution is simply remove the ASSERT. Then an error message is dumped out and system continues. @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? Maybe we just need to call ReserveEmuVariableNvStore a bit later? I think we can still call ReserveEmuVariableNvStore at PEI phase, but move the initialization of EmuVariableNvStore to https://github.com/tianocore/edk2/blob/master/OvmfPkg/EmuVariableFvbR u ntimeDxe/Fvb.c#L780-L783 @Tom Lendacky At this moment, is SEV guest available to read the content from VarStore? It's quite possible. If you can work up a quick patch, I'll test it out. Yes, the patch is uploaded here https://bugzilla.tianocore.org/show_bug.cgi?id=4379#c17 Hi Min, Thanks for the quick turn-around, but that patch didn't work for me. I've update the bugzilla. Thanks, Tom Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102714): https://edk2.groups.io/g/devel/message/102714 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi Min Xu, On Fri, Apr 07, 2023 at 01:56:05AM +, Min Xu via groups.io wrote: > On Friday, April 7, 2023 4:29 AM, Tom Lendacky wrote: > > On 4/5/23 20:42, Xu, Min M wrote: > > > On April 3, 2023 7:21 PM, Gerd Hoffmann wrote: > > I agree that the efi variable store is not secure without smm. But > > after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE > > doesn't > > work with SEV. System just hangs in "NvVarStore FV headers were > > invalid." > > >>> Hi, Joeyli > > >>> ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is > > >>> skipped > > >> and an error code is returned. So system will not hang. > > >>> So another solution is simply remove the ASSERT. Then an error > > >>> message is > > >> dumped out and system continues. > > >>> > > >>> @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? > > >> > > >> Maybe we just need to call ReserveEmuVariableNvStore a bit later? > > >> > > > I think we can still call ReserveEmuVariableNvStore at PEI phase, but > > > move the initialization of EmuVariableNvStore to > > > > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/EmuVariableFvbR > > u > > > ntimeDxe/Fvb.c#L780-L783 @Tom Lendacky At this moment, is SEV guest > > > available to read the content from VarStore? > > > > It's quite possible. If you can work up a quick patch, I'll test it out. > > > Yes, the patch is uploaded here > https://bugzilla.tianocore.org/show_bug.cgi?id=4379#c17 > I have tested new patch. The issue is not produced, but after I applied debug patch. Looks that the InitializeFvAndVariableStoreForSecureBoot() not be called. I have put detail log on bugzilla. Thanks a lot! Joey Lee -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102704): https://edk2.groups.io/g/devel/message/102704 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On April 7, 2023 5:41 PM, joeyli wrote: > > Hi, Joeyli > > ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped > and an error code is returned. So system will not hang. > > So another solution is simply remove the ASSERT. Then an error message is > dumped out and system continues. > > > > Ah! You are right. I forgot that I enabled debug mode. > > > @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? > > > > Removing ASSERT in debug mode can workaround problem. Looks that it just > hide a problem. @joeyli Based on Gerd's suggestion, there is another patch to fix this issue. If you can test it in your side, that will be great. https://bugzilla.tianocore.org/attachment.cgi?id=1353&action=diff Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102690): https://edk2.groups.io/g/devel/message/102690 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Mon, Apr 03, 2023 at 12:21:38AM +, Xu, Min M wrote: > On Friday, March 31, 2023 10:49 PM, Joeyli wrote: > > On Fri, Mar 31, 2023 at 10:25:09AM +0200, Gerd Hoffmann wrote: > > > On Fri, Mar 31, 2023 at 03:59:56PM +0800, joeyli wrote: > > > > Hi Gerd, > > > > > > > > On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote: > > > > > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > > > > > > From: Min M Xu > > > > > > > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > > > > > > > > > > > PlatformInitEmuVariableNvStore is called to initialize the > > > > > > EmuVariableNvStore with the content pointed by > > > > > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is > > > > > > launched with -bios parameter, UEFI variables will be partially > > > > > > emulated, and non-volatile variables may lose their contents > > > > > > after a reboot. This makes the secure boot feature not working. > > > > > > > > > > > > But in SEV guest, this design doesn't work. Because at this > > > > > > point the variable store mapping is still private/encrypted, > > > > > > OVMF will see ciphertext. So we skip the call of > > > > > > PlatformInitEmuVariableNvStore in SEV guest. > > > > > > > > > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. > > > > > Without initializing the emu var store you will not get a > > > > > functional secure boot setup anyway. > > > > > > > > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a > > > > couple of versions. Removing it will causes problem in VM live > > > > migration. > > > > > > Hmm? qemu live-migrates the rom image too. Only after poweroff and > > > reboot the guest will see an updated firmware image. > > > > > > > Thanks for your explanation. Understood. > > > > > > I will prefer Min M's solution, until SEV experts found better > > > > solution. > > > > > > I'd prefer to not poke holes into secure boot. Re-Initializing the > > > emu var store from rom on each reset is also needed for security > > > reasons in case the efi variable store is not in smm-protected flash > > > memory. > > > > > > > I agree that the efi variable store is not secure without smm. But after > > 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work > > with SEV. System just hangs in "NvVarStore FV headers were invalid." > Hi, Joeyli > ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped > and an error code is returned. So system will not hang. > So another solution is simply remove the ASSERT. Then an error message is > dumped out and system continues. > Ah! You are right. I forgot that I enabled debug mode. > @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? > Removing ASSERT in debug mode can workaround problem. Looks that it just hide a problem. Thanks! Joey Lee -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102677): https://edk2.groups.io/g/devel/message/102677 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Friday, April 7, 2023 4:29 AM, Tom Lendacky wrote: > On 4/5/23 20:42, Xu, Min M wrote: > > On April 3, 2023 7:21 PM, Gerd Hoffmann wrote: > I agree that the efi variable store is not secure without smm. But > after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE > doesn't > work with SEV. System just hangs in "NvVarStore FV headers were > invalid." > >>> Hi, Joeyli > >>> ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is > >>> skipped > >> and an error code is returned. So system will not hang. > >>> So another solution is simply remove the ASSERT. Then an error > >>> message is > >> dumped out and system continues. > >>> > >>> @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? > >> > >> Maybe we just need to call ReserveEmuVariableNvStore a bit later? > >> > > I think we can still call ReserveEmuVariableNvStore at PEI phase, but > > move the initialization of EmuVariableNvStore to > > > https://github.com/tianocore/edk2/blob/master/OvmfPkg/EmuVariableFvbR > u > > ntimeDxe/Fvb.c#L780-L783 @Tom Lendacky At this moment, is SEV guest > > available to read the content from VarStore? > > It's quite possible. If you can work up a quick patch, I'll test it out. > Yes, the patch is uploaded here https://bugzilla.tianocore.org/show_bug.cgi?id=4379#c17 Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102652): https://edk2.groups.io/g/devel/message/102652 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On 4/5/23 20:42, Xu, Min M wrote: On April 3, 2023 7:21 PM, Gerd Hoffmann wrote: I agree that the efi variable store is not secure without smm. But after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work with SEV. System just hangs in "NvVarStore FV headers were invalid." Hi, Joeyli ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped and an error code is returned. So system will not hang. So another solution is simply remove the ASSERT. Then an error message is dumped out and system continues. @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? Maybe we just need to call ReserveEmuVariableNvStore a bit later? I think we can still call ReserveEmuVariableNvStore at PEI phase, but move the initialization of EmuVariableNvStore to https://github.com/tianocore/edk2/blob/master/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c#L780-L783 @Tom Lendacky At this moment, is SEV guest available to read the content from VarStore? It's quite possible. If you can work up a quick patch, I'll test it out. Thanks, Tom Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102628): https://edk2.groups.io/g/devel/message/102628 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On April 3, 2023 7:21 PM, Gerd Hoffmann wrote: > > > I agree that the efi variable store is not secure without smm. But > > > after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't > > > work with SEV. System just hangs in "NvVarStore FV headers were invalid." > > Hi, Joeyli > > ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped > and an error code is returned. So system will not hang. > > So another solution is simply remove the ASSERT. Then an error message is > dumped out and system continues. > > > > @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? > > Maybe we just need to call ReserveEmuVariableNvStore a bit later? > I think we can still call ReserveEmuVariableNvStore at PEI phase, but move the initialization of EmuVariableNvStore to https://github.com/tianocore/edk2/blob/master/OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c#L780-L783 @Tom Lendacky At this moment, is SEV guest available to read the content from VarStore? Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102584): https://edk2.groups.io/g/devel/message/102584 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi,
> > I agree that the efi variable store is not secure without smm. But after
> > 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work
> > with SEV. System just hangs in "NvVarStore FV headers were invalid."
> Hi, Joeyli
> ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped
> and an error code is returned. So system will not hang.
> So another solution is simply remove the ASSERT. Then an error message is
> dumped out and system continues.
>
> @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought?
Maybe we just need to call ReserveEmuVariableNvStore a bit later?
take care,
Gerd
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 148240342b4b..99d40636431f 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -377,10 +377,6 @@ InitializePlatform (
InitializeRamRegions (PlatformInfoHob);
if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME) {
-if (!PlatformInfoHob->SmmSmramRequire) {
- ReserveEmuVariableNvStore ();
-}
-
PeiFvInitialization (PlatformInfoHob);
MemTypeInfoInitialization (PlatformInfoHob);
MemMapInitialization (PlatformInfoHob);
@@ -389,6 +385,12 @@ InitializePlatform (
InstallClearCacheCallback ();
AmdSevInitialize (PlatformInfoHob);
+
+ if (PlatformInfoHob->BootMode != BOOT_ON_S3_RESUME &&
+ !PlatformInfoHob->SmmSmramRequire) {
+ReserveEmuVariableNvStore ();
+ }
+
if (PlatformInfoHob->HostBridgeDevId == 0x) {
MiscInitializationForMicrovm (PlatformInfoHob);
} else {
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102385): https://edk2.groups.io/g/devel/message/102385
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Friday, March 31, 2023 10:49 PM, Joeyli wrote: > On Fri, Mar 31, 2023 at 10:25:09AM +0200, Gerd Hoffmann wrote: > > On Fri, Mar 31, 2023 at 03:59:56PM +0800, joeyli wrote: > > > Hi Gerd, > > > > > > On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote: > > > > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > > > > > From: Min M Xu > > > > > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > > > > > > > > > PlatformInitEmuVariableNvStore is called to initialize the > > > > > EmuVariableNvStore with the content pointed by > > > > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is > > > > > launched with -bios parameter, UEFI variables will be partially > > > > > emulated, and non-volatile variables may lose their contents > > > > > after a reboot. This makes the secure boot feature not working. > > > > > > > > > > But in SEV guest, this design doesn't work. Because at this > > > > > point the variable store mapping is still private/encrypted, > > > > > OVMF will see ciphertext. So we skip the call of > > > > > PlatformInitEmuVariableNvStore in SEV guest. > > > > > > > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. > > > > Without initializing the emu var store you will not get a > > > > functional secure boot setup anyway. > > > > > > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a > > > couple of versions. Removing it will causes problem in VM live migration. > > > > Hmm? qemu live-migrates the rom image too. Only after poweroff and > > reboot the guest will see an updated firmware image. > > > > Thanks for your explanation. Understood. > > > > I will prefer Min M's solution, until SEV experts found better > > > solution. > > > > I'd prefer to not poke holes into secure boot. Re-Initializing the > > emu var store from rom on each reset is also needed for security > > reasons in case the efi variable store is not in smm-protected flash memory. > > > > I agree that the efi variable store is not secure without smm. But after > 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work > with SEV. System just hangs in "NvVarStore FV headers were invalid." Hi, Joeyli ASSERT is triggered in DEBUG version. In RELEASE version ASSERT is skipped and an error code is returned. So system will not hang. So another solution is simply remove the ASSERT. Then an error message is dumped out and system continues. @Gerd Hoffmann @Tom Lendacky @joeyli What's your thought? Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102368): https://edk2.groups.io/g/devel/message/102368 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Fri, Mar 31, 2023 at 10:25:09AM +0200, Gerd Hoffmann wrote: > On Fri, Mar 31, 2023 at 03:59:56PM +0800, joeyli wrote: > > Hi Gerd, > > > > On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote: > > > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > > > > From: Min M Xu > > > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > > > > > > > PlatformInitEmuVariableNvStore is called to initialize the > > > > EmuVariableNvStore with the content pointed by > > > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched > > > > with -bios parameter, UEFI variables will be partially emulated, and > > > > non-volatile variables may lose their contents after a reboot. This > > > > makes > > > > the secure boot feature not working. > > > > > > > > But in SEV guest, this design doesn't work. Because at this point the > > > > variable store mapping is still private/encrypted, OVMF will see > > > > ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in > > > > SEV guest. > > > > > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. > > > Without initializing the emu var store you will not get a functional > > > secure boot setup anyway. > > > > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a couple > > of versions. Removing it will causes problem in VM live migration. > > Hmm? qemu live-migrates the rom image too. Only after poweroff and > reboot the guest will see an updated firmware image. > Thanks for your explanation. Understood. > > I will prefer Min M's solution, until SEV experts found better > > solution. > > I'd prefer to not poke holes into secure boot. Re-Initializing the emu > var store from rom on each reset is also needed for security reasons in > case the efi variable store is not in smm-protected flash memory. > I agree that the efi variable store is not secure without smm. But after 58eb8517ad7b be introduced, the -D SECURE_BOOT_ENABLE doesn't work with SEV. System just hangs in "NvVarStore FV headers were invalid." If secure boot can not work with SEV (even it is not really secure), why not just block the building process when SEV with SECURE_BOOT_ENABLE? At least the issue will not happen at runtime. Thanks Joey Lee -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102316): https://edk2.groups.io/g/devel/message/102316 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Fri, Mar 31, 2023 at 03:59:56PM +0800, joeyli wrote: > Hi Gerd, > > On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote: > > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > > > From: Min M Xu > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > > > > > PlatformInitEmuVariableNvStore is called to initialize the > > > EmuVariableNvStore with the content pointed by > > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched > > > with -bios parameter, UEFI variables will be partially emulated, and > > > non-volatile variables may lose their contents after a reboot. This makes > > > the secure boot feature not working. > > > > > > But in SEV guest, this design doesn't work. Because at this point the > > > variable store mapping is still private/encrypted, OVMF will see > > > ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in > > > SEV guest. > > > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. > > Without initializing the emu var store you will not get a functional > > secure boot setup anyway. > > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a couple > of versions. Removing it will causes problem in VM live migration. Hmm? qemu live-migrates the rom image too. Only after poweroff and reboot the guest will see an updated firmware image. > I will prefer Min M's solution, until SEV experts found better > solution. I'd prefer to not poke holes into secure boot. Re-Initializing the emu var store from rom on each reset is also needed for security reasons in case the efi variable store is not in smm-protected flash memory. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102251): https://edk2.groups.io/g/devel/message/102251 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
Hi Gerd, On Thu, Mar 30, 2023 at 09:50:53AM +0200, Gerd Hoffmann wrote: > On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > > From: Min M Xu > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > > > PlatformInitEmuVariableNvStore is called to initialize the > > EmuVariableNvStore with the content pointed by > > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched > > with -bios parameter, UEFI variables will be partially emulated, and > > non-volatile variables may lose their contents after a reboot. This makes > > the secure boot feature not working. > > > > But in SEV guest, this design doesn't work. Because at this point the > > variable store mapping is still private/encrypted, OVMF will see > > ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in > > SEV guest. > > I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. > Without initializing the emu var store you will not get a functional > secure boot setup anyway. > In our case, we already shipped ovmf with -D SECURE_BOOT_ENABLE in a couple of versions. Removing it will causes problem in VM live migration. I will prefer Min M's solution, until SEV experts found better solution. Thank! Joey Lee -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102247): https://edk2.groups.io/g/devel/message/102247 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
On Wed, Mar 29, 2023 at 01:23:10PM +0800, Min Xu wrote: > From: Min M Xu > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379 > > PlatformInitEmuVariableNvStore is called to initialize the > EmuVariableNvStore with the content pointed by > PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched > with -bios parameter, UEFI variables will be partially emulated, and > non-volatile variables may lose their contents after a reboot. This makes > the secure boot feature not working. > > But in SEV guest, this design doesn't work. Because at this point the > variable store mapping is still private/encrypted, OVMF will see > ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in > SEV guest. I'd suggest to simply build without -D SECURE_BOOT_ENABLE instead. Without initializing the emu var store you will not get a functional secure boot setup anyway. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102158): https://edk2.groups.io/g/devel/message/102158 Mute This Topic: https://groups.io/mt/97922617/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [PATCH V1 1/1] OvmfPkg/PlatformPei: Skip PlatformInitEmuVariableNvStore in SEV guest
From: Min M Xu
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4379
PlatformInitEmuVariableNvStore is called to initialize the
EmuVariableNvStore with the content pointed by
PcdOvmfFlashNvStorageVariableBase. This is because when OVMF is launched
with -bios parameter, UEFI variables will be partially emulated, and
non-volatile variables may lose their contents after a reboot. This makes
the secure boot feature not working.
But in SEV guest, this design doesn't work. Because at this point the
variable store mapping is still private/encrypted, OVMF will see
ciphertext. So we skip the call of PlatformInitEmuVariableNvStore in
SEV guest.
Cc: Erdem Aktas
Cc: James Bottomley
Cc: Jiewen Yao
Cc: Tom Lendacky
Cc: Michael Roth
Cc: Gerd Hoffmann
Reported-by: Joey Lee
Tested-by: Joey Lee
Signed-off-by: Min Xu
---
OvmfPkg/PlatformPei/Platform.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 148240342b4b..be9ba3e00124 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -223,7 +223,20 @@ ReserveEmuVariableNvStore (
PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, VariableStore);
#ifdef SECURE_BOOT_FEATURE_ENABLED
- PlatformInitEmuVariableNvStore ((VOID *)(UINTN)VariableStore);
+ //
+ // PlatformInitEmuVariableNvStore is called to initialize the
EmuVariableNvStore
+ // with the content pointed by PcdOvmfFlashNvStorageVariableBase. This is
because
+ // when OVMF is launched with -bios parameter, UEFI variables will be
partially emulated,
+ // and non-volatile variables may lose their contents after a reboot. This
makes the secure
+ // boot feature not working.
+ // But in SEV guest, this design doesn't work. Because at this point the
variable store
+ // mapping is still private/encrypted, OVMF will see ciphertext. So we skip
the call
+ // of PlatformInitEmuVariableNvStore in SEV guest.
+ //
+ if (!MemEncryptSevIsEnabled ()) {
+PlatformInitEmuVariableNvStore ((VOID *)(UINTN)VariableStore);
+ }
+
#endif
ASSERT_RETURN_ERROR (PcdStatus);
--
2.39.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102098): https://edk2.groups.io/g/devel/message/102098
Mute This Topic: https://groups.io/mt/97922617/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
