* [edk2-devel] [PATCH v1 1/3] ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1
2023-07-24 20:15 [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts Kun Qin
@ 2023-07-24 20:15 ` Kun Qin
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 2/3] ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo Kun Qin
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Kun Qin @ 2023-07-24 20:15 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar
From: Kun Qin <kuqin@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
This change extended the functionality of ArmGic to support sending
software generated interrupts to non-secure group 1 at EL1.
The change made here follows the ARM documentation `ICC_SGI1R_EL1,
Interrupt Controller Software Generated Interrupt Group 1 Register`.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin@microsoft.com>
---
ArmPkg/ArmPkg.ci.yaml | 1 +
ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S | 11 +++++++++++
ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S | 10 ++++++++++
ArmPkg/Include/Library/ArmGicLib.h | 5 +++++
4 files changed, 27 insertions(+)
diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
index d31248161189..8a8f738437d5 100644
--- a/ArmPkg/ArmPkg.ci.yaml
+++ b/ArmPkg/ArmPkg.ci.yaml
@@ -158,6 +158,7 @@
"ipriority",
"irouter",
"isenabler",
+ "ishst",
"istatus",
"itargets",
"lable",
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S b/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
index 20f83aa85f3b..f2ab57174be3 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
+++ b/ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S
@@ -23,6 +23,7 @@
#define ICC_IAR1_EL1 S3_0_C12_C12_0
#define ICC_PMR_EL1 S3_0_C4_C6_0
#define ICC_BPR1_EL1 S3_0_C12_C12_3
+#define ICC_SGI1R_EL1 S3_0_C12_C11_5
#endif
@@ -55,6 +56,16 @@ ASM_FUNC(ArmGicV3SetControlSystemRegisterEnable)
4: isb
ret
+// VOID
+// ArmGicV3SendNsG1Sgi (
+// IN UINT64 SgiVal
+// );
+ASM_FUNC(ArmGicV3SendNsG1Sgi)
+ dsb ishst
+ msr ICC_SGI1R_EL1, x0
+ isb
+ ret
+
//VOID
//ArmGicV3EnableInterruptInterface (
// VOID
diff --git a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
index 8c43a613dc57..79e57e4afb70 100644
--- a/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
+++ b/ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S
@@ -29,6 +29,16 @@ ASM_FUNC(ArmGicV3SetControlSystemRegisterEnable)
isb
bx lr
+// VOID
+// ArmGicV3SendNsG1Sgi (
+// IN UINT64 SgiVal
+// );
+ASM_FUNC(ArmGicV3SendNsG1Sgi)
+ dsb ishst
+ mcrr p15, 0, r0, r1, c12 // ICC_SGI1R_EL1
+ isb
+ bx lr
+
//VOID
//ArmGicV3EnableInterruptInterface (
// VOID
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index 93ce8aeb1994..773b27954522 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -332,4 +332,9 @@ ArmGicV3SetPriorityMask (
IN UINTN Priority
);
+VOID
+ArmGicV3SendNsG1Sgi (
+ IN UINT64 SgiVal
+ );
+
#endif // ARMGIC_H_
--
2.41.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107188): https://edk2.groups.io/g/devel/message/107188
Mute This Topic: https://groups.io/mt/100337222/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] 9+ messages in thread* [edk2-devel] [PATCH v1 2/3] ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo
2023-07-24 20:15 [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts Kun Qin
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 1/3] ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1 Kun Qin
@ 2023-07-24 20:15 ` Kun Qin
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 3/3] ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts Kun Qin
2023-07-26 8:45 ` [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and " Ard Biesheuvel
3 siblings, 0 replies; 9+ messages in thread
From: Kun Qin @ 2023-07-24 20:15 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar
From: Kun Qin <kuqin@microsoft.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
This change extended the existing function `ArmGicSendSgiTo` of ArmGicLib
to format the incoming parameters to comply with GICv3 and GICv4 spec,
and signal software generated interrupts to non secure group 1 at EL1.
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin@microsoft.com>
---
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 52 +++++++++++++++++---
ArmPkg/Include/Library/ArmGicLib.h | 22 +++++++++
2 files changed, 68 insertions(+), 6 deletions(-)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 7f4bb248fc72..830d822d2c05 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -146,12 +146,52 @@ ArmGicSendSgiTo (
IN UINT8 SgiId
)
{
- MmioWrite32 (
- GicDistributorBase + ARM_GIC_ICDSGIR,
- ((TargetListFilter & 0x3) << 24) |
- ((CPUTargetList & 0xFF) << 16) |
- (SgiId & 0xF)
- );
+ ARM_GIC_ARCH_REVISION Revision;
+ UINT32 ApplicableTargets;
+ UINT32 AFF3;
+ UINT32 AFF2;
+ UINT32 AFF1;
+ UINT32 AFF0;
+ UINT32 Irm;
+ UINT64 SGIValue;
+
+ Revision = ArmGicGetSupportedArchRevision ();
+ if (Revision == ARM_GIC_ARCH_REVISION_2) {
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDSGIR,
+ ((TargetListFilter & 0x3) << 24) |
+ ((CPUTargetList & 0xFF) << 16) |
+ (SgiId & 0xF)
+ );
+ } else {
+ // Below routine is adopted from gicv3_raise_secure_g0_sgi in TF-A
+
+ /* Extract affinity fields from target */
+ AFF0 = GET_MPIDR_AFF0 (CPUTargetList);
+ AFF1 = GET_MPIDR_AFF1 (CPUTargetList);
+ AFF2 = GET_MPIDR_AFF2 (CPUTargetList);
+ AFF3 = GET_MPIDR_AFF3 (CPUTargetList);
+
+ /*
+ * Make target list from affinity 0, and ensure GICv3 SGI can target
+ * this PE.
+ */
+ ApplicableTargets = (1 << AFF0);
+
+ /*
+ * Evaluate the filter to see if this is for the target or all others
+ */
+ Irm = (TargetListFilter == ARM_GIC_ICDSGIR_FILTER_EVERYONEELSE) ? SGIR_IRM_TO_OTHERS : SGIR_IRM_TO_AFF;
+
+ /* Raise SGI to PE specified by its affinity */
+ SGIValue = GICV3_SGIR_VALUE (AFF3, AFF2, AFF1, SgiId, Irm, ApplicableTargets);
+
+ /*
+ * Ensure that any shared variable updates depending on out of band
+ * interrupt trigger are observed before raising SGI.
+ */
+ ArmGicV3SendNsG1Sgi (SGIValue);
+ }
}
/*
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index 773b27954522..28d58f187d4f 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -110,6 +110,28 @@
// Bit Mask for
#define ARM_GIC_ICCIAR_ACKINTID 0x3FF
+/* ICC SGI macros */
+#define SGIR_TGT_MASK ((UINT64)0xffff)
+#define SGIR_AFF1_SHIFT 16
+#define SGIR_INTID_SHIFT 24
+#define SGIR_INTID_MASK ((UINT64)0xf)
+#define SGIR_AFF2_SHIFT 32
+#define SGIR_IRM_SHIFT 40
+#define SGIR_IRM_MASK ((UINT64)0x1)
+#define SGIR_AFF3_SHIFT 48
+#define SGIR_AFF_MASK ((UINT64)0xff)
+
+#define SGIR_IRM_TO_AFF 0
+#define SGIR_IRM_TO_OTHERS 1
+
+#define GICV3_SGIR_VALUE(_aff3, _aff2, _aff1, _intid, _irm, _tgt) \
+ ((((UINT64) (_aff3) & SGIR_AFF_MASK) << SGIR_AFF3_SHIFT) | \
+ (((UINT64) (_irm) & SGIR_IRM_MASK) << SGIR_IRM_SHIFT) | \
+ (((UINT64) (_aff2) & SGIR_AFF_MASK) << SGIR_AFF2_SHIFT) | \
+ (((_intid) & SGIR_INTID_MASK) << SGIR_INTID_SHIFT) | \
+ (((_aff1) & SGIR_AFF_MASK) << SGIR_AFF1_SHIFT) | \
+ ((_tgt) & SGIR_TGT_MASK))
+
UINT32
EFIAPI
ArmGicGetInterfaceIdentification (
--
2.41.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107189): https://edk2.groups.io/g/devel/message/107189
Mute This Topic: https://groups.io/mt/100337223/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] 9+ messages in thread* [edk2-devel] [PATCH v1 3/3] ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts
2023-07-24 20:15 [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts Kun Qin
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 1/3] ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1 Kun Qin
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 2/3] ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo Kun Qin
@ 2023-07-24 20:15 ` Kun Qin
2023-07-26 8:45 ` [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and " Ard Biesheuvel
3 siblings, 0 replies; 9+ messages in thread
From: Kun Qin @ 2023-07-24 20:15 UTC (permalink / raw)
To: devel; +Cc: Leif Lindholm, Ard Biesheuvel, Sami Mujawar
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
This change provides additional functionalities to `ArmGicLib` to
manipulate pending interrupt related status. The added functions include:
- `ArmGicSetPendingInterrupt`
- `ArmGicClearPendingInterrupt`
- `ArmGicIsInterruptPending`
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Kun Qin <kuqin@microsoft.com>
---
ArmPkg/Drivers/ArmGic/ArmGicLib.c | 162 ++++++++++++++++++++
ArmPkg/ArmPkg.ci.yaml | 2 +
ArmPkg/Include/Library/ArmGicLib.h | 49 ++++++
3 files changed, 213 insertions(+)
diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
index 830d822d2c05..3844dc05e2af 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
+++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
@@ -33,6 +33,12 @@
#define IPRIORITY_ADDRESS(base, offset) ((base) +\
ARM_GICR_CTLR_FRAME_SIZE + ARM_GIC_ICDIPR + 4 * (offset))
+#define ISPENDR_ADDRESS(base, offset) ((base) +\
+ ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ISPENDR + 4 * (offset))
+
+#define ICPENDR_ADDRESS(base, offset) ((base) +\
+ ARM_GICR_CTLR_FRAME_SIZE + ARM_GICR_ICPENDR + 4 * (offset))
+
/**
*
* Return whether the Source interrupt index refers to a shared interrupt (SPI)
@@ -440,6 +446,162 @@ ArmGicIsInterruptEnabled (
return ((Interrupts & (1 << RegShift)) != 0);
}
+/**
+ Set an interrupt to pending state from GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+**/
+VOID
+EFIAPI
+ArmGicSetPendingInterrupt (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ )
+{
+ UINT32 RegOffset;
+ UINTN RegShift;
+ ARM_GIC_ARCH_REVISION Revision;
+ UINTN GicCpuRedistributorBase;
+
+ // Calculate enable register offset and bit position
+ RegOffset = (UINT32)(Source / 32);
+ RegShift = Source % 32;
+
+ Revision = ArmGicGetSupportedArchRevision ();
+ if ((Revision == ARM_GIC_ARCH_REVISION_2) ||
+ FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
+ SourceIsSpi (Source))
+ {
+ // Write set-pending register
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDSPR + (4 * RegOffset),
+ 1 << RegShift
+ );
+ } else {
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
+ if (GicCpuRedistributorBase == 0) {
+ ASSERT_EFI_ERROR (EFI_NOT_FOUND);
+ return;
+ }
+
+ // Write set-enable register
+ MmioWrite32 (
+ ISPENDR_ADDRESS (GicCpuRedistributorBase, RegOffset),
+ 1 << RegShift
+ );
+ }
+}
+
+/**
+ Clear a pending interrupt from GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+**/
+VOID
+EFIAPI
+ArmGicClearPendingInterrupt (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ )
+{
+ UINT32 RegOffset;
+ UINTN RegShift;
+ ARM_GIC_ARCH_REVISION Revision;
+ UINTN GicCpuRedistributorBase;
+
+ // Calculate enable register offset and bit position
+ RegOffset = (UINT32)(Source / 32);
+ RegShift = Source % 32;
+
+ Revision = ArmGicGetSupportedArchRevision ();
+ if ((Revision == ARM_GIC_ARCH_REVISION_2) ||
+ FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
+ SourceIsSpi (Source))
+ {
+ // Write clear-enable register
+ MmioWrite32 (
+ GicDistributorBase + ARM_GIC_ICDICPR + (4 * RegOffset),
+ 1 << RegShift
+ );
+ } else {
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
+ if (GicCpuRedistributorBase == 0) {
+ return;
+ }
+
+ // Write clear-enable register
+ MmioWrite32 (
+ ICPENDR_ADDRESS (GicCpuRedistributorBase, RegOffset),
+ 1 << RegShift
+ );
+ }
+}
+
+/**
+ Check if an interrupt is pending in GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+
+ @return BOOLEAN TRUE if the interrupt is pending, FALSE otherwise.
+**/
+BOOLEAN
+EFIAPI
+ArmGicIsInterruptPending (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ )
+{
+ UINT32 RegOffset;
+ UINTN RegShift;
+ ARM_GIC_ARCH_REVISION Revision;
+ UINTN GicCpuRedistributorBase;
+ UINT32 Interrupts;
+
+ // Calculate enable register offset and bit position
+ RegOffset = (UINT32)(Source / 32);
+ RegShift = Source % 32;
+
+ Revision = ArmGicGetSupportedArchRevision ();
+ if ((Revision == ARM_GIC_ARCH_REVISION_2) ||
+ FeaturePcdGet (PcdArmGicV3WithV2Legacy) ||
+ SourceIsSpi (Source))
+ {
+ Interrupts = MmioRead32 (
+ GicDistributorBase + ARM_GIC_ICDSPR + (4 * RegOffset)
+ );
+ } else {
+ GicCpuRedistributorBase = GicGetCpuRedistributorBase (
+ GicRedistributorBase,
+ Revision
+ );
+ if (GicCpuRedistributorBase == 0) {
+ return 0;
+ }
+
+ // Read set-enable register
+ Interrupts = MmioRead32 (
+ ISPENDR_ADDRESS (GicCpuRedistributorBase, RegOffset)
+ );
+ }
+
+ return ((Interrupts & (1 << RegShift)) != 0);
+}
+
VOID
EFIAPI
ArmGicDisableDistributor (
diff --git a/ArmPkg/ArmPkg.ci.yaml b/ArmPkg/ArmPkg.ci.yaml
index 8a8f738437d5..06e31498cf79 100644
--- a/ArmPkg/ArmPkg.ci.yaml
+++ b/ArmPkg/ArmPkg.ci.yaml
@@ -154,11 +154,13 @@
"icdsgir",
"icdspr",
"icenabler",
+ "icpendr",
"intid",
"ipriority",
"irouter",
"isenabler",
"ishst",
+ "ispendr",
"istatus",
"itargets",
"lable",
diff --git a/ArmPkg/Include/Library/ArmGicLib.h b/ArmPkg/Include/Library/ArmGicLib.h
index 28d58f187d4f..83d52756d61a 100644
--- a/ArmPkg/Include/Library/ArmGicLib.h
+++ b/ArmPkg/Include/Library/ArmGicLib.h
@@ -78,6 +78,8 @@
// GIC SGI & PPI Redistributor frame
#define ARM_GICR_ISENABLER 0x0100 // Interrupt Set-Enable Registers
#define ARM_GICR_ICENABLER 0x0180 // Interrupt Clear-Enable Registers
+#define ARM_GICR_ISPENDR 0x0200 // Interrupt Set-Pending Registers
+#define ARM_GICR_ICPENDR 0x0280 // Interrupt Clear-Pending Registers
// GIC Cpu interface
#define ARM_GIC_ICCICR 0x00 // CPU Interface Control Register
@@ -167,6 +169,53 @@ ArmGicDisableInterruptInterface (
IN UINTN GicInterruptInterfaceBase
);
+/**
+ Set an interrupt to pending state from GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+**/
+VOID
+EFIAPI
+ArmGicSetPendingInterrupt (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ );
+
+/**
+ Clear a pending interrupt from GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+**/
+VOID
+EFIAPI
+ArmGicClearPendingInterrupt (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ );
+
+/**
+ Check if an interrupt is pending in GIC.
+
+ @param GicDistributorBase Base address of platform GIC Distributor.
+ @param GicRedistributorBase Base address of platform GIC Redistributor.
+ @param Source Interrupt source ID.
+
+ @return BOOLEAN TRUE if the interrupt is pending, FALSE otherwise.
+**/
+BOOLEAN
+EFIAPI
+ArmGicIsInterruptPending (
+ IN UINTN GicDistributorBase,
+ IN UINTN GicRedistributorBase,
+ IN UINTN Source
+ );
+
VOID
EFIAPI
ArmGicEnableDistributor (
--
2.41.0.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107190): https://edk2.groups.io/g/devel/message/107190
Mute This Topic: https://groups.io/mt/100337225/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] 9+ messages in thread* Re: [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts
2023-07-24 20:15 [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts Kun Qin
` (2 preceding siblings ...)
2023-07-24 20:15 ` [edk2-devel] [PATCH v1 3/3] ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts Kun Qin
@ 2023-07-26 8:45 ` Ard Biesheuvel
2023-07-26 18:45 ` Kun Qin
3 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2023-07-26 8:45 UTC (permalink / raw)
To: Kun Qin; +Cc: devel, Leif Lindholm, Ard Biesheuvel, Sami Mujawar
On Mon, 24 Jul 2023 at 22:15, Kun Qin <kuqin12@gmail.com> wrote:
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
>
> This patch series introduce a few improvements related to interrupt
> handling for ArmGic driver and library.
>
> 1. The current implementation of the `ArmGicSendSgiTo` is based on GIC v2
> and does not work on GIC v3 and on.
> 2. The pending interrupt related primitives does not exist for end users.
>
> The change added the SGI support compatible with GIC v3 and v4. A few
> pending interrupt related utility functions were also added.
>
> This change was tested on QEMU, FVP and proprietary hardware platforms.
>
> Pathc v1 branch: https://github.com/kuqin12/edk2/tree/improve_gic_v1
>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
>
> Kun Qin (3):
> ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1
> ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo
> ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts
>
Hello Kun,
Could you please explain how this will be used by platforms?
SGIs are only used by the MPcore SEC code, which is deprecated and
shouldn't be used. And manipulating pending interrupts is another
thing we should only support if there is a compelling use case.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107266): https://edk2.groups.io/g/devel/message/107266
Mute This Topic: https://groups.io/mt/100337221/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts
2023-07-26 8:45 ` [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and " Ard Biesheuvel
@ 2023-07-26 18:45 ` Kun Qin
2023-08-03 15:33 ` Ard Biesheuvel
0 siblings, 1 reply; 9+ messages in thread
From: Kun Qin @ 2023-07-26 18:45 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, Leif Lindholm, Ard Biesheuvel, Sami Mujawar
Hi Ard,
Our current use case is around AP core suspension and wake-ups.
The program can suspend the secondary cores through PSCI interfaces
(after powering
them on). BSP can then wake up the suspended cores through SGI on demand.
The pending interrupt manipulation is to support BSP suspension and
wakeup. We
currently use watchdog timer to wake up suspended BSPs after a timeout.
Platforms can perform needed tasks during suspension, such as core power
consumption
analysis, in UEFI environment.
Please let me if you have any suggestions.
Thanks,
Kun
On 7/26/2023 1:45 AM, Ard Biesheuvel wrote:
> On Mon, 24 Jul 2023 at 22:15, Kun Qin <kuqin12@gmail.com> wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
>>
>> This patch series introduce a few improvements related to interrupt
>> handling for ArmGic driver and library.
>>
>> 1. The current implementation of the `ArmGicSendSgiTo` is based on GIC v2
>> and does not work on GIC v3 and on.
>> 2. The pending interrupt related primitives does not exist for end users.
>>
>> The change added the SGI support compatible with GIC v3 and v4. A few
>> pending interrupt related utility functions were also added.
>>
>> This change was tested on QEMU, FVP and proprietary hardware platforms.
>>
>> Pathc v1 branch: https://github.com/kuqin12/edk2/tree/improve_gic_v1
>>
>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>>
>> Kun Qin (3):
>> ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1
>> ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo
>> ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts
>>
> Hello Kun,
>
> Could you please explain how this will be used by platforms?
>
> SGIs are only used by the MPcore SEC code, which is deprecated and
> shouldn't be used. And manipulating pending interrupts is another
> thing we should only support if there is a compelling use case.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107276): https://edk2.groups.io/g/devel/message/107276
Mute This Topic: https://groups.io/mt/100337221/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts
2023-07-26 18:45 ` Kun Qin
@ 2023-08-03 15:33 ` Ard Biesheuvel
2023-08-03 17:02 ` Leif Lindholm
0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2023-08-03 15:33 UTC (permalink / raw)
To: Kun Qin, Leif Lindholm; +Cc: devel, Sami Mujawar
On Wed, 26 Jul 2023 at 20:45, Kun Qin <kuqin12@gmail.com> wrote:
>
> Hi Ard,
>
> Our current use case is around AP core suspension and wake-ups.
>
> The program can suspend the secondary cores through PSCI interfaces
> (after powering
> them on). BSP can then wake up the suspended cores through SGI on demand.
>
The use of PSCI is already dubious in the context of UEFI - combining
it with the use of SGIs for communication between cores seems like a
slippery slope I don't think we should be going down.
> The pending interrupt manipulation is to support BSP suspension and
> wakeup. We
> currently use watchdog timer to wake up suspended BSPs after a timeout.
>
suspended APs right?
> Platforms can perform needed tasks during suspension, such as core power
> consumption
> analysis, in UEFI environment.
>
> Please let me if you have any suggestions.
>
Is there any basis in the UEFI or PI specifications for this functionality?
> On 7/26/2023 1:45 AM, Ard Biesheuvel wrote:
> > On Mon, 24 Jul 2023 at 22:15, Kun Qin <kuqin12@gmail.com> wrote:
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4466
> >>
> >> This patch series introduce a few improvements related to interrupt
> >> handling for ArmGic driver and library.
> >>
> >> 1. The current implementation of the `ArmGicSendSgiTo` is based on GIC v2
> >> and does not work on GIC v3 and on.
> >> 2. The pending interrupt related primitives does not exist for end users.
> >>
> >> The change added the SGI support compatible with GIC v3 and v4. A few
> >> pending interrupt related utility functions were also added.
> >>
> >> This change was tested on QEMU, FVP and proprietary hardware platforms.
> >>
> >> Pathc v1 branch: https://github.com/kuqin12/edk2/tree/improve_gic_v1
> >>
> >> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Sami Mujawar <sami.mujawar@arm.com>
> >>
> >> Kun Qin (3):
> >> ArmPkg: ArmGic: Added support to send SGI to NS G1 EL1
> >> ArmPkg: ArmGicLib: Added GIC v3 and v4 support to ArmGicSendSgiTo
> >> ArmPkg: ArmGic: Added functionalities to manipulate pending interrupts
> >>
> > Hello Kun,
> >
> > Could you please explain how this will be used by platforms?
> >
> > SGIs are only used by the MPcore SEC code, which is deprecated and
> > shouldn't be used. And manipulating pending interrupts is another
> > thing we should only support if there is a compelling use case.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107552): https://edk2.groups.io/g/devel/message/107552
Mute This Topic: https://groups.io/mt/100337221/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts
2023-08-03 15:33 ` Ard Biesheuvel
@ 2023-08-03 17:02 ` Leif Lindholm
2023-08-03 18:15 ` Kun Qin
0 siblings, 1 reply; 9+ messages in thread
From: Leif Lindholm @ 2023-08-03 17:02 UTC (permalink / raw)
To: devel, ardb, Kun Qin; +Cc: Sami Mujawar
On 2023-08-03 16:33, Ard Biesheuvel wrote:
> On Wed, 26 Jul 2023 at 20:45, Kun Qin <kuqin12@gmail.com> wrote:
>>
>> Hi Ard,
>>
>> Our current use case is around AP core suspension and wake-ups.
>>
>> The program can suspend the secondary cores through PSCI interfaces
>> (after powering
>> them on). BSP can then wake up the suspended cores through SGI on demand.
>>
>
> The use of PSCI is already dubious in the context of UEFI - combining
> it with the use of SGIs for communication between cores seems like a
> slippery slope I don't think we should be going down.
Well, UEFI has no concept of multiple cores, so I don't think there's
anything fishy about using PSCI to bring up/down APs. But I agree adding
interrupt support, not considered by either UEFI nor PI, feels off.
What is the fundamental use-case? (Pre-)silicon testing?
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107554): https://edk2.groups.io/g/devel/message/107554
Mute This Topic: https://groups.io/mt/100337221/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] 9+ messages in thread* Re: [edk2-devel] [PATCH v1 0/3] Add support for handling SGI and pending interrupts
2023-08-03 17:02 ` Leif Lindholm
@ 2023-08-03 18:15 ` Kun Qin
0 siblings, 0 replies; 9+ messages in thread
From: Kun Qin @ 2023-08-03 18:15 UTC (permalink / raw)
To: Leif Lindholm, devel, ardb; +Cc: Sami Mujawar
[-- Attachment #1: Type: text/plain, Size: 1793 bytes --]
Hi Leif & Ard,
You are correct. The main use case is around silicon testing.
There is currently no UEFI or PI spec around such functionality though.
Manipulating pending interrupts might be a far fetch, but can we fix
"ArmGicSendSgiTo" function on GIC v3 and v4? It is declared as a public
interface provided through ArmGicLib, but the fact that it does not work
on GIC v3 and v4 without any notes seems confusing and misleading.
Please let me know if you have other suggestions.
Regards,
Kun
On 8/3/2023 10:02 AM, Leif Lindholm wrote:
> On 2023-08-03 16:33, Ard Biesheuvel wrote:
>> On Wed, 26 Jul 2023 at 20:45, Kun Qin <kuqin12@gmail.com> wrote:
>>>
>>> Hi Ard,
>>>
>>> Our current use case is around AP core suspension and wake-ups.
>>>
>>> The program can suspend the secondary cores through PSCI interfaces
>>> (after powering
>>> them on). BSP can then wake up the suspended cores through SGI on
>>> demand.
>>>
>>
>> The use of PSCI is already dubious in the context of UEFI - combining
>> it with the use of SGIs for communication between cores seems like a
>> slippery slope I don't think we should be going down.
>
> Well, UEFI has no concept of multiple cores, so I don't think there's
> anything fishy about using PSCI to bring up/down APs. But I agree
> adding interrupt support, not considered by either UEFI nor PI, feels
> off.
>
> What is the fundamental use-case? (Pre-)silicon testing?
>
> /
> Leif
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107555): https://edk2.groups.io/g/devel/message/107555
Mute This Topic: https://groups.io/mt/100337221/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 3830 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread