xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: [PATCH 4/4] xen/x86: Correct mandatory and SMP barrier definitions
Date: Mon, 5 Dec 2016 10:05:17 +0000	[thread overview]
Message-ID: <1480932317-22962-5-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <1480932317-22962-1-git-send-email-andrew.cooper3@citrix.com>

Barriers are a complicated topic, a common source of confusion in submitted
code, and their incorrect use is a common cause of bugs.  It *really* doesn't
help when Xen's API is the same as Linux, but its ABI different.

Bring the two back in line, so programmers stand a chance of actually getting
their use correct.

As Xen has no current need for mandatory barriers, leave them commented out to
avoid accidential misue.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/include/asm-x86/system.h        | 31 +++++++++++++++++++++++--------
 xen/include/asm-x86/x86_64/system.h |  3 ---
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index 9cb6fd7..9cd401a 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -164,23 +164,38 @@ static always_inline unsigned long __xadd(
     ((typeof(*(ptr)))__xadd(ptr, (typeof(*(ptr)))(v), sizeof(*(ptr))))
 
 /*
+ * Mandatory barriers, for the ordering of reads and writes with MMIO devices
+ * mapped with reduced cacheability.
+ *
+ * Xen has no such device drivers, and therefore no need for mandatory
+ * barriers.  These these are hidden to avoid their misuse; If a future need
+ * is found, they can be re-introduced, but chances are very good that a
+ * programmer actually should be using the smp_*() barriers.
+ *
+#define mb()            asm volatile ("mfence" ::: "memory")
+#define rmb()           asm volatile ("lfence" ::: "memory")
+#define wmb()           asm volatile ("sfence" ::: "memory")
+ */
+
+/*
+ * SMP barriers, for ordering of reads and writes between CPUs, most commonly
+ * used with shared memory.
+ *
  * Both Intel and AMD agree that, from a programmer's viewpoint:
  *  Loads cannot be reordered relative to other loads.
  *  Stores cannot be reordered relative to other stores.
- * 
+ *  Loads may be reordered ahead of an unaliasing store.
+ *
  * Intel64 Architecture Memory Ordering White Paper
  * <http://developer.intel.com/products/processor/manuals/318147.pdf>
- * 
+ *
  * AMD64 Architecture Programmer's Manual, Volume 2: System Programming
  * <http://www.amd.com/us-en/assets/content_type/\
  *  white_papers_and_tech_docs/24593.pdf>
  */
-#define rmb()           barrier()
-#define wmb()           barrier()
-
-#define smp_mb()        mb()
-#define smp_rmb()       rmb()
-#define smp_wmb()       wmb()
+#define smp_mb()        asm volatile ("mfence" ::: "memory")
+#define smp_rmb()       barrier()
+#define smp_wmb()       barrier()
 
 #define set_mb(var, value) do { xchg(&var, value); } while (0)
 #define set_wmb(var, value) do { var = value; smp_wmb(); } while (0)
diff --git a/xen/include/asm-x86/x86_64/system.h b/xen/include/asm-x86/x86_64/system.h
index 7026c05..bdf45e5 100644
--- a/xen/include/asm-x86/x86_64/system.h
+++ b/xen/include/asm-x86/x86_64/system.h
@@ -79,7 +79,4 @@ static always_inline __uint128_t __cmpxchg16b(
     _rc;                                                                \
 })
 
-#define mb()                    \
-    asm volatile ( "mfence" : : : "memory" )
-
 #endif /* __X86_64_SYSTEM_H__ */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  parent reply	other threads:[~2016-12-05 10:05 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-05 10:05 [PATCH 0/4] Fixes to common and x86 barriers Andrew Cooper
2016-12-05 10:05 ` [PATCH 1/4] xen/common: Replace incorrect mandatory barriers with SMP barriers Andrew Cooper
2016-12-05 10:55   ` Jan Beulich
2016-12-05 19:07   ` Stefano Stabellini
2016-12-05 10:05 ` [PATCH 2/4] xen/x86: Drop erronious barriers Andrew Cooper
2016-12-05 11:18   ` Jan Beulich
2016-12-05 11:25     ` Andrew Cooper
2016-12-05 12:28       ` Jan Beulich
2016-12-05 13:43         ` Andrew Cooper
2016-12-05 13:50           ` Jan Beulich
2016-12-05 13:59             ` Andrew Cooper
2016-12-05 14:07               ` Jan Beulich
2016-12-05 19:14                 ` Stefano Stabellini
2016-12-05 19:17   ` Stefano Stabellini
2016-12-06  0:10     ` Andrew Cooper
2016-12-06 20:27       ` Stefano Stabellini
2016-12-06 20:32         ` Stefano Stabellini
2016-12-07  1:03           ` Andrew Cooper
2016-12-07  1:20             ` Stefano Stabellini
2016-12-07  1:46               ` Andrew Cooper
2016-12-07 18:31           ` Julien Grall
2016-12-07 18:44             ` Stefano Stabellini
2016-12-07 18:55               ` Julien Grall
2016-12-05 10:05 ` [PATCH 3/4] xen/x86: Replace incorrect mandatory barriers with SMP barriers Andrew Cooper
2016-12-05 11:47   ` Jan Beulich
2016-12-05 13:29     ` Andrew Cooper
2016-12-05 14:03       ` Jan Beulich
2016-12-05 14:24         ` Andrew Cooper
2016-12-05 14:33           ` Jan Beulich
2016-12-05 10:05 ` Andrew Cooper [this message]
2016-12-05 10:11   ` [PATCH 4/4] xen/x86: Correct mandatory and SMP barrier definitions Juergen Gross
2016-12-05 13:45     ` Andrew Cooper
2016-12-05 11:51   ` Jan Beulich
2016-12-05 14:08     ` Andrew Cooper
2016-12-05 12:01   ` David Vrabel
2016-12-05 13:46     ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1480932317-22962-5-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).