From 424fc73a46dfdd606c21d362f11b46a6602d11f6 Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Tue, 14 Nov 2017 10:52:20 +0000 Subject: [PATCH 1/4] SDEI: Fix security state check for explicit dispatch Change-Id: Ic381ab5d03ec68c7f6e8d357ac2e2cbf0cc6b2e8 Signed-off-by: Jeenu Viswambharan --- services/std_svc/sdei/sdei_intr_mgmt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/std_svc/sdei/sdei_intr_mgmt.c b/services/std_svc/sdei/sdei_intr_mgmt.c index 4551a8b1e..42bf46d0f 100644 --- a/services/std_svc/sdei/sdei_intr_mgmt.c +++ b/services/std_svc/sdei/sdei_intr_mgmt.c @@ -475,8 +475,10 @@ int sdei_dispatch_event(int ev_num, unsigned int preempted_sec_state) sdei_cpu_state_t *state; /* Validate preempted security state */ - if ((preempted_sec_state != SECURE) || (preempted_sec_state != NON_SECURE)) + if ((preempted_sec_state != SECURE) && + (preempted_sec_state != NON_SECURE)) { return -1; + } /* Can't dispatch if events are masked on this PE */ state = sdei_get_this_pe_state(); From b968241faf9455f461fc896d9c62fbb902559311 Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Tue, 14 Nov 2017 15:35:41 +0000 Subject: [PATCH 2/4] SDEI: Fix type of register count Register count is currently declared as unsigned, where as there are asserts in place to check it being negative during unregister. These are flagged as never being true. Change-Id: I34f00f0ac5bf88205791e9c1298a175dababe7c8 Signed-off-by: Jeenu Viswambharan --- include/services/sdei.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/services/sdei.h b/include/services/sdei.h index b07e93b11..ce9a008c5 100644 --- a/include/services/sdei.h +++ b/include/services/sdei.h @@ -154,7 +154,7 @@ typedef struct sdei_ev_map { int32_t ev_num; /* Event number */ unsigned int intr; /* Physical interrupt number for a bound map */ unsigned int map_flags; /* Mapping flags, see SDEI_MAPF_* */ - unsigned int reg_count; /* Registration count */ + int reg_count; /* Registration count */ spinlock_t lock; /* Per-event lock */ } sdei_ev_map_t; From f1a67d0565fcc774790030db6a98804d2ea91619 Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Thu, 16 Nov 2017 12:06:34 +0000 Subject: [PATCH 3/4] SDEI: Assert that dynamic events have Normal priority The SDEI specification requires that binding a client interrupt dispatches SDEI Normal priority event. This means that dynamic events can't have Critical priority. Add asserts for this. Change-Id: I0bdd9e0e642fb2b61810cb9f4cbfbd35bba521d1 Signed-off-by: Jeenu Viswambharan --- services/std_svc/sdei/sdei_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/std_svc/sdei/sdei_main.c b/services/std_svc/sdei/sdei_main.c index 4fe990ad5..2f08c8ba9 100644 --- a/services/std_svc/sdei/sdei_main.c +++ b/services/std_svc/sdei/sdei_main.c @@ -120,6 +120,7 @@ void sdei_class_init(sdei_class_t class) /* Platform events are always bound, so set the bound flag */ if (is_map_dynamic(map)) { assert(map->intr == SDEI_DYN_IRQ); + assert(is_event_normal(map)); num_dyn_shrd_slots++; } else { /* Shared mappings must be bound to shared interrupt */ @@ -171,6 +172,7 @@ void sdei_class_init(sdei_class_t class) if (map->ev_num != SDEI_EVENT_0) { if (is_map_dynamic(map)) { assert(map->intr == SDEI_DYN_IRQ); + assert(is_event_normal(map)); num_dyn_priv_slots++; } else { /* From 1a0f8f3957a371b05a147cbfbd8826fa14ae407f Mon Sep 17 00:00:00 2001 From: Jeenu Viswambharan Date: Thu, 16 Nov 2017 12:34:15 +0000 Subject: [PATCH 4/4] SDEI: Update doc to clarify delegation The explicit event dispatch sequence currently depicts handling done in Secure EL1, although further error handling is typically done inside a Secure Partition. Clarify the sequence diagram to that effect. Change-Id: I53deedc6d5ee0706626890067950c2c541a62c78 Signed-off-by: Jeenu Viswambharan --- docs/plantuml/sdei_explicit_dispatch.puml | 12 +++++------ docs/plantuml/sdei_explicit_dispatch.svg | 2 +- docs/sdei.rst | 25 +++++++++++++---------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/docs/plantuml/sdei_explicit_dispatch.puml b/docs/plantuml/sdei_explicit_dispatch.puml index 51214f536..c80fcd118 100644 --- a/docs/plantuml/sdei_explicit_dispatch.puml +++ b/docs/plantuml/sdei_explicit_dispatch.puml @@ -9,7 +9,7 @@ autonumber "[#]" participant "SDEI client" as EL2 participant EL3 -participant SEL1 +participant "Secure Partition" as SP activate EL2 EL2->EL3: **SDEI_EVENT_REGISTER**(ev, handler, ...) @@ -24,11 +24,11 @@ EL3->EL2: 1 EL3<--]: **CRITICAL EVENT** activate EL3 #red note over EL3: Critical event triage -EL3->SEL1: dispatch -activate SEL1 #salmon -note over SEL1: Critical event handling -SEL1->EL3: done -deactivate SEL1 +EL3->SP: dispatch +activate SP #salmon +note over SP: Critical event handling +SP->EL3: done +deactivate SP EL3-->EL3: sdei_dispatch_event(ev) note over EL3: Prepare SDEI dispatch EL3->EL2: dispatch diff --git a/docs/plantuml/sdei_explicit_dispatch.svg b/docs/plantuml/sdei_explicit_dispatch.svg index b33944e54..182df0af6 100644 --- a/docs/plantuml/sdei_explicit_dispatch.svg +++ b/docs/plantuml/sdei_explicit_dispatch.svg @@ -1 +1 @@ -SDEI clientSDEI clientEL3EL3SEL1SEL1[1]SDEI_EVENT_REGISTER(ev, handler, ...)[2]success[3]SDEI_EVENT_ENABLE(ev)[4]success[5]SDEI_PE_UNMASK()[6]1<<Business as usual>>[7]CRITICAL EVENTCritical event triage[8]dispatchCritical event handling[9]done[10]sdei_dispatch_event(ev)Prepare SDEI dispatch[11]dispatchSDEI handler[12]SDEI_EVENT_COMPLETE()Complete SDEI dispatch[13]resumes preempted execution<<Normal execution resumes>> \ No newline at end of file +SDEI clientSDEI clientEL3EL3Secure PartitionSecure Partition[1]SDEI_EVENT_REGISTER(ev, handler, ...)[2]success[3]SDEI_EVENT_ENABLE(ev)[4]success[5]SDEI_PE_UNMASK()[6]1<<Business as usual>>[7]CRITICAL EVENTCritical event triage[8]dispatchCritical event handling[9]done[10]sdei_dispatch_event(ev)Prepare SDEI dispatch[11]dispatchSDEI handler[12]SDEI_EVENT_COMPLETE()Complete SDEI dispatch[13]resumes preempted execution<<Normal execution resumes>> \ No newline at end of file diff --git a/docs/sdei.rst b/docs/sdei.rst index 0731a5a81..a67b72486 100644 --- a/docs/sdei.rst +++ b/docs/sdei.rst @@ -232,13 +232,20 @@ bound or dynamic events can't be explicitly dispatched (see the section below). At a later point in time, a critical event [#critical-event]_ is trapped into EL3 [7]. EL3 performs a first-level triage of the event, and decides to dispatch -to Secure EL1 for further handling [8]. The dispatch completes, but intends to -involve Non-secure world in further handling, and therefore decides to -explicitly dispatch an event [10] (which the client had already registered for -[1]). The rest of the sequence is similar to that in the `general SDEI -dispatch`_: the requested event is dispatched to the client (assuming all the -conditions are met), and when the handler completes, the preempted execution -resumes. +to a Secure Partition [#secpart]_ for further handling [8]. The dispatch +completes, but intends to involve Non-secure world in further handling, and +therefore decides to explicitly dispatch an event [10] (which the client had +already registered for [1]). The rest of the sequence is similar to that in the +`general SDEI dispatch`_: the requested event is dispatched to the client +(assuming all the conditions are met), and when the handler completes, the +preempted execution resumes. + +.. [#critical-event] Examples of critical event are *SError*, *Synchronous + External Abort*, *Fault Handling interrupt*, or *Error + Recovery interrupt* from one of RAS nodes in the system. + +.. [#secpart] Dispatching to Secure Partition involves *Secure Partition + Manager*, which isn't depicted in the sequence. Conditions for event dispatch ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -295,10 +302,6 @@ dispatcher: context is resumed (as indicated by the ``preempted_sec_state`` parameter of the API). -.. [#critical-event] Examples of critical event are *SError*, *Synchronous - External Abort*, *Fault Handling interrupt*, or *Error - Recovery interrupt* from one of RAS nodes in the system. - Porting requirements --------------------