[edk2-devel] [PATCH v8 34/46] OvmfPkg: Reserve a page in memory for the SEV-ES usage
Laszlo Ersek
lersek at redhat.com
Mon May 25 16:00:25 UTC 2020
On 05/19/20 23:51, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> 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.justen at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> Reviewed-by: Laszlo Ersek <lersek at redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
> ---
> OvmfPkg/OvmfPkgX64.fdf | 3 +++
> OvmfPkg/ResetVector/ResetVector.inf | 1 +
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 11 +++++++++++
> OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
> 4 files changed, 16 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 88b1e880e603..8836b30a0cef 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
>
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
> index 483fd90fe785..e94e1bfcce7e 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -34,6 +34,7 @@ [BuildOptions]
> *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
>
> [Pcd]
> + gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index c3587a1b7814..73a4eaadb1b6 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -89,6 +89,10 @@ SevExit:
> ; If SEV-ES is disabled then EAX will be zero.
> ;
> CheckSevEsFeature:
> + ; Initialize the first byte of the workarea to zero to communicate to
> + ; the SEC phase that SEV-ES is not enabled.
> + mov byte[SEV_ES_WORK_AREA], 0
> +
> xor eax, eax
>
> ; SEV-ES can't be enabled if SEV isn't, so first check the encryption
> @@ -108,6 +112,13 @@ CheckSevEsFeature:
> ; Restore encryption mask
> mov edx, ebx
>
> + test eax, eax
> + jz NoSevEs
> +
> + ; Set the first byte of the workarea to one to communicate to the SEC
> + ; phase that SEV-ES is enabled.
> + mov byte[SEV_ES_WORK_AREA], 1
> +
> NoSevEs:
> OneTimeCallRet CheckSevEsFeature
>
> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
> index bfb77e439105..2967617bfaa0 100644
> --- a/OvmfPkg/ResetVector/ResetVector.nasmb
> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb
> @@ -72,6 +72,7 @@
> %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
> %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
> %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
> + %define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
> %include "Ia32/PageTables64.asm"
> %endif
>
>
The OvmfPkg/ResetVector modifications have been moved to this patch, at
least in part, from patch "OvmfPkg/ResetVector: Add support for a 32-bit
SEV check".
And I don't understand why.
I mean it's possible that setting the first byte of the work area to 1
does not belong in "OvmfPkg/ResetVector: Add support for a 32-bit SEV
check". That's OK; then said manipulation of the work area should be
split to its own patch, which I should then review afresh.
What's not OK is to move code between two reviewed patches *and* keep my
R-b on both.
Please be more transparent about incremental changes.
(1) Please revert this patch to its v7 state, and keep my R-b on it.
(2) Please split the ResetVector changes to a new patch. For the subject
line, I suggest:
OvmfPkg/ResetVector: communicate SEV-ES status to SEC before exceptions
or something similar.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60214): https://edk2.groups.io/g/devel/message/60214
Mute This Topic: https://groups.io/mt/74336596/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list