On 2/22/24 12:54, Gerd Hoffmann wrote:
> Use separate control flows for SEV and non-CoCo cases.
>
> SevClearPageEncMaskForGhcbPage and GetSevCBitMaskAbove31 will now only
> be called when running in SEV mode, so the SEV check in these functions
> is not needed any more.
>
> No functional change.
>
> Signed-off-by: Gerd Hoffmann
> ---
> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 16 ++--
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 17 ++---
> 2 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 043c88a7abbe..ed94f1dc668f 100644
> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> @@ -152,12 +152,8 @@ SevEsUnexpectedRespTerminate:
>
> %ifdef ARCH_X64
>
> -; If SEV-ES is enabled then initialize and make the GHCB page shared
> +; initialize and make the GHCB page shared
(1) This comment update is unjustified, I suggest reverting it.
(The SEV check is indeed superfluous below, but you -- correctly -- keep
the SEV-ES check, and the comment here is about SEV-ES, not SEV. Because
the check stays, the comment should stay too.)
> SevClearPageEncMaskForGhcbPage:
> -; Check if SEV is enabled
> -cmp byte[WORK_AREA_GUEST_TYPE], 1
> -jnz SevClearPageEncMaskForGhcbPageExit
> -
> ; Check if SEV-ES is enabled
> mov ecx, 1
> bt[SEV_ES_WORK_AREA_STATUS_MSR], ecx
> @@ -195,20 +191,12 @@ pageTableEntries4kLoop:
> SevClearPageEncMaskForGhcbPageExit:
> OneTimeCallRet SevClearPageEncMaskForGhcbPage
>
> -; Check if SEV is enabled, and get the C-bit mask above 31.
> +; Get the C-bit mask above 31.
> ; Modified: EDX
> ;
> ; The value is returned in the EDX
> GetSevCBitMaskAbove31:
> -xor edx, edx
> -
> -; Check if SEV is enabled
> -cmp byte[WORK_AREA_GUEST_TYPE], 1
> -jnz GetSevCBitMaskAbove31Exit
> -
> mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> -
> -GetSevCBitMaskAbove31Exit:
> OneTimeCallRet GetSevCBitMaskAbove31
>
> %endif
> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> index 166e80293c89..84a7b4efc019 100644
> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
> @@ -118,15 +118,26 @@ SetCr3ForPageTables64:
>
> ; Check whether the SEV is active and populate the SevEsWorkArea
> OneTimeCall CheckSevFeatures
> +cmp byte[WORK_AREA_GUEST_TYPE], 1
> +jzSevInit
>
> +;
> +; normal (non-CoCo) workflow
> +;
> +ClearOvmfPageTables
> +CreatePageTables4Level 0
> +jmp SetCr3
> +
> +SevInit:
> +;
> +; SEV workflow
> +;
> +ClearOvmfPageTables
> ; If SEV is enabled, the C-bit position is always above 31.
> ; The mask will be saved in the EDX and applied during the
> ; the page table build below.
> OneTimeCall GetSevCBitMaskAbove31
> -
> -ClearOvmfPageTables
> CreatePageTables4Level edx
> -
> ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
> OneTimeCall SevClearPageEncMaskForGhcbPage
> jmp SetCr3
Nice.
The patch also sneakily reorders ClearOvmfPageTables against
GetSevCBitMaskAbove31 -- but that's an improvement: this way we no
longer depend on ClearOvmfPageTables not modifying EDX; instead, EDX
directly passes from GetSevCBitMaskAbove31 to CreatePageTables4Level.
With (1) undone:
Reviewed-by: Laszlo Ersek
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116084): https://edk2.groups.io/g/devel/message/116084
Mute This Topic: https://groups.io/mt/104506794/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
