Module Name:    src
Committed By:   riastradh
Date:           Tue Jul 16 22:44:38 UTC 2024

Modified Files:
        src/common/lib/libc/arch/i386/atomic: atomic.S
        src/common/lib/libc/arch/x86_64/atomic: atomic.S
        src/sys/arch/amd64/amd64: cpufunc.S
        src/sys/arch/i386/i386: cpufunc.S
        src/sys/arch/xen/include: hypervisor.h xenring.h

Log Message:
xen: Don't hotpatch away LOCK prefix in xen_mb, even on UP boots.

Both xen_mb and membar_sync are designed to provide store-before-load
ordering, but xen_mb has to provide it in synchronizing guest with
hypervisor, while membar_sync only has to provide it in synchronizing
one (guest) CPU with another (guest) CPU.

It is safe to hotpatch away the LOCK prefix in membar_sync on a
uniprocessor boot because membar_sync is only designed to coordinate
between normal memory on multiple CPUs, and is never necessary when
there's only one CPU involved.

But xen_mb is used to coordinate between the guest and the `device'
implemented by a hypervisor, which might be running on another
_physical_ CPU even if the NetBSD guest only sees one `CPU', i.e.,
one _virtual_ CPU.  So even on `uniprocessor' boots, xen_mb must
still issue an instruction with store-before-load ordering on
multiprocessor systems, such as a LOCK ADD (or MFENCE, but MFENCE is
costlier for no benefit here).

No need to change xen_wmb (release ordering, load/store-before-store)
or xen_rmb (acquire ordering, load-before-load/store) because every
x86 store is a store-release and every x86 load is a load-acquire,
even on multiprocessor systems, so there's no hotpatching involved
anyway.

PR kern/57199


To generate a diff of this commit:
cvs rdiff -u -r1.36 -r1.37 src/common/lib/libc/arch/i386/atomic/atomic.S
cvs rdiff -u -r1.29 -r1.30 src/common/lib/libc/arch/x86_64/atomic/atomic.S
cvs rdiff -u -r1.67 -r1.68 src/sys/arch/amd64/amd64/cpufunc.S
cvs rdiff -u -r1.51 -r1.52 src/sys/arch/i386/i386/cpufunc.S
cvs rdiff -u -r1.59 -r1.60 src/sys/arch/xen/include/hypervisor.h
cvs rdiff -u -r1.7 -r1.8 src/sys/arch/xen/include/xenring.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/common/lib/libc/arch/i386/atomic/atomic.S
diff -u src/common/lib/libc/arch/i386/atomic/atomic.S:1.36 src/common/lib/libc/arch/i386/atomic/atomic.S:1.37
--- src/common/lib/libc/arch/i386/atomic/atomic.S:1.36	Sat Jul 30 14:11:00 2022
+++ src/common/lib/libc/arch/i386/atomic/atomic.S	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: atomic.S,v 1.36 2022/07/30 14:11:00 riastradh Exp $	*/
+/*	$NetBSD: atomic.S,v 1.37 2024/07/16 22:44:38 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2007 The NetBSD Foundation, Inc.
@@ -211,6 +211,8 @@ ENTRY(_membar_sync)
 	 * https://pvk.ca/Blog/2014/10/19/performance-optimisation-~-writing-an-essay/
 	 * https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
 	 * https://www.agner.org/optimize/instruction_tables.pdf
+	 *
+	 * Sync with xen_mb in sys/arch/i386/i386/cpufunc.S.
 	 */
 	LOCK
 	addl	$0, -4(%esp)

Index: src/common/lib/libc/arch/x86_64/atomic/atomic.S
diff -u src/common/lib/libc/arch/x86_64/atomic/atomic.S:1.29 src/common/lib/libc/arch/x86_64/atomic/atomic.S:1.30
--- src/common/lib/libc/arch/x86_64/atomic/atomic.S:1.29	Sat Jul 30 14:11:00 2022
+++ src/common/lib/libc/arch/x86_64/atomic/atomic.S	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: atomic.S,v 1.29 2022/07/30 14:11:00 riastradh Exp $	*/
+/*	$NetBSD: atomic.S,v 1.30 2024/07/16 22:44:38 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2007 The NetBSD Foundation, Inc.
@@ -279,6 +279,8 @@ ENTRY(_membar_sync)
 	 * https://pvk.ca/Blog/2014/10/19/performance-optimisation-~-writing-an-essay/
 	 * https://shipilev.net/blog/2014/on-the-fence-with-dependencies/
 	 * https://www.agner.org/optimize/instruction_tables.pdf
+	 *
+	 * Sync with xen_mb in sys/arch/amd64/amd64/cpufunc.S.
 	 */
 	LOCK
 	addq	$0, -8(%rsp)

Index: src/sys/arch/amd64/amd64/cpufunc.S
diff -u src/sys/arch/amd64/amd64/cpufunc.S:1.67 src/sys/arch/amd64/amd64/cpufunc.S:1.68
--- src/sys/arch/amd64/amd64/cpufunc.S:1.67	Fri Nov  3 09:07:56 2023
+++ src/sys/arch/amd64/amd64/cpufunc.S	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: cpufunc.S,v 1.67 2023/11/03 09:07:56 chs Exp $	*/
+/*	$NetBSD: cpufunc.S,v 1.68 2024/07/16 22:44:38 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2007, 2008, 2020, 2023 The NetBSD Foundation, Inc.
@@ -61,6 +61,28 @@ ENTRY(x86_mfence)
 	ret
 END(x86_mfence)
 
+#ifdef XEN
+ENTRY(xen_mb)
+	/*
+	 * Store-before-load ordering with respect to matching logic
+	 * on the hypervisor side.
+	 *
+	 * This is the same as membar_sync, but without hotpatching
+	 * away the LOCK prefix on uniprocessor boots -- because under
+	 * Xen, we still have to coordinate with a `device' backed by a
+	 * hypervisor that is potentially on another physical CPU even
+	 * if we observe only one virtual CPU as the guest.
+	 *
+	 * See common/lib/libc/arch/x86_64/atomic/atomic.S for
+	 * rationale and keep this in sync with the implementation
+	 * of membar_sync there.
+	 */
+	lock
+	addq	$0,-8(%rsp)
+	ret
+END(xen_mb)
+#endif	/* XEN */
+
 #ifndef XENPV
 ENTRY(invlpg)
 #ifdef SVS

Index: src/sys/arch/i386/i386/cpufunc.S
diff -u src/sys/arch/i386/i386/cpufunc.S:1.51 src/sys/arch/i386/i386/cpufunc.S:1.52
--- src/sys/arch/i386/i386/cpufunc.S:1.51	Fri Nov  3 09:07:56 2023
+++ src/sys/arch/i386/i386/cpufunc.S	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: cpufunc.S,v 1.51 2023/11/03 09:07:56 chs Exp $	*/
+/*	$NetBSD: cpufunc.S,v 1.52 2024/07/16 22:44:38 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 1998, 2007, 2020, 2023 The NetBSD Foundation, Inc.
@@ -39,7 +39,7 @@
 
 #include <machine/asm.h>
 #include <machine/frameasm.h>
-__KERNEL_RCSID(0, "$NetBSD: cpufunc.S,v 1.51 2023/11/03 09:07:56 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cpufunc.S,v 1.52 2024/07/16 22:44:38 riastradh Exp $");
 
 #include "opt_dtrace.h"
 #include "opt_xen.h"
@@ -67,6 +67,28 @@ ENTRY(x86_mfence)
 	ret
 END(x86_mfence)
 
+#ifdef XEN
+ENTRY(xen_mb)
+	/*
+	 * Store-before-load ordering with respect to matching logic
+	 * on the hypervisor side.
+	 *
+	 * This is the same as membar_sync, but without hotpatching
+	 * away the LOCK prefix on uniprocessor boots -- because under
+	 * Xen, we still have to coordinate with a `device' backed by a
+	 * hypervisor that is potentially on another physical CPU even
+	 * if we observe only one virtual CPU as the guest.
+	 *
+	 * See common/lib/libc/arch/i386/atomic/atomic.S for
+	 * rationale and keep this in sync with the implementation
+	 * of membar_sync there.
+	 */
+	lock
+	addl	$0,-4(%esp)
+	ret
+END(xen_mb)
+#endif	/* XEN */
+
 #ifndef XENPV
 ENTRY(lidt)
 	movl	4(%esp), %eax

Index: src/sys/arch/xen/include/hypervisor.h
diff -u src/sys/arch/xen/include/hypervisor.h:1.59 src/sys/arch/xen/include/hypervisor.h:1.60
--- src/sys/arch/xen/include/hypervisor.h:1.59	Mon Oct 16 17:31:17 2023
+++ src/sys/arch/xen/include/hypervisor.h	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: hypervisor.h,v 1.59 2023/10/16 17:31:17 bouyer Exp $	*/
+/*	$NetBSD: hypervisor.h,v 1.60 2024/07/16 22:44:38 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2006 Manuel Bouyer.
@@ -112,7 +112,7 @@ struct xen_npx_attach_args {
 #undef xen_rmb
 #undef xen_wmb
 
-#define xen_mb()  membar_sync()
+void xen_mb(void);
 #define xen_rmb() membar_acquire()
 #define xen_wmb() membar_release()
 #endif /* __XEN_INTERFACE_VERSION */

Index: src/sys/arch/xen/include/xenring.h
diff -u src/sys/arch/xen/include/xenring.h:1.7 src/sys/arch/xen/include/xenring.h:1.8
--- src/sys/arch/xen/include/xenring.h:1.7	Sat Feb 25 00:32:26 2023
+++ src/sys/arch/xen/include/xenring.h	Tue Jul 16 22:44:38 2024
@@ -1,4 +1,4 @@
-/* $NetBSD: xenring.h,v 1.7 2023/02/25 00:32:26 riastradh Exp $ */
+/* $NetBSD: xenring.h,v 1.8 2024/07/16 22:44:38 riastradh Exp $ */
 
 /*
  * Glue goop for xbd ring request/response protocol structures.
@@ -24,7 +24,7 @@
 #undef xen_rmb
 #undef xen_wmb
 
-#define xen_mb()  membar_sync()
+void xen_mb(void);
 #define xen_rmb() membar_acquire()
 #define xen_wmb() membar_release()
 

Reply via email to