git: e769bc771843 - main - sym(4): Employ memory barriers also on x86
Date: Mon, 26 Jan 2026 15:57:45 UTC
The branch main has been updated by marius:
URL: https://cgit.FreeBSD.org/src/commit/?id=e769bc77184312b6137a9b180c97b87c0760b849
commit e769bc77184312b6137a9b180c97b87c0760b849
Author: Marius Strobl <marius@FreeBSD.org>
AuthorDate: 2026-01-26 13:58:57 +0000
Commit: Marius Strobl <marius@FreeBSD.org>
CommitDate: 2026-01-26 15:54:48 +0000
sym(4): Employ memory barriers also on x86
In an MP world, it doesn't hold that x86 requires no memory barriers.
This change should also fix panics due to out-of-sync data seen with
FreeBSD VMs on top of OpenStack and HBAs of type lsiLogic. [1]
While at it:
- Improve the granularity somewhat by distinguishing between read and
write memory barriers as well as refer to existing *mb(9) functions
instead of duplicating these [2], unless IO barriers are also used.
- Nuke the unused SYM_DRIVER_NAME macro.
PR: 270816 [1]
Obtained from: BSD-licensed Linux sym53c8xx driver [2]
MFC after: 1 week
---
sys/dev/sym/sym_hipd.c | 42 +++++++++++++++---------------------------
1 file changed, 15 insertions(+), 27 deletions(-)
diff --git a/sys/dev/sym/sym_hipd.c b/sys/dev/sym/sym_hipd.c
index 0e51607fb07a..f78d595a73ce 100644
--- a/sys/dev/sym/sym_hipd.c
+++ b/sys/dev/sym/sym_hipd.c
@@ -58,7 +58,6 @@
*/
#include <sys/cdefs.h>
-#define SYM_DRIVER_NAME "sym-1.6.5-20000902"
/* #define SYM_DEBUG_GENERIC_SUPPORT */
@@ -114,27 +113,16 @@ typedef u_int32_t u32;
#include <dev/sym/sym_fw.h>
/*
- * IA32 architecture does not reorder STORES and prevents
- * LOADS from passing STORES. It is called `program order'
- * by Intel and allows device drivers to deal with memory
- * ordering by only ensuring that the code is not reordered
- * by the compiler when ordering is required.
- * Other architectures implement a weaker ordering that
- * requires memory barriers (and also IO barriers when they
- * make sense) to be used.
- */
-#if defined __i386__ || defined __amd64__
-#define MEMORY_BARRIER() do { ; } while(0)
-#elif defined __powerpc__
-#define MEMORY_BARRIER() __asm__ volatile("eieio; sync" : : : "memory")
-#elif defined __arm__
-#define MEMORY_BARRIER() dmb()
-#elif defined __aarch64__
-#define MEMORY_BARRIER() dmb(sy)
-#elif defined __riscv
-#define MEMORY_BARRIER() fence()
+ * Architectures may implement weak ordering that requires memory barriers
+ * to be used for LOADS and STORES to become globally visible (and also IO
+ * barriers when they make sense).
+ */
+#ifdef __powerpc__
+#define MEMORY_READ_BARRIER() __asm__ volatile("eieio; sync" : : : "memory")
+#define MEMORY_WRITE_BARRIER() MEMORY_READ_BARRIER()
#else
-#error "Not supported platform"
+#define MEMORY_READ_BARRIER() rmb()
+#define MEMORY_WRITE_BARRIER() wmb()
#endif
/*
@@ -892,13 +880,13 @@ struct sym_nvram {
*/
#define OUTL_DSP(v) \
do { \
- MEMORY_BARRIER(); \
+ MEMORY_WRITE_BARRIER(); \
OUTL (nc_dsp, (v)); \
} while (0)
#define OUTONB_STD() \
do { \
- MEMORY_BARRIER(); \
+ MEMORY_WRITE_BARRIER(); \
OUTONB (nc_dcntl, (STD|NOCOM)); \
} while (0)
@@ -2908,7 +2896,7 @@ static void sym_put_start_queue(hcb_p np, ccb_p cp)
if (qidx >= MAX_QUEUE*2) qidx = 0;
np->squeue [qidx] = cpu_to_scr(np->idletask_ba);
- MEMORY_BARRIER();
+ MEMORY_WRITE_BARRIER();
np->squeue [np->squeueput] = cpu_to_scr(cp->ccb_ba);
np->squeueput = qidx;
@@ -2920,7 +2908,7 @@ static void sym_put_start_queue(hcb_p np, ccb_p cp)
* Script processor may be waiting for reselect.
* Wake it up.
*/
- MEMORY_BARRIER();
+ MEMORY_WRITE_BARRIER();
OUTB (nc_istat, SIGP|np->istat_sem);
}
@@ -3061,7 +3049,7 @@ static int sym_wakeup_done (hcb_p np)
cp = sym_ccb_from_dsa(np, dsa);
if (cp) {
- MEMORY_BARRIER();
+ MEMORY_READ_BARRIER();
sym_complete_ok (np, cp);
++n;
} else
@@ -3859,7 +3847,7 @@ static void sym_intr1 (hcb_p np)
* On paper, a memory barrier may be needed here.
* And since we are paranoid ... :)
*/
- MEMORY_BARRIER();
+ MEMORY_READ_BARRIER();
/*
* First, interrupts we want to service cleanly.