[edk2-devel] [PATCH edk2-platforms 0/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

[edk2-devel] [PATCH edk2-platforms 0/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

[Marcin Juszkiewicz]

Arm BSA (Base System Architecture) specification requires Armv8.1+ cpus
to have non-secure EL2 virtual timer. Which we lacked.

In previous week I wrote a small patch to QEMU which enabled it for SBSA
Reference Platform. Leif Lindholm refactored code around timers to make
it more readable.

Then he added missing timer into EDK2 ArmPkg and to "virt" platform.

This patch enables NS EL2 virtual timer on SBSA Reference Platform.


Marcin Juszkiewicz (1):
  Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

 Platform/Qemu/SbsaQemu/SbsaQemu.dsc             | 2 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 +
 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc      | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.41.0



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

[edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

[Marcin Juszkiewicz]

Armv8.1+ cpus have Virtual Host Extension (VHE) which added non-secure
EL2 virtual timer.

This change adds it into GTDT to fullfil Arm BSA (Base System
Architecture) requirements.

Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
---
 Platform/Qemu/SbsaQemu/SbsaQemu.dsc             | 2 ++
 Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 +
 Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc      | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
index be406144c242..8bea9793451a 100644
--- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
+++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
@@ -447,6 +447,8 @@ [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|27
   # PPI #10
   gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|26
+  # PPI #12
+  gArmTokenSpaceGuid.PcdArmArchTimerHypVirtIntrNum|28
 
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x60010000
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
index 97021f7971c7..343c75f0b4ec 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
@@ -36,6 +36,7 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
+  gArmTokenSpaceGuid.PcdArmArchTimerHypVirtIntrNum
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
index ba145aff6413..b5e8f8405d61 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
@@ -91,8 +91,8 @@
       SBSA_PLATFORM_TIMER_COUNT,                    // UINT32  PlatformTimerCount
       sizeof(EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE),
                                                     // UINT32  PlatformTimerOffset
-      0,                                            // UINT32  VirtualPL2TimerGSIV
-      0                                             // UINT32  VirtualPL2TimerFlags
+      FixedPcdGet32 (PcdArmArchTimerHypVirtIntrNum),// UINT32  VirtualPL2TimerGSIV
+      GTDT_GTIMER_FLAGS                             // UINT32  VirtualPL2TimerFlags
     },
     EFI_ACPI_6_3_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
     SBSAQEMU_WDT_REFRESH_FRAME_BASE,
-- 
2.41.0



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

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

[Leif Lindholm]

On Wed, Sep 20, 2023 at 10:25:09 +0200, Marcin Juszkiewicz wrote:
> Armv8.1+ cpus have Virtual Host Extension (VHE) which added non-secure
> EL2 virtual timer.
> 
> This change adds it into GTDT to fullfil Arm BSA (Base System
> Architecture) requirements.
> 
> Signed-off-by: Marcin Juszkiewicz <marcin.juszkiewicz@linaro.org>
> ---
>  Platform/Qemu/SbsaQemu/SbsaQemu.dsc             | 2 ++
>  Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf | 1 +
>  Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc      | 4 ++--
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> index be406144c242..8bea9793451a 100644
> --- a/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> +++ b/Platform/Qemu/SbsaQemu/SbsaQemu.dsc
> @@ -447,6 +447,8 @@ [PcdsFixedAtBuild.common]
>    gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|27
>    # PPI #10
>    gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|26
> +  # PPI #12
> +  gArmTokenSpaceGuid.PcdArmArchTimerHypVirtIntrNum|28
>  
>    ## PL031 RealTimeClock
>    gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x60010000
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> index 97021f7971c7..343c75f0b4ec 100644
> --- a/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/AcpiTables.inf
> @@ -36,6 +36,7 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
>    gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
>    gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
> +  gArmTokenSpaceGuid.PcdArmArchTimerHypVirtIntrNum
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
> diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
> index ba145aff6413..b5e8f8405d61 100644
> --- a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
> +++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
> @@ -91,8 +91,8 @@
>        SBSA_PLATFORM_TIMER_COUNT,                    // UINT32  PlatformTimerCount
>        sizeof(EFI_ACPI_6_3_GENERIC_TIMER_DESCRIPTION_TABLE),
>                                                      // UINT32  PlatformTimerOffset
> -      0,                                            // UINT32  VirtualPL2TimerGSIV
> -      0                                             // UINT32  VirtualPL2TimerFlags
> +      FixedPcdGet32 (PcdArmArchTimerHypVirtIntrNum),// UINT32  VirtualPL2TimerGSIV
> +      GTDT_GTIMER_FLAGS                             // UINT32  VirtualPL2TimerFlags

It's still valid to use other CPUs than "max" with this platform.
Don't we need to conditionalise this based on the contents of the VH
bits in ID_AA64MFR1_EL1?

Ideally, we'd add a helper function in edk2
ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c, like ArmHasCcidx(), and
conditionalise on that.

Hmm, but we'd probably also need to move from .aslc to manually
construction GTDT in SbsaQemuAcpiDxe...

If you're up for doing the GTDT rework, I could create the ArmLib
helper function.

/
    Leif


>      },
>      EFI_ACPI_6_3_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
>      SBSAQEMU_WDT_REFRESH_FRAME_BASE,
> -- 
> 2.41.0
> 


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

Re: [edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT

[Marcin Juszkiewicz]

W dniu 20.09.2023 o 12:04, Leif Lindholm pisze:
> On Wed, Sep 20, 2023 at 10:25:09 +0200, Marcin Juszkiewicz wrote:
>> Armv8.1+ cpus have Virtual Host Extension (VHE) which added non-secure
>> EL2 virtual timer.

> It's still valid to use other CPUs than "max" with this platform.
> Don't we need to conditionalise this based on the contents of the VH
> bits in ID_AA64MFR1_EL1?

Most of cpu cores available for SBSA Reference Platform are v8.2+ ones. 
And default is Neoverse-N1.

> Ideally, we'd add a helper function in edk2
> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c, like ArmHasCcidx(), and
> conditionalise on that.
> 
> Hmm, but we'd probably also need to move from .aslc to manually
> construction GTDT in SbsaQemuAcpiDxe...
> 
> If you're up for doing the GTDT rework, I could create the ArmLib
> helper function.

Sooner or later it needs to be done anyway as we need to add system 
timers there (which iirc only Ampere does in EDK2).



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