In case of an ivshmem link between two non-root cells, one can go down
while the other is running. That is different from root to non-root
links, because the non-root cell always leaves first, and the root cell
is stopped meanwhile. We have to avoid the, after destroying one peer,
the other will still send interrupts to the former peer.

The simplest way to achieve this is using a spinlock around
dereferencing the remote pointer of an ivshmem endpoint and sending out
the interrupt to the peer on one side and around the clearing of the
remote pointer during destruction. The latter will ensure that the
sender is either already done with the transmission or will see NULL as
remote, thus will not send any message anymore. This resolves the race
between flushing pending interrupts from the destructed cell and sending
new ones from the former peer cell.

Spinlocks come with memory barrier semantics, so we can remove the
explicit barriers.

Now that we have the lock, we can also synchronize the rstate readout of
the remaining peer with device destruction to ensure that no other CPU
will consider the device alive after ivshmem_exit returned.

Signed-off-by: Jan Kiszka <[email protected]>
---
 hypervisor/include/jailhouse/ivshmem.h |  2 ++
 hypervisor/ivshmem.c                   | 32 +++++++++++++++++++++-----------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/hypervisor/include/jailhouse/ivshmem.h 
b/hypervisor/include/jailhouse/ivshmem.h
index b6a1044..82a4837 100644
--- a/hypervisor/include/jailhouse/ivshmem.h
+++ b/hypervisor/include/jailhouse/ivshmem.h
@@ -16,6 +16,7 @@
 
 #include <jailhouse/pci.h>
 #include <asm/ivshmem.h>
+#include <asm/spinlock.h>
 
 #define IVSHMEM_CFG_MSIX_CAP   0x50
 #define IVSHMEM_CFG_SIZE       (IVSHMEM_CFG_MSIX_CAP + 12)
@@ -35,6 +36,7 @@ struct ivshmem_endpoint {
        u64 bar4_address;
        struct pci_device *device;
        struct ivshmem_endpoint *remote;
+       spinlock_t remote_lock;
        struct arch_pci_ivshmem arch;
        u32 intx_ctrl_reg;
 };
diff --git a/hypervisor/ivshmem.c b/hypervisor/ivshmem.c
index 1e3d27d..bbf830d 100644
--- a/hypervisor/ivshmem.c
+++ b/hypervisor/ivshmem.c
@@ -72,10 +72,14 @@ static const u32 default_cspace[IVSHMEM_CFG_SIZE / 
sizeof(u32)] = {
 
 static void ivshmem_remote_interrupt(struct ivshmem_endpoint *ive)
 {
-       struct ivshmem_endpoint *remote = ive->remote;
-
-       if (remote)
+       /*
+        * Hold the remote lock while sending the interrupt so that
+        * ivshmem_exit can synchronize on the completion of the delivery.
+        */
+       spin_lock(&ive->remote_lock);
+       if (ive->remote)
                arch_ivshmem_trigger_interrupt(ive->remote);
+       spin_unlock(&ive->remote_lock);
 }
 
 static enum mmio_result ivshmem_register_mmio(void *arg,
@@ -110,7 +114,6 @@ static enum mmio_result ivshmem_register_mmio(void *arg,
        if (mmio->address == IVSHMEM_REG_LSTATE) {
                if (mmio->is_write) {
                        ive->state = mmio->value;
-                       memory_barrier();
                        ivshmem_remote_interrupt(ive);
                } else {
                        mmio->value = ive->state;
@@ -119,10 +122,9 @@ static enum mmio_result ivshmem_register_mmio(void *arg,
        }
 
        if (mmio->address == IVSHMEM_REG_RSTATE && !mmio->is_write) {
-               if (ive->remote)
-                       mmio->value = ive->remote->state;
-               else
-                       mmio->value = 0;
+               spin_lock(&ive->remote_lock);
+               mmio->value = ive->remote ? ive->remote->state : 0;
+               spin_unlock(&ive->remote_lock);
                return MMIO_HANDLED;
        }
 
@@ -398,11 +400,19 @@ int ivshmem_init(struct cell *cell, struct pci_device 
*device)
 void ivshmem_exit(struct pci_device *device)
 {
        struct ivshmem_endpoint *ive = device->ivshmem_endpoint;
+       struct ivshmem_endpoint *remote = ive->remote;
        struct ivshmem_data **ivp, *iv;
 
-       if (ive->remote) {
-               ive->remote->remote = NULL;
-               memory_barrier();
+       if (remote) {
+               /*
+                * The spinlock synchronizes the disconnection of the remote
+                * device with any in-flight interrupts targeting the device
+                * to be destroyed.
+                */
+               spin_lock(&remote->remote_lock);
+               remote->remote = NULL;
+               spin_unlock(&remote->remote_lock);
+
                ivshmem_remote_interrupt(ive);
 
                ive->device = NULL;
-- 
2.1.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].
For more options, visit https://groups.google.com/d/optout.

Reply via email to