Hi Marc, Thank you for this patch.
Edk2 project has moved to a GitHub pull request model for code review and merge. Please let me know if you need any help/information in getting a PR setup. Regards, Sami Mujawar From: Marc Zyngier <[email protected]> Date: Saturday, 27 June 2026 at 16:26 To: [email protected] <[email protected]> Cc: Pierre Gondois <[email protected]>; Sami Mujawar <[email protected]>; Leif Lindholm <[email protected]>; Ard Biesheuvel <[email protected]> Subject: [PATCH] DynamicTablesPkg: ArmGenericTimerParser: Correctly parse/generate EL2 virtual timer entry Running as a kvmtool guest in NV mode, the timer parser explodes quickly, as finding the EL2 virtual timer is too much for it. Granted, it only has been 11+ years since ARMv8.1 was publicly announced, so this is awfully modern. Sigh. Teach the ArmGenericTimerParser about all the architectural per-CPU timers, checking that each of them actually exists in the DT. Tested as a KVM guest at EL1 and EL2 (both E2H==0 and E2H==1). Signed-off-by: Marc Zyngier <[email protected]> Cc: Pierre Gondois <[email protected]> Cc: Sami Mujawar <[email protected]> Cc: Leif Lindholm <[email protected]> Cc: Ard Biesheuvel <[email protected]> --- .../Arm/GenericTimer/ArmGenericTimerParser.c | 86 ++++++++++++------- .../Arm/GenericTimer/ArmGenericTimerParser.h | 1 + 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.c b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.c index a14ed3a98a..65e30eb4c0 100644 --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.c +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.c @@ -56,7 +56,8 @@ TimerNodeParser ( UINT32 GicVersion; INT32 DataSize; INT32 IntCells; - BOOLEAN AlwaysOnTimer; + INT32 IntCount; + UINT32 AlwaysOnTimerFlag; if ((Fdt == NULL) || (GenericTimerInfo == NULL)) @@ -67,9 +68,9 @@ TimerNodeParser ( Data = FdtGetProp (Fdt, TimerNode, "always-on", &DataSize); if ((Data == NULL) || (DataSize < 0)) { - AlwaysOnTimer = FALSE; + AlwaysOnTimerFlag = 0; } else { - AlwaysOnTimer = TRUE; + AlwaysOnTimerFlag = BIT2; } // Get the associated interrupt-controller. @@ -99,36 +100,63 @@ TimerNodeParser ( } Data = FdtGetProp (Fdt, TimerNode, "interrupts", &DataSize); + IntCount = DataSize / IntCells / sizeof (UINT32); if ((Data == NULL) || - (DataSize != (FdtMaxTimerItem * IntCells * sizeof (UINT32)))) - { + (IntCount > FdtMaxTimerItem)) { // If error or not FdtMaxTimerItem interrupts. ASSERT (0); return EFI_ABORTED; } - GenericTimerInfo->SecurePL1TimerGSIV = - FdtGetInterruptId (&Data[FdtSecureTimerIrq * IntCells]); - GenericTimerInfo->SecurePL1TimerFlags = - FdtGetInterruptFlags (&Data[FdtSecureTimerIrq * IntCells]); - GenericTimerInfo->NonSecurePL1TimerGSIV = - FdtGetInterruptId (&Data[FdtNonSecureTimerIrq * IntCells]); - GenericTimerInfo->NonSecurePL1TimerFlags = - FdtGetInterruptFlags (&Data[FdtNonSecureTimerIrq * IntCells]); - GenericTimerInfo->VirtualTimerGSIV = - FdtGetInterruptId (&Data[FdtVirtualTimerIrq * IntCells]); - GenericTimerInfo->VirtualTimerFlags = - FdtGetInterruptFlags (&Data[FdtVirtualTimerIrq * IntCells]); - GenericTimerInfo->NonSecurePL2TimerGSIV = - FdtGetInterruptId (&Data[FdtHypervisorTimerIrq * IntCells]); - GenericTimerInfo->NonSecurePL2TimerFlags = - FdtGetInterruptFlags (&Data[FdtHypervisorTimerIrq * IntCells]); - - if (AlwaysOnTimer) { - GenericTimerInfo->SecurePL1TimerFlags |= BIT2; - GenericTimerInfo->NonSecurePL1TimerFlags |= BIT2; - GenericTimerInfo->VirtualTimerFlags |= BIT2; - GenericTimerInfo->NonSecurePL2TimerFlags |= BIT2; + if ((IntCount > FdtSecureTimerIrq)) { + GenericTimerInfo->SecurePL1TimerGSIV = + FdtGetInterruptId (&Data[FdtSecureTimerIrq * IntCells]); + GenericTimerInfo->SecurePL1TimerFlags = + FdtGetInterruptFlags (&Data[FdtSecureTimerIrq * IntCells]) | AlwaysOnTimerFlag; + } else { + // No timer, no luck + ASSERT (0); + return EFI_ABORTED; + } + + if ((IntCount > FdtNonSecureTimerIrq)) { + GenericTimerInfo->NonSecurePL1TimerGSIV = + FdtGetInterruptId (&Data[FdtNonSecureTimerIrq * IntCells]); + GenericTimerInfo->NonSecurePL1TimerFlags = + FdtGetInterruptFlags (&Data[FdtNonSecureTimerIrq * IntCells]) | AlwaysOnTimerFlag; + } else { + GenericTimerInfo->NonSecurePL1TimerGSIV = 0; + GenericTimerInfo->NonSecurePL1TimerFlags = 0; + } + + if ((IntCount > FdtVirtualTimerIrq)) { + GenericTimerInfo->VirtualTimerGSIV = + FdtGetInterruptId (&Data[FdtVirtualTimerIrq * IntCells]); + GenericTimerInfo->VirtualTimerFlags = + FdtGetInterruptFlags (&Data[FdtVirtualTimerIrq * IntCells]) | AlwaysOnTimerFlag; + } else { + GenericTimerInfo->VirtualTimerGSIV = 0; + GenericTimerInfo->VirtualTimerFlags = 0; + } + + if ((IntCount > FdtHypervisorTimerIrq)) { + GenericTimerInfo->NonSecurePL2TimerGSIV = + FdtGetInterruptId (&Data[FdtHypervisorTimerIrq * IntCells]); + GenericTimerInfo->NonSecurePL2TimerFlags = + FdtGetInterruptFlags (&Data[FdtHypervisorTimerIrq * IntCells]) | AlwaysOnTimerFlag; + } else { + GenericTimerInfo->NonSecurePL2TimerGSIV = 0; + GenericTimerInfo->NonSecurePL2TimerFlags = 0; + } + + if ((IntCount > FdtHypervisorVTimerIrq)) { + GenericTimerInfo->VirtualPL2TimerGSIV = + FdtGetInterruptId (&Data[FdtHypervisorVTimerIrq * IntCells]); + GenericTimerInfo->VirtualPL2TimerFlags = + FdtGetInterruptFlags (&Data[FdtHypervisorVTimerIrq * IntCells]) | AlwaysOnTimerFlag; + } else { + GenericTimerInfo->VirtualPL2TimerGSIV = 0; + GenericTimerInfo->VirtualPL2TimerFlags = 0; } // Setup default values @@ -138,10 +166,6 @@ TimerNodeParser ( GenericTimerInfo->CounterControlBaseAddress = 0xFFFFFFFFFFFFFFFF; GenericTimerInfo->CounterReadBaseAddress = 0xFFFFFFFFFFFFFFFF; - // For systems not implementing ARMv8.1 VHE, this field is 0. - GenericTimerInfo->VirtualPL2TimerGSIV = 0; - GenericTimerInfo->VirtualPL2TimerFlags = 0; - return EFI_SUCCESS; } diff --git a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.h b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.h index f4712048ef..8b685397b9 100644 --- a/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.h +++ b/DynamicTablesPkg/Library/FdtHwInfoParserLib/Arm/GenericTimer/ArmGenericTimerParser.h @@ -17,6 +17,7 @@ typedef enum FdtTimerInterruptItems { FdtNonSecureTimerIrq, ///< Non-secure timer IRQ FdtVirtualTimerIrq, ///< Virtual timer IRQ FdtHypervisorTimerIrq, ///< Hypervisor timer IRQ + FdtHypervisorVTimerIrq, ///< Hypervisor virtual timer IRQ FdtMaxTimerItem ///< Max timer item } FDT_TIMER_INTERRUPT_ITEMS; -- 2.47.3 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#122010): https://edk2.groups.io/g/devel/message/122010 Mute This Topic: https://groups.io/mt/120005632/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
