Hi Ard, Thank you for your feedback.
Please see my response inline marked [SAMI]. Regards, Sami Mujawar On 27/03/2024, 14:00, "Ard Biesheuvel" <a...@kernel.org <mailto:a...@kernel.org>> wrote: Hello Sami, On Tue, 26 Mar 2024 at 19:07, Sami Mujawar <sami.muja...@arm.com <mailto:sami.muja...@arm.com>> wrote: > > The Kvmtool guest firmware uses the dynamic HII > PCD PcdForceNoAcpi to determine if ACPI tables > or the DT must be used for booting an OS. > > This PcdForceNoAcpi is a BOOLEAN value that can > be configured using the variable "ForceNoAcpi" > specifing the gOvmfVariableGuid GUID which is > "50BEA1E5-A2C5-46E9-9B3A-59596516B00A". > > However, this feature was not working as the > PCD was not defined in the platform DSC file > and the DEPEX section in KvmtoolPlatfomDxe.inf > was not set correctly. > Understood. I do wonder whether gEfiVariableArchProtocolGuid is the appropriate protocol here to DEPEX on. Shouldn't PcdDxe depend on this already, and should we depend on the PCD protocol instead? [SAMI] I did some experiments and realised we have a cyclic dependency. I expect some further changes once the Arm CCA support patches are merged and will therefore send out a v2 with the dependency fixed later. [/SAMI] Other than that, this looks fine to me. > Therefore, fix this issue so that the ACPI/DT > boot selection can be done from the UEFI shell > as shown below. > > 1. Check the status of the 'ForceNoAcpi' variable > setvar ForceNoAcpi -guid > "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" > -nv -bs > > Value 00 indicates ACPI boot > Value 01 indicates DT boot > > 2. Set the boot mode to ACPI > setvar ForceNoAcpi -guid > "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" > -nv -bs =0x00 > > 3. Set the boot mode to DT > setvar ForceNoAcpi -guid > "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" > -nv -bs =0x01 > > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org > <mailto:ardb+tianoc...@kernel.org>> > Cc: Leif Lindholm <quic_llind...@quicinc.com > <mailto:quic_llind...@quicinc.com>> > Cc: Gerd Hoffmann <kra...@redhat.com <mailto:kra...@redhat.com>> > Signed-off-by: Sami Mujawar <sami.muja...@arm.com > <mailto:sami.muja...@arm.com>> > --- > The changes can be seen at: > https://github.com/samimujawar/edk2/tree/2954_kvmtool_fix_acpi_dt_selection_v1 > > <https://github.com/samimujawar/edk2/tree/2954_kvmtool_fix_acpi_dt_selection_v1> > > ArmVirtPkg/ArmVirtKvmTool.dsc | 14 ++++++++++++++ > ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 2 +- > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc > index > 20da3319667900e64755272fa110d57452d1fc67..c3c27b2765b34599c7312026ce5cb9474a22c684 > 100644 > --- a/ArmVirtPkg/ArmVirtKvmTool.dsc > +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc > @@ -189,6 +189,20 @@ [PcdsPatchableInModule.common] > [PcdsDynamicHii] > gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5 > > + # > + # Dynamic Hii PCD to select ACPI/DT boot. > + # > + # 1. Check the status of the 'ForceNoAcpi' variable > + # setvar ForceNoAcpi -guid "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" -nv -bs > + # Value 00 indicates ACPI boot > + # Value 01 indicates DT boot > + # 2. Set the boot mode to ACPI > + # setvar ForceNoAcpi -guid "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" -nv -bs > =0x00 > + # 3. Set the boot mode to DT > + # setvar ForceNoAcpi -guid "50BEA1E5-A2C5-46E9-9B3A-59596516B00A" -nv -bs > =0x01 > + # > + > gUefiOvmfPkgTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gOvmfVariableGuid|0x0|FALSE|NV,BS > + > [PcdsDynamicDefault.common] > gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0 > gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0 > diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf > b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf > index > b0583d52058805aaeece31d7e3776ac498f101ad..508bfa60c2c2cb3f3e7456b010f4e9057437cda8 > 100644 > --- a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf > +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf > @@ -42,4 +42,4 @@ [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdForceNoAcpi > > [Depex] > - TRUE > + gEfiVariableArchProtocolGuid > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117720): https://edk2.groups.io/g/devel/message/117720 Mute This Topic: https://groups.io/mt/105162199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
