Re: [RFC PATCH 1/4] x86, mwaitt: add monitorx and mwaitx instruction

Huang Rui
2015-05-21T08:59:40Z

On Tue, May 19, 2015 at 07:29:45PM +0800, Borislav Petkov wrote:
> On Tue, May 19, 2015 at 04:01:09PM +0800, Huang Rui wrote:
> > On AMD Carrizo processors (Family 15h, Model 60h-6fh), there is a new
> > feature called MWAITT (Mwait with a timer) as an extension of
> > Monitor/Mwait.
> >
> > MWAITT, another name is MWAITX (MWAIT with extensions), has a configurable
> > timer that causes MWAITX to exit on expiration.
> >
> > Compared with MONITOR/MWAIT, there are minor differences in opcode and
> > input parameters.
> >
> > MWAITX ECX[1]: enable timer if set
> > MWAITX EBX[31:0]: max wait time expressed in SW P0 clocks
>
> What's the behavior if you set EBX to some value but don't enable the
> timer with ECX? Normal MWAIT?
>

Apology to late reply. I was having some AMD internal discussions and
collecting more data about this feature these two days.

EBX will be unused if disable timer. You're right, then the behavior
is like normal MWAIT. :)

> > The software P0 frequency is the same as the TSC frequency.
> >
> > Max timeout = EBX/(TSC frequency)
>
> That's max timeout in seconds then.
>

Actually, timeout = EBX * TSC cycle, EBX is the loop number that timer counts.
This is 32-bit counter, and the maximum value is 0xffffffff.

> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> > arch/x86/include/asm/cpufeature.h | 1 +
> > arch/x86/include/asm/mwait.h | 25 +++++++++++++++++++++++++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> > index 3d6606f..3ef1f6e 100644
> > --- a/arch/x86/include/asm/cpufeature.h
> > +++ b/arch/x86/include/asm/cpufeature.h
> > @@ -176,6 +176,7 @@
> > #define X86_FEATURE_PERFCTR_NB ( 6*32+24) /* NB performance counter extensions */
> > #define X86_FEATURE_BPEXT (6*32+26) /* data breakpoint extension */
> > #define X86_FEATURE_PERFCTR_L2 ( 6*32+28) /* L2 performance counter extensions */
> > +#define X86_FEATURE_MWAITT ( 6*32+29) /* Mwait extension (MonitorX/MwaitX) */
> >
> > /*
> > * Auxiliary flags: Linux defined - For features scattered in various
> > diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
> > index 653dfa7..b91136f 100644
> > --- a/arch/x86/include/asm/mwait.h
> > +++ b/arch/x86/include/asm/mwait.h
> > @@ -23,6 +23,14 @@ static inline void __monitor(const void *eax, unsigned long ecx,
> > :: "a" (eax), "c" (ecx), "d"(edx));
> > }
> >
> > +static inline void __monitorx(const void *eax, unsigned long ecx,
> > + unsigned long edx)
> > +{
> > + /* "monitorx %eax, %ecx, %edx;" */
> > + asm volatile(".byte 0x0f, 0x01, 0xfa;"
>
> Ah ok, ModRM extension to secondary opcode 0x1. Simply filling out the
> empty slots after SWAPGS, RDTSCP, ... :)
>

Sorry, I might not get your meaning. Should it update like below:

asm volatile(".byte 0x0f, 0x01, 0xfa;"
:: "a" (eax), "c" (ecx), "d" (edx));
^^^

> > + :: "a" (eax), "c" (ecx), "d"(edx));
> > +}
> > +
> > static inline void __mwait(unsigned long eax, unsigned long ecx)
> > {
> > /* "mwait %eax, %ecx;" */
> > @@ -30,6 +38,14 @@ static inline void __mwait(unsigned long eax, unsigned long ecx)
> > :: "a" (eax), "c" (ecx));
> > }
> >
> > +static inline void __mwaitx(unsigned long eax, unsigned long ebx,
> > + unsigned long ecx)
> > +{
> > + /* "mwaitx %eax, %ebx, %ecx;" */
> > + asm volatile(".byte 0x0f, 0x01, 0xfb;"
> > + :: "a" (eax), "b" (ebx), "c" (ecx));
> > +}
> > +
> > static inline void __sti_mwait(unsigned long eax, unsigned long ecx)
> > {
> > trace_hardirqs_on();
> > @@ -38,6 +54,15 @@ static inline void __sti_mwait(unsigned long eax, unsigned long ecx)
> > :: "a" (eax), "c" (ecx));
> > }
> >
> > +static inline void __sti_mwaitx(unsigned long eax, unsigned long ebx,
> > + unsigned long ecx)
>
> Please align the argument on the new line to the opening brace:
>
> static inline void __sti_mwaitx(unsigned long eax, unsigned long ebx,
> unsigned long ecx)

Got it, thanks.

Thanks,
Rui