From: Jan Kiszka <[email protected]>

On ARM, the spin_unlock is sufficient to provide a memory barrier before
calling arch_send_event. On other archs, the implementation of
arch_send_event has to take care of this. Clarify this at the respective
call sites and the function documentation.

Signed-off-by: Jan Kiszka <[email protected]>
---
 hypervisor/arch/arm-common/psci.c      | 5 +++++
 hypervisor/control.c                   | 4 ++++
 hypervisor/include/jailhouse/control.h | 5 +++++
 3 files changed, 14 insertions(+)

diff --git a/hypervisor/arch/arm-common/psci.c 
b/hypervisor/arch/arm-common/psci.c
index 65155710..6a9abf60 100644
--- a/hypervisor/arch/arm-common/psci.c
+++ b/hypervisor/arch/arm-common/psci.c
@@ -46,6 +46,11 @@ static long psci_emulate_cpu_on(struct trap_context *ctx)
                result = PSCI_ALREADY_ON;
        }
 
+       /*
+        * The unlock has memory barrier semantic on ARM v7 and v8. Therefore
+        * the changes to target_data will be visible when sending the kick
+        * below.
+        */
        spin_unlock(&target_data->control_lock);
 
        if (kick_cpu)
diff --git a/hypervisor/control.c b/hypervisor/control.c
index 016f97cc..b38ac2e9 100644
--- a/hypervisor/control.c
+++ b/hypervisor/control.c
@@ -102,6 +102,10 @@ static void suspend_cpu(unsigned int cpu_id)
        target_data->suspend_cpu = true;
        target_suspended = target_data->cpu_suspended;
 
+       /*
+        * Acts as memory barrier on certain architectures to make suspend_cpu
+        * visible. Otherwise, arch_send_event() will take care of that.
+        */
        spin_unlock(&target_data->control_lock);
 
        if (!target_suspended) {
diff --git a/hypervisor/include/jailhouse/control.h 
b/hypervisor/include/jailhouse/control.h
index 5476d590..9b94f563 100644
--- a/hypervisor/include/jailhouse/control.h
+++ b/hypervisor/include/jailhouse/control.h
@@ -180,6 +180,11 @@ void arch_park_cpu(unsigned int cpu_id);
  * When the state of the target CPU was updated and action is required on the
  * remote side, this function can be called. Processing of the state change is
  * architecture specific.
+ *
+ * The caller of this function is required to have performed the state changes
+ * under a spinlock and called spin_unlock prior to this. The implementation of
+ * arch_send_event() has to account for the case when spin_unlock does not
+ * imply a memory barrier and issue this explicitly.
  */
 void arch_send_event(struct public_per_cpu *target_data);
 
-- 
2.16.4

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/19cf7a61caaa793bb533d38b531519e703cccf49.1583516039.git.jan.kiszka%40siemens.com.

Reply via email to