On 4/30/20 4:12 PM, Tom Lendacky wrote:
On 4/30/20 1:58 PM, Laszlo Ersek wrote:
Hi Tom,
Hi Laszlo,
On 04/22/20 19:41, Lendacky, Thomas wrote:
BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7Cce256f35aa2e4748e8e008d7ed3874ae%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637238699042461059&sdata=tXX8nkBo3fB4OVTs2avevW8pwL6AcqJHvFhvlshKySI%3D&reserved=0
Reserve a fixed area of memory for SEV-ES use and set a fixed PCD,
PcdSevEsWorkAreaBase, to this value.
This area will be used by SEV-ES support for two purposes:
1. Communicating the SEV-ES status during BSP boot to SEC:
Using a byte of memory from the page, the BSP reset vector code can
communicate the SEV-ES status to SEC for use before exception
handling can be enabled in SEC. After SEC, this field is no longer
valid and the standard way of determine if SEV-ES is active should
be used.
2. Establishing an area of memory for AP boot support:
A hypervisor is not allowed to update an SEV-ES guest's register
state, so when booting an SEV-ES guest AP, the hypervisor is not
allowed to set the RIP to the guest requested value. Instead an
SEV-ES AP must be re-directed from within the guest to the actual
requested staring location as specified in the INIT-SIPI-SIPI
sequence.
Use this memory for reset vector code that can be programmed to have
the AP jump to the desired RIP location after starting the AP. This
is required for only the very first AP reset.
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
---
OvmfPkg/OvmfPkgX64.fdf | 3 +++
1 file changed, 3 insertions(+)
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 36414c1f8b49..a0bea86f9875 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -82,6 +82,9 @@ [FD.MEMFD]
0x009000|0x002000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
+0x00B000|0x001000
+gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
+
0x010000|0x010000
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
in patch #28 ("OvmfPkg: Create a GHCB page for use during Sec phase"),
we carve out two ranges in FD.MEMFD, and introduce a matching set of 4
PCDs.
Then in patch #29 ("OvmfPkg/PlatformPei: Reserve GHCB-related areas if
S3 is supported"), we reserve those ranges from the OS, as AcpiNVS, if
S3 is supported. The reason we only reserve those ranges if S3 is
enabled because the ranges are only needed in SEC. (See the details in
the commit mesage of patch #29.)
In this patch (patch #33), we carve out a third region in FD.MEMFD. We
don't seem to ever reserve it. I think that's minimally a problem for
S3; the same argument should apply as to the other two areas. Do you agree?
Nice catch! Yes, I missed this one.
Furthermore, I wonder if we should reserve this "work area" from the OS,
and even from the DXE phase (!), *regardless* of S3. I can't immediately
tell when it's the last time (with S3 disabled) when this area is used.
As I understand it, it is only used the first time the APs are booted
up. And that should happen still in the PEI phase, because CpuMpPei
boots up all the APs and counts them. Afterwards (still in the PEI
phase), the APs should be sleeping in ApWakeupFunction(), namely in the
code added by patch #40 ("UefiCpuPkg: Allow AP booting under SEV-ES").
If the AP is woken again, it is actually only "released" by the
hypervisor, and it goes through the special 64bit->16bit transition,
again implemented in patch#40.
So ultimately it shouldn't be necessary to reserve this third region (at
PcdSevEsWorkAreaBase), if S3 is disabled, because it is never used past
the very first AP boot (which happens when CpuMpPei counts the APs).
Do I understand right?
Yes, that is correct. So I just need to do the same thing for this area
that I did in patch #29.
I think I might want to protect the area from DXE allocations, too. Is
there an easy way to detect that PEI is active vs DXE? Even so, will it
*always* be the case that PEI will start the APs first? I'd hate to see a
change down the road where PEI doesn't start the APs and then things break.
Thanks,
Tom
I can probably shift patch #29 after #33 and have one patch for the S3
reservation instead of having two separate patches doing S3 reservation.
Thanks,
Tom
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58481): https://edk2.groups.io/g/devel/message/58481
Mute This Topic: https://groups.io/mt/73201943/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
