* [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Kvmtool: Fix ACPI/DT boot selection
@ 2024-03-26 17:07 Sami Mujawar
2024-03-27 8:29 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Sami Mujawar @ 2024-03-26 17:07 UTC (permalink / raw)
To: devel
Cc: Sami Mujawar, ardb+tianocore, quic_llindhol, kraxel,
Pierre.Gondois, Akanksha.Jain2, Sibel.Allinson, nd
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.
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+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at:
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 (#117136): https://edk2.groups.io/g/devel/message/117136
Mute This Topic: https://groups.io/mt/105162199/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Kvmtool: Fix ACPI/DT boot selection
2024-03-26 17:07 [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Kvmtool: Fix ACPI/DT boot selection Sami Mujawar
@ 2024-03-27 8:29 ` Ard Biesheuvel
2024-04-13 12:38 ` Sami Mujawar
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2024-03-27 8:29 UTC (permalink / raw)
To: Sami Mujawar
Cc: devel, ardb+tianocore, quic_llindhol, kraxel, Pierre.Gondois,
Akanksha.Jain2, Sibel.Allinson, nd
Hello Sami,
On Tue, 26 Mar 2024 at 19:07, Sami Mujawar <sami.mujawar@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?
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+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at:
> 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 (#117160): https://edk2.groups.io/g/devel/message/117160
Mute This Topic: https://groups.io/mt/105162199/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Kvmtool: Fix ACPI/DT boot selection
2024-03-27 8:29 ` Ard Biesheuvel
@ 2024-04-13 12:38 ` Sami Mujawar
0 siblings, 0 replies; 3+ messages in thread
From: Sami Mujawar @ 2024-04-13 12:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, ardb+tianocore, quic_llindhol, kraxel, Pierre Gondois,
Akanksha Jain, Sibel Allinson, nd
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" <ardb@kernel.org <mailto:ardb@kernel.org>> wrote:
Hello Sami,
On Tue, 26 Mar 2024 at 19:07, Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@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+tianocore@kernel.org <mailto:ardb+tianocore@kernel.org>>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com <mailto:quic_llindhol@quicinc.com>>
> Cc: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com>>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-04-13 12:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-26 17:07 [edk2-devel] [PATCH v1 1/1] ArmVirtPkg: Kvmtool: Fix ACPI/DT boot selection Sami Mujawar
2024-03-27 8:29 ` Ard Biesheuvel
2024-04-13 12:38 ` Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox