refactor(interrupts): deprecate HF_INTERRUPT_DEACTIVATE
Now that the bitmap and queue for tracking interrupts are kept
consistent and interrupts are popped during HF_INTERRUPT_GET,
HF_INTERRUPT_DEACTIVATE is no longer required. So have it simply
return 0.
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>
Change-Id: Ieec7c237aaf72987206ead01e5a92285e38c0ca1
diff --git a/inc/hf/vcpu.h b/inc/hf/vcpu.h
index 2e26ff4..1c4f50d 100644
--- a/inc/hf/vcpu.h
+++ b/inc/hf/vcpu.h
@@ -178,19 +178,6 @@
*/
struct vcpu *preempted_vcpu;
- /**
- * Per FF-A v1.1-Beta0 spec section 8.3, an SP can use multiple
- * mechanisms to signal completion of secure interrupt handling. SP
- * can invoke explicit FF-A ABIs, namely FFA_MSG_WAIT and FFA_RUN,
- * when in WAITING/BLOCKED state respectively, but has to perform
- * implicit signal completion mechanism by dropping the priority
- * of the virtual secure interrupt when SPMC signaled the virtual
- * interrupt in PREEMPTED state(The vCPU was preempted by a Self S-Int
- * while running). This variable helps SPMC to keep a track of such
- * mechanism and perform appropriate bookkeeping.
- */
- bool requires_deactivate_call;
-
/** SP call chain. */
struct call_chain call_chain;
diff --git a/src/ffa/spmc/cpu_cycles.c b/src/ffa/spmc/cpu_cycles.c
index 4785f39..d4446d9 100644
--- a/src/ffa/spmc/cpu_cycles.c
+++ b/src/ffa/spmc/cpu_cycles.c
@@ -128,13 +128,6 @@
if (target_vcpu != current->preempted_vcpu) {
dlog_verbose("Skipping intermediate vCPUs\n");
}
- /*
- * This flag should not have been set by SPMC when it
- * signaled the virtual interrupt to the SP while SP was
- * in WAITING or BLOCKED states. Refer the embedded
- * comment in vcpu.h file for further description.
- */
- assert(!current->requires_deactivate_call);
/*
* Clear fields corresponding to secure interrupt
diff --git a/src/ffa/spmc/interrupts.c b/src/ffa/spmc/interrupts.c
index 6529ee4..74bc5b8 100644
--- a/src/ffa/spmc/interrupts.c
+++ b/src/ffa/spmc/interrupts.c
@@ -19,9 +19,9 @@
#include "hf/vm.h"
/**
- * Drops the current interrupt priority and deactivate the given interrupt ID
- * for the calling vCPU.
- *
+ * This function has been deprecated and it's contents moved into
+ * api_interrupt_get in order to align the bitmap and queue for tracking
+ * interupts.
* Returns 0 on success, or -1 otherwise.
*/
int64_t ffa_interrupts_deactivate(uint32_t pint_id, uint32_t vint_id,
@@ -29,20 +29,8 @@
{
(void)pint_id;
(void)vint_id;
- int ret = 0;
- struct vcpu_locked current_locked;
-
- current_locked = vcpu_lock(current);
-
- if (current->requires_deactivate_call) {
- /* There is no preempted vCPU to resume. */
- assert(current->preempted_vcpu == NULL);
-
- vcpu_secure_interrupt_complete(current_locked);
- }
-
- vcpu_unlock(¤t_locked);
- return ret;
+ (void)current;
+ return 0;
}
static struct vcpu *ffa_interrupts_find_target_vcpu_secure_interrupt(
@@ -405,18 +393,6 @@
break;
case VCPU_STATE_RUNNING:
- if (current == target_vcpu) {
- /*
- * This is the special scenario where the current
- * running execution context also happens to be the
- * target of the secure interrupt. In this case, it
- * needs to signal completion of secure interrupt
- * implicitly. Refer to the embedded comment in vcpu.h
- * file for the description of this variable.
- */
- current->requires_deactivate_call = true;
- }
-
next = NULL;
ffa_interrupts_set_preempted_vcpu(
target_vcpu_locked, (struct vcpu_locked){.vcpu = NULL});
@@ -510,13 +486,6 @@
/* Resume current vCPU. */
*next = NULL;
} else {
- /*
- * SPMC has started handling a secure interrupt with a clean
- * slate. This signal should be false unless there was a bug in
- * source code. Hence, use assert rather than CHECK.
- */
- assert(!target_vcpu->requires_deactivate_call);
-
/* Set the interrupt pending in the target vCPU. */
vcpu_virt_interrupt_inject(target_vcpu_locked, v_intid);
@@ -834,7 +803,6 @@
target_vcpu->scheduling_mode = SPMC_MODE;
target_vcpu->rt_model = RTM_SEC_INTERRUPT;
target_vcpu->state = VCPU_STATE_RUNNING;
- target_vcpu->requires_deactivate_call = false;
}
bool ffa_interrupts_intercept_call(struct vcpu_locked current_locked,
diff --git a/src/vcpu.c b/src/vcpu.c
index 3175ac2..bf07bd3 100644
--- a/src/vcpu.c
+++ b/src/vcpu.c
@@ -477,7 +477,6 @@
vcpu = vcpu_locked.vcpu;
vcpu->preempted_vcpu = NULL;
- vcpu->requires_deactivate_call = false;
}
void vcpu_virt_interrupt_enable(struct vcpu_locked vcpu_locked,
diff --git a/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/el0/sp_helpers.c b/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/el0/sp_helpers.c
index 8e60322..d094e27 100644
--- a/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/el0/sp_helpers.c
+++ b/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/el0/sp_helpers.c
@@ -106,9 +106,6 @@
abort();
}
- /* Perform secure interrupt de-activation. */
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
-
if (yield_while_handling_sec_interrupt) {
struct ffa_value ret;
HFTEST_LOG("Yield cycles while handling secure interrupt");
diff --git a/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/secure_interrupts.c b/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/secure_interrupts.c
index beccba6..1712c9e 100644
--- a/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/secure_interrupts.c
+++ b/test/vmapi/ffa_secure_partitions/services/arch/aarch64/secure/secure_interrupts.c
@@ -62,23 +62,6 @@
HFTEST_LOG("Resuming the suspended command");
}
-static void deactivate_interrupt_and_yield(uint32_t intid)
-{
- /* Perform secure interrupt de-activation. */
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
-
- if (yield_while_handling_sec_interrupt) {
- struct ffa_value ret;
- HFTEST_LOG("Yield cycles while handling secure interrupt");
- ret = ffa_yield();
-
- ASSERT_EQ(ret.func, FFA_SUCCESS_32);
- HFTEST_LOG("Resuming secure interrupt handling");
- }
-
- exception_handler_set_last_interrupt(intid);
-}
-
static void irq_current(void)
{
uint32_t intid;
@@ -131,7 +114,6 @@
sp_sleep_active_wait(5);
}
- deactivate_interrupt_and_yield(intid);
break;
}
case IRQ_AP_REFCLK_BASE1_INTID: {
@@ -142,20 +124,17 @@
HFTEST_LOG("AP_REFCLK timer stopped: %u", intid);
cancel_ap_refclk_timer();
- deactivate_interrupt_and_yield(intid);
break;
}
case HF_VIRTUAL_TIMER_INTID: {
/* Disable the EL1 physical arch timer. */
timer_disable();
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
-
- exception_handler_set_last_interrupt(intid);
/* Configure timer to expire periodically. */
timer_set(periodic_timer_ms);
timer_start();
HFTEST_LOG("EL1 Physical timer stopped and restarted");
+
break;
}
default:
@@ -164,6 +143,20 @@
intid);
abort();
}
+
+ if (yield_while_handling_sec_interrupt) {
+ struct ffa_value ret;
+ HFTEST_LOG("Yield cycles while handling secure interrupt");
+ ret = ffa_yield();
+
+ ASSERT_EQ(ret.func, FFA_SUCCESS_32);
+ HFTEST_LOG("Resuming secure interrupt handling");
+ }
+
+ /* Don't log NPIs as these are used during test coordination. */
+ if (intid != HF_NOTIFICATION_PENDING_INTID) {
+ exception_handler_set_last_interrupt(intid);
+ }
}
struct ffa_value sp_virtual_interrupt_cmd(ffa_id_t test_source,
diff --git a/test/vmapi/primary_with_secondaries/services/arch/aarch64/secure/secure_interrupts.c b/test/vmapi/primary_with_secondaries/services/arch/aarch64/secure/secure_interrupts.c
index 04f10fb..f01163b 100644
--- a/test/vmapi/primary_with_secondaries/services/arch/aarch64/secure/secure_interrupts.c
+++ b/test/vmapi/primary_with_secondaries/services/arch/aarch64/secure/secure_interrupts.c
@@ -94,22 +94,25 @@
HFTEST_LOG("Received Trusted WatchDog Interrupt: %u.", intid);
twdog_stop();
- /* Perform secure interrupt de-activation. */
+ /*
+ * Keep the call to hf_interrupt_deactive although it is
+ * deprecated to test backwards compatibility.
+ */
ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
+ /* Perform secure interrupt de-activation. */
break;
case RTM_INIT_ESPI_ID:
HFTEST_LOG("interrupt id: %u", intid);
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
rtm_init_espi_handled = true;
break;
case HF_IPI_INTID:
- HFTEST_LOG(
- "Received Inter-Processor Interrupt %u, "
- "partition %x.\n",
- intid, hf_vm_get_id());
- ASSERT_TRUE(hftest_ipi_state_is(SENT));
+ HFTEST_LOG("Received inter-processor interrupt %u, vm %x.",
+ intid, hf_vm_get_id());
+ ASSERT_TRUE(hftest_ipi_state_is(SENT) ||
+ (hftest_ipi_state_is(HANDLED) &&
+ hftest_ipi_state_get_interrupt_count() > 0 &&
+ multiple_interrupts_expected));
hftest_ipi_state_set(HANDLED);
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
break;
default:
panic("Interrupt ID not recongnised\n");
@@ -300,7 +303,6 @@
hftest_twdog_state_set(HANDLED);
/* Perform secure interrupt de-activation. */
- ASSERT_EQ(hf_interrupt_deactivate(intid), 0);
} else if (intid == HF_NOTIFICATION_PENDING_INTID) {
/* RX buffer full notification. */
HFTEST_LOG("Received notification pending interrupt %u.",
@@ -546,36 +548,6 @@
ffa_yield();
}
-/*
- * IRQ Handler for IPIs to target vCPUs in the WAITING state. In this case
- * the SP is not required to deactivate the interrupt.
- * TODO: Consolidate with the generic IRQ handler once the secure interrupt
- * reworking is done.
- */
-static void ipi_waiting_irq_handler(void)
-{
- uint32_t intid = hf_interrupt_get();
-
- switch (intid) {
- case HF_NOTIFICATION_PENDING_INTID:
- /* RX buffer full notification. */
- HFTEST_LOG("Received notification pending interrupt %u.",
- intid);
- break;
- case HF_IPI_INTID:
- HFTEST_LOG("Received inter-processor interrupt %u, vm %x.",
- intid, hf_vm_get_id());
- ASSERT_TRUE(hftest_ipi_state_is(SENT) ||
- (hftest_ipi_state_is(HANDLED) &&
- hftest_ipi_state_get_interrupt_count() > 0 &&
- multiple_interrupts_expected));
- hftest_ipi_state_set(HANDLED);
- break;
- default:
- panic("Invalid interrupt received: %u\n", intid);
- }
-}
-
/**
* Test service to validate IPI behaviour when target vCPU is in the waiting
* state.
@@ -593,7 +565,7 @@
{
struct ffa_value ret;
- exception_setup(ipi_waiting_irq_handler, NULL);
+ exception_setup(irq_handler, NULL);
interrupts_enable();
/* Enable the IPI. */