From: Jan Kiszka <[email protected]>

Since 2017, the Intel manual suggests to use mfence plus lfence as
barrier to make data changes visible triggering an interrupt via the
x2APIC interface. Jailhouse was so far not using any barrier
consistently in those cases, neither for internal NMI IPIs, nor for
those triggered via the ivshmem doorbell interface.

This adds the recommended mfence;lfence sequence to all IPIs triggered
via apic_send_irq or apic_send_ipi, at the risk of having more than
needed, e.g. when issuing an IPI on behalf of a guest that already used
a barrier itself. Compared to the risk of missing a cases and given the
overhead that the intercepted IPI submission comes with anyway, this is
the preferable option.

Note that this also ensures proper serialization of data writes and
kicks for the ivshmem doorbell interface on x86. Such a property is
going to be demanded by the ivshmem specification.

Signed-off-by: Jan Kiszka <[email protected]>
---
 hypervisor/arch/x86/apic.c    | 11 +++++++++++
 hypervisor/arch/x86/control.c |  3 ---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hypervisor/arch/x86/apic.c b/hypervisor/arch/x86/apic.c
index 04b46501..d36c2033 100644
--- a/hypervisor/arch/x86/apic.c
+++ b/hypervisor/arch/x86/apic.c
@@ -89,6 +89,10 @@ static void send_xapic_ipi(u32 apic_id, u32 icr_lo)
 {
        while (read_xapic(APIC_REG_ICR) & APIC_ICR_DS_PENDING)
                cpu_relax();
+       /*
+        * No need for an explicit barrier, the mmio access serves as
+        * implicit one.
+        */
        mmio_write32(xapic_page + XAPIC_REG(APIC_REG_ICR_HI),
                     apic_id << XAPIC_DEST_SHIFT);
        mmio_write32(xapic_page + XAPIC_REG(APIC_REG_ICR), icr_lo);
@@ -111,6 +115,13 @@ static void write_x2apic(unsigned int reg, u32 val)
 
 static void send_x2apic_ipi(u32 apic_id, u32 icr_lo)
 {
+       /*
+        * Intel SDM, Volume 3, 10.12.3:
+        * We either have to execute a serializing instruction or the
+        * mfence;lfence sequence to publish data changes before the IPI goes
+        * out. The latter is clearer and likely also faster.
+        */
+       asm volatile("mfence; lfence" : : : "memory");
        write_msr(MSR_X2APIC_BASE + APIC_REG_ICR,
                  ((unsigned long)apic_id) << 32 | icr_lo);
 }
diff --git a/hypervisor/arch/x86/control.c b/hypervisor/arch/x86/control.c
index 4f667d43..2aea807a 100644
--- a/hypervisor/arch/x86/control.c
+++ b/hypervisor/arch/x86/control.c
@@ -77,9 +77,6 @@ void arch_flush_cell_vcpu_caches(struct cell *cell)
                        vcpu_tlb_flush();
                } else {
                        public_per_cpu(cpu)->flush_vcpu_caches = true;
-                       /* make sure the value is written before we kick
-                        * the remote core */
-                       memory_barrier();
                        apic_send_nmi_ipi(public_per_cpu(cpu));
                }
 }
-- 
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/564f354155a3a21e95041c1b8fb79605356abf65.1583516039.git.jan.kiszka%40siemens.com.

Reply via email to