On Mon, Jan 26, 2026 at 03:57:45PM +0000, Marius Strobl wrote:
> The branch main has been updated by marius:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=e769bc77184312b6137a9b180c97b87c0760b849
>
> commit e769bc77184312b6137a9b180c97b87c0760b849
> Author: Marius Strobl <[email protected]>
> AuthorDate: 2026-01-26 13:58:57 +0000
> Commit: Marius Strobl <[email protected]>
> 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.
It does hold. x86 is much more strongly ordered than all other arches
we currently support.
That said, the use of the barriers in drivers is usually not justified
(I did not looked at this specific case).
Even if needed, please stop using rmb/wmb etc. Use atomic_thread_fence()
of appropriate kind, see atomic(9). Then on x86 it does the right thing.
>
> 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.
