Re: [edk2-devel] [PATCH v2 08/23] OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support

["Gerd Hoffmann" <kraxel@redhat.com>]

> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
> @@ -23,6 +23,8 @@ STATIC BOOLEAN          mAddressEncMaskChecked = FALSE;
>  STATIC UINT64           mAddressEncMask;
>  STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;
>  
> +STATIC VOID  *mPscBuffer = NULL;
> +
>  typedef enum {
>    SetCBit,
>    ClearCBit

Oh.  Global variable in PEI code (both pre-existing and newly added).

This is problematic because in OVMF PEI is executed in-place and the
firmware volumes is measured by TPM PEIM.  Global variables modify
the PEI firmware volume and break the measurement.

A while back OVMF added EFI_HOB_PLATFORM_INFO (see
OvmfPkg/Include/Library/PlatformInitLib.h) to fix that.  Most fields
in that struct used to be global variables.

> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
> @@ -17,6 +17,8 @@
>  #include "SnpPageStateChange.h"
>  #include "VirtualMemory.h"
>  
> +STATIC UINT8  mPscBufferPage[EFI_PAGE_SIZE];
> +

Same problem.

Given this is a pre-exising problem, affects SEV only and the rest of
the patch looks fine:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

But it should be cleaned up at some point.  BaseMemEncryptSevLib needs
an update anyway (use CpuPageTableLib, support 5-level paging).

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116026): https://edk2.groups.io/g/devel/message/116026
Mute This Topic: https://groups.io/mt/104512949/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-