fix(gic): SGI helper to use cpu struct id affinity
The plat_interrupts_send_sgi interface uses a target list bitfield
parameter to identify the PE to which an SGI is to be sent. The target
list is infered from the target core linear id.
Furthermore, gicv3_send_sgi uses the target list combined with the
current core MPIDR Aff3 to Aff1 affinities.
This approach is slightly wrong because the target list shall be
derived from the Aff0 value of the target PE.
This used to 'work' on qemu because Aff3 to Aff1 are zero and Aff0
encodes the linear PE id. This implementation exhibits a failure on
FVP because Aff0 is zero on all cores.
This change redefines the plat_interrupts_send_sgi interface to trigger
an SGI by using the id from the cpu struct passed as parameter.
gicv3_send_sgi is made independent from MPIDR and calculates the target
list from Aff0.
Signed-off-by: Olivier Deprez <olivier.deprez@arm.com>
Change-Id: I0ab925498955d27c2b31b313e294dbcdbc4cc3f9
diff --git a/inc/hf/plat/interrupts.h b/inc/hf/plat/interrupts.h
index 1530b22..c1a86e8 100644
--- a/inc/hf/plat/interrupts.h
+++ b/inc/hf/plat/interrupts.h
@@ -29,6 +29,5 @@
uint32_t plat_interrupts_get_pending_interrupt_id(void);
void plat_interrupts_end_of_interrupt(uint32_t id);
void plat_interrupts_configure_interrupt(struct interrupt_descriptor int_desc);
-void plat_interrupts_send_sgi(uint32_t id, bool send_to_all,
- uint32_t target_list,
+void plat_interrupts_send_sgi(uint32_t id, struct cpu *cpu,
bool to_this_security_state);
diff --git a/src/arch/aarch64/hypervisor/handler.c b/src/arch/aarch64/hypervisor/handler.c
index bc141c2..4bd8a3b 100644
--- a/src/arch/aarch64/hypervisor/handler.c
+++ b/src/arch/aarch64/hypervisor/handler.c
@@ -697,7 +697,8 @@
#if SECURE_WORLD == 1
/*
* If giving back execution to the NWd, check if the Schedule
- * Receiver Interrupt has been delayed, and trigger it if so.
+ * Receiver Interrupt has been delayed, and trigger it on
+ * current core if so.
*/
if ((*next != NULL && (*next)->vm->id == HF_OTHER_WORLD_ID) ||
(*next == NULL && vcpu->vm->id == HF_OTHER_WORLD_ID)) {
diff --git a/src/arch/aarch64/plat/ffa/spmc.c b/src/arch/aarch64/plat/ffa/spmc.c
index bc91546..77bdbe7 100644
--- a/src/arch/aarch64/plat/ffa/spmc.c
+++ b/src/arch/aarch64/plat/ffa/spmc.c
@@ -1317,11 +1317,10 @@
static void plat_ffa_send_schedule_receiver_interrupt(struct cpu *cpu)
{
- dlog_verbose("Setting Schedule Receiver SGI %d on core: %d\n",
+ dlog_verbose("Setting Schedule Receiver SGI %u on core: %u\n",
HF_SCHEDULE_RECEIVER_INTID, cpu_index(cpu));
- plat_interrupts_send_sgi(HF_SCHEDULE_RECEIVER_INTID, false,
- (1 << cpu_index(cpu)), false);
+ plat_interrupts_send_sgi(HF_SCHEDULE_RECEIVER_INTID, cpu, false);
}
void plat_ffa_sri_trigger_if_delayed(struct cpu *cpu)
diff --git a/src/arch/aarch64/plat/interrupts/absent.c b/src/arch/aarch64/plat/interrupts/absent.c
index a803e09..4cdd79e 100644
--- a/src/arch/aarch64/plat/interrupts/absent.c
+++ b/src/arch/aarch64/plat/interrupts/absent.c
@@ -75,11 +75,10 @@
(void)int_desc;
}
-void plat_interrupts_send_sgi(uint32_t id, bool send_to_all,
- uint32_t target_list, bool to_this_security_state)
+void plat_interrupts_send_sgi(uint32_t id, struct cpu *cpu,
+ bool to_this_security_state)
{
(void)id;
- (void)send_to_all;
- (void)target_list;
+ (void)cpu;
(void)to_this_security_state;
}
diff --git a/src/arch/aarch64/plat/interrupts/gicv3.c b/src/arch/aarch64/plat/interrupts/gicv3.c
index 803ff2c..aed5845 100644
--- a/src/arch/aarch64/plat/interrupts/gicv3.c
+++ b/src/arch/aarch64/plat/interrupts/gicv3.c
@@ -415,16 +415,14 @@
(void)core_pos;
}
-void gicv3_send_sgi(uint32_t sgi_id, bool send_to_all, uint32_t target_list,
+void gicv3_send_sgi(uint32_t sgi_id, bool send_to_all, uint64_t mpidr_target,
bool to_this_security_state)
{
uint64_t sgir;
uint64_t irm;
- uint64_t mpidr_reg;
CHECK(is_sgi_ppi(sgi_id));
- mpidr_reg = read_msr(MPIDR_EL1);
sgir = (sgi_id & SGIR_INTID_MASK) << SGIR_INTID_SHIFT;
/* Check the interrupt routing mode. */
@@ -438,6 +436,7 @@
* generated.
*/
+ uint64_t aff0;
uint64_t aff1;
uint64_t aff2;
uint64_t aff3;
@@ -447,12 +446,10 @@
* will be delivered the interrupt. At least one has to be
* enabled.
*/
-
- CHECK(target_list != 0U);
-
- aff3 = (mpidr_reg >> MPIDR_AFF3_SHIFT) & (0xff);
- aff2 = (mpidr_reg >> MPIDR_AFF2_SHIFT) & (0xff);
- aff1 = (mpidr_reg >> MPIDR_AFF1_SHIFT) & (0xff);
+ aff3 = (mpidr_target >> MPIDR_AFF3_SHIFT) & (0xff);
+ aff2 = (mpidr_target >> MPIDR_AFF2_SHIFT) & (0xff);
+ aff1 = (mpidr_target >> MPIDR_AFF1_SHIFT) & (0xff);
+ aff0 = (mpidr_target >> MPIDR_AFF0_SHIFT) & (0xff);
/* Populate the various affinity fields. */
sgir |= ((aff3 & SGIR_AFF_MASK) << SGIR_AFF3_SHIFT) |
@@ -460,7 +457,7 @@
((aff1 & SGIR_AFF_MASK) << SGIR_AFF1_SHIFT);
/* Construct the SGI target affinity. */
- sgir |= (target_list & SGIR_TGT_MASK) << SGIR_TGT_SHIFT;
+ sgir |= ((1U << aff0) & SGIR_TGT_MASK) << SGIR_TGT_SHIFT;
}
/* Populate the Interrupt Routing Mode field. */
@@ -638,8 +635,8 @@
gicv3_enable_interrupt(intr_num, core_idx);
}
-void plat_interrupts_send_sgi(uint32_t id, bool send_to_all,
- uint32_t target_list, bool to_this_security_state)
+void plat_interrupts_send_sgi(uint32_t id, struct cpu *cpu,
+ bool to_this_security_state)
{
- gicv3_send_sgi(id, send_to_all, target_list, to_this_security_state);
+ gicv3_send_sgi(id, false, cpu->id, to_this_security_state);
}
diff --git a/src/arch/aarch64/plat/interrupts/gicv3_helpers.h b/src/arch/aarch64/plat/interrupts/gicv3_helpers.h
index 944db49..2bf817c 100644
--- a/src/arch/aarch64/plat/interrupts/gicv3_helpers.h
+++ b/src/arch/aarch64/plat/interrupts/gicv3_helpers.h
@@ -240,7 +240,7 @@
#define SGIR_IRM_SHIFT 40
#define SGIR_IRM_MASK 0x1
#define SGIR_AFF3_SHIFT 48
-#define SGIR_AFF_MASK 0xf
+#define SGIR_AFF_MASK 0xff
#define SGIR_IRM_TO_AFF (0)