With the recent simplifications, it becomes obvious that smp_mb() isn't the
right barrier. Strictly speaking, MONITOR is ordered as a load, but smp_rmb()
isn't correct either, as this only pertains to local ordering. All we need is
a compiler barrier().
Merge the barier() into the monitor() itself, along with an explantion.
No functional change.
Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Jan Beulich <jbeul...@suse.com>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Anthony PERARD <anthony.per...@vates.tech>
CC: Michal Orzel <michal.or...@amd.com>
CC: Julien Grall <jul...@xen.org>
CC: Stefano Stabellini <sstabell...@kernel.org>
v2:
* Move earlier in the series, not that it matters IMO.
* Expand the commit message.
---
xen/arch/x86/acpi/cpu_idle.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 141f0f0facf6..68dd44be5bb0 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -66,8 +66,12 @@ static always_inline void monitor(
alternative_input("", "clflush (%[addr])", X86_BUG_CLFLUSH_MONITOR,
[addr] "a" (addr));
+ /*
+ * The memory clobber is a compiler barrier. Subseqeunt reads from the
+ * monitored cacheline must not be reordered over MONITOR.
+ */
asm volatile ( "monitor"
- :: "a" (addr), "c" (ecx), "d" (edx) );
+ :: "a" (addr), "c" (ecx), "d" (edx) : "memory" );
}
static always_inline void mwait(unsigned int eax, unsigned int ecx)
@@ -453,7 +457,6 @@ void mwait_idle_with_hints(unsigned int eax, unsigned int
ecx)
const unsigned int *this_softirq_pending = &softirq_pending(cpu);
monitor(this_softirq_pending, 0, 0);
- smp_mb();
if ( !*this_softirq_pending )
{
--
2.39.5
