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 <kra...@redhat.com> > --- > 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 > + jz SevInit > > + ; > + ; 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 <ler...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- 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] -=-=-=-=-=-=-=-=-=-=-=-
