Hi Ard, Thank you for the feedback.
Please see my response inline marked [SAMI]. Regards, Sami Mujawar On 10/05/2023, 12:33, "Ard Biesheuvel" <a...@kernel.org <mailto:a...@kernel.org>> wrote: On Tue, 25 Apr 2023 at 18:04, Sami Mujawar <sami.muja...@arm.com <mailto:sami.muja...@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.muja...@arm.com > <mailto:sami.muja...@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? [SAMI] I have done some experiments and I think we can make this selection at runtime based on the CFI entry in the Kvmtool provided DT. I will submit a separate patch series that enables the dynamic detection of CFI flash and either uses the Nor Flash storage or Emulated Runtime variable. I can subsequently drop this patch from the RFC v2 series. [/SAMI] > @@ -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) [SAMI] I think we can drop this patch once we have dynamic detection of the CFI flash interface. > + # 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)' > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104863): https://edk2.groups.io/g/devel/message/104863 Mute This Topic: https://groups.io/mt/98495953/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
