>>> On 05.12.16 at 15:24, <andrew.coop...@citrix.com> wrote: > On 05/12/16 14:03, Jan Beulich wrote: >>>>> On 05.12.16 at 14:29, <andrew.coop...@citrix.com> wrote: >>> On 05/12/16 11:47, Jan Beulich wrote: >>>>>>> On 05.12.16 at 11:05, <andrew.coop...@citrix.com> wrote: >>>>> --- a/xen/arch/x86/acpi/cpu_idle.c >>>>> +++ b/xen/arch/x86/acpi/cpu_idle.c >>>>> @@ -391,9 +391,9 @@ void mwait_idle_with_hints(unsigned int eax, unsigned >>>>> int > ecx) >>>>> >>>>> if ( boot_cpu_has(X86_FEATURE_CLFLUSH_MONITOR) ) >>>>> { >>>>> - mb(); >>>>> + smp_mb(); >>>>> clflush((void *)&mwait_wakeup(cpu)); >>>>> - mb(); >>>>> + smp_mb(); >>>>> } >>>> Both need to stay as they are afaict: In order for the clflush() to do >>>> what we want we have to order it wrt earlier as well as later writes, >>>> regardless of SMP-ness. Or wait - the SDM has changed in that >>>> respect (and a footnote describes the earlier specified behavior now). >>>> AMD, otoh, continues to require MFENCE for ordering purposes. >>> mb() == smp_mb(). They are both mfence instructions. >> Of course. But still smp_mb() would be less correct from an >> abstract perspective > > ? That is entirely the purpose and intended meaning of the abstraction. > > smp_mb() orders operations such that (visible to other CPUs in the > system), all writes will have completed before any subsequent reads begin.
Yes, but this code is not about multiple CPUs, but just the local one (we want to make sure CLFLUSH does what we want). Hence using smp_ prefixed barriers would be wrong. But anyway, with this becoming explicit mfence() invocations, the discussion is all moot now. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel