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()