public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH edk2-platforms 0/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT
@ 2023-09-20  8:25 Marcin Juszkiewicz
  2023-09-20  8:25 ` [edk2-devel] [PATCH edk2-platforms 1/1] " Marcin Juszkiewicz
  0 siblings, 1 reply; 4+ messages in thread
From: Marcin Juszkiewicz @ 2023-09-20  8:25 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Graeme Gregory, Ard Biesheuvel, 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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT
  2023-09-20  8:25 [edk2-devel] [PATCH edk2-platforms 0/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT Marcin Juszkiewicz
@ 2023-09-20  8:25 ` Marcin Juszkiewicz
  2023-09-20 10:04   ` Leif Lindholm
  0 siblings, 1 reply; 4+ messages in thread
From: Marcin Juszkiewicz @ 2023-09-20  8:25 UTC (permalink / raw)
  To: devel; +Cc: Leif Lindholm, Graeme Gregory, Ard Biesheuvel, 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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT
  2023-09-20  8:25 ` [edk2-devel] [PATCH edk2-platforms 1/1] " Marcin Juszkiewicz
@ 2023-09-20 10:04   ` Leif Lindholm
  2023-09-20 11:49     ` Marcin Juszkiewicz
  0 siblings, 1 reply; 4+ messages in thread
From: Leif Lindholm @ 2023-09-20 10:04 UTC (permalink / raw)
  To: Marcin Juszkiewicz; +Cc: devel, Graeme Gregory, Ard Biesheuvel

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH edk2-platforms 1/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT
  2023-09-20 10:04   ` Leif Lindholm
@ 2023-09-20 11:49     ` Marcin Juszkiewicz
  0 siblings, 0 replies; 4+ messages in thread
From: Marcin Juszkiewicz @ 2023-09-20 11:49 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: devel, Graeme Gregory, Ard Biesheuvel

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-09-20 11:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-20  8:25 [edk2-devel] [PATCH edk2-platforms 0/1] Platform/QemuSbsa: define NS EL2 virtual timer in GTDT Marcin Juszkiewicz
2023-09-20  8:25 ` [edk2-devel] [PATCH edk2-platforms 1/1] " Marcin Juszkiewicz
2023-09-20 10:04   ` Leif Lindholm
2023-09-20 11:49     ` Marcin Juszkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox