Re: [edk2-devel] [RFC PATCH v1 01/30] ArmVirtPkg: kvmtool: Add Emulated Runtime variable support

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 25 Apr 2023 at 18:04, Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> Although Kvmtool supports a CFI flash interface, it is currently
> implemented using file backed support on the Host. This scenario
> requires the VMM to be within the trust boundary.
>
> In Confidential Compute Architecture the VMM is outside the trust
> boundary. For such architectures Emulated Runtime variable storage
> is desirable.
>
> Therefore, make Emulated Runtime variable storage as the default
> option and add a build flag ENABLE_CFI_FLASH to configure the
> firmware build to use the CFI Flash as the Variable storage.
>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>  ArmVirtPkg/ArmVirtKvmTool.dsc | 22 +++++++++++++++++++-
>  ArmVirtPkg/ArmVirtKvmTool.fdf |  4 +++-
>  2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index d0afe1b49e250c554313c2077b89650d6f6d67cb..d2228a95726b24fe5c2edfbc84b1f5c23a85feba 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -1,7 +1,7 @@
>  #  @file
>  #  Workspace file for KVMTool virtual platform.
>  #
> -#  Copyright (c) 2018 - 2022, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #

Please add a DEFINE for this variable at the start here.

And can we make it default to TRUE?

> @@ -50,7 +50,9 @@ [LibraryClasses.common]
>    ArmVirtMemInfoLib|ArmVirtPkg/Library/KvmtoolVirtMemInfoLib/KvmtoolVirtMemInfoLib.inf
>
>    TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +!ifdef ENABLE_CFI_FLASH
>    VirtNorFlashPlatformLib|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> +!endif
>
>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>
> @@ -156,6 +158,13 @@ [PcdsFixedAtBuild.common]
>    #
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>
> +!ifndef ENABLE_CFI_FLASH

Not sure what the difference is, but we tend to use

!if $(ENABLE_CFI_FLASH) == TRUE

(and use a local DEFINE - see above)



> +  # Emulate Runtime Variable storage
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvModeEnable|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +
>  [PcdsPatchableInModule.common]
>    #
>    # This will be overridden in the code
> @@ -211,6 +220,7 @@ [PcdsDynamicDefault.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
>
> +!ifdef ENABLE_CFI_FLASH
>    # Setup Flash storage variables
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x40000
> @@ -218,6 +228,10 @@ [PcdsDynamicDefault.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x40000
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x40000
> +!else
> +  # Emulate Runtime Variable storage
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
> +!endif
>
>    ## RTC Register address in MMIO space.
>    gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0
> @@ -263,7 +277,9 @@ [Components.common]
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> +!ifdef ENABLE_CFI_FLASH
>        NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> +!endif
>        BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>    }
>
> @@ -271,7 +287,9 @@ [Components.common]
>    MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf {
>      <LibraryClasses>
> +!ifdef ENABLE_CFI_FLASH
>        NULL|ArmVirtPkg/Library/NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf
> +!endif
>    }
>
>    MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> @@ -296,11 +314,13 @@ [Components.common]
>        NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
>    }
>
> +!ifdef ENABLE_CFI_FLASH
>    OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf {
>      <LibraryClasses>
>        # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
>        BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>    }
> +!endif
>
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
> index 82aff47673cb3085c91c1dd7431683c8353c16e6..8ccbccd71e134e0ea97d49380293687aca43e8b9 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.fdf
> +++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2018 - 2022, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018 - 2023, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -154,7 +154,9 @@ [FV.FvMain]
>    INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>    INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
>    INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
> +!ifdef ENABLE_CFI_FLASH
>    INF OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
> +!endif
>
>    #
>    # FAT filesystem + GPT/MBR partitioning + UDF filesystem
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
>
> 
>
>