[PATCH v3 0/2] x86, mwaitt: introduce AMD mwaitt support

[PATCH v3 0/2] x86, mwaitt: introduce AMD mwaitt support

[Huang Rui]

Hi,

This patch set introduces a new instruction support on AMD Carrizo (Family
15h, Model 60h-6fh). It adds mwaitx delay function with a configurable
timer.

Andy and Boris provide a suggestion which use mwaitx on delay method.

Some discussions of the background, please see:
http://marc.info/?l=linux-kernel&m=143202042530498&w=2
http://marc.info/?l=linux-kernel&m=143161327003541&w=2
http://marc.info/?l=linux-kernel&m=143222815331016&w=2

They are rebased on tip/master.

Changes from v1 -> v2
- Remove mwaitx idle implementation since some disputes without power
  improvement.
- Add a patch which implement another use case on delay.
- Introduce a kernel parameter (delay) to make delay method configurable.

Changes from v2 -> v3
- Add compared data on commit message
- Remove kernel parameter
- Add hint to avoid to access deep state in future
- Update mwaitx delay method as Petter's suggestion

I already do some testing with mwaitx on udelay.

Test scenario:

glb_loops = usec_to_tsc(delay_usec)
rdtsc -> read TSC counters as start
mwaitx_delay(glb_loops)
rdtsc -> read TSC counters as end
diff = end - start

Compared the real TSC counts (diff), that means the loops of counter goes.
And glb_loops is the input value of the EBX, that is the expect loops which
user configures. Below is 10000 us of mwaitx delay, we could see about 1200
(diff - glb_loops) delayed with mwaitx.

[ 2369.008651] start=4401974718758, end=4401992576888, diff=17858130, glb_loops=17856939

Thanks,
Rui

Huang Rui (2):
  x86, mwaitt: add monitorx and mwaitx instruction
  x86, mwaitt: introduce mwaix delay with a configurable timer

 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/mwait.h      | 27 ++++++++++++++++++++++++++
 arch/x86/lib/delay.c              | 41 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 68 insertions(+), 1 deletion(-)

-- 
1.9.1

[PATCH v3 1/2] x86, mwaitt: add monitorx and mwaitx instruction

[Huang Rui]

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

                MWAIT                           MWAITX
opcode          0f 01 c9           |            0f 01 fb
ECX[0]                  value of RFLAGS.IF seen by instruction
ECX[1]          unused/#GP if set  |            enable timer if set
ECX[31:2]                     unused/#GP if set
EAX                           unused (reserve for hint)
EBX[31:0]       unused             |            max wait time (loops)

                MONITOR                         MONITORX
opcode          0f 01 c8           |            0f 01 fa
EAX                     (logical) address to monitor
ECX                     #GP if not zero

The software P0 frequency is the same as the TSC frequency.

Max timeout = EBX/(TSC frequency)

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/mwait.h      | 16 ++++++++++++++++
 2 files changed, 17 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..ece8048 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;"
+		     :: "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();
-- 
1.9.1

Re: [PATCH v3 1/2] x86, mwaitt: add monitorx and mwaitx instruction

[Borislav Petkov]

On Thu, Jun 11, 2015 at 10:14:10PM +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
> 
>                 MWAIT                           MWAITX
> opcode          0f 01 c9           |            0f 01 fb
> ECX[0]                  value of RFLAGS.IF seen by instruction
> ECX[1]          unused/#GP if set  |            enable timer if set
> ECX[31:2]                     unused/#GP if set
> EAX                           unused (reserve for hint)
> EBX[31:0]       unused             |            max wait time (loops)
> 
>                 MONITOR                         MONITORX
> opcode          0f 01 c8           |            0f 01 fa
> EAX                     (logical) address to monitor
> ECX                     #GP if not zero
> 
> The software P0 frequency is the same as the TSC frequency.
> 
> Max timeout = EBX/(TSC frequency)

As suggested last time, please put the description of MWAITX over
__mwaitx() in the code as comments below. See below.

> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  arch/x86/include/asm/cpufeature.h |  1 +
>  arch/x86/include/asm/mwait.h      | 16 ++++++++++++++++
>  2 files changed, 17 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..ece8048 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;"
> +		     :: "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));
>  }


<Add comment here!>

> +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();
> -- 
> 1.9.1
> 

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v3 1/2] x86, mwaitt: add monitorx and mwaitx instruction

[Huang Rui]

On Thu, Jun 11, 2015 at 04:19:04PM +0200, Borislav Petkov wrote:
> On Thu, Jun 11, 2015 at 10:14:10PM +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
> > 
> >                 MWAIT                           MWAITX
> > opcode          0f 01 c9           |            0f 01 fb
> > ECX[0]                  value of RFLAGS.IF seen by instruction
> > ECX[1]          unused/#GP if set  |            enable timer if set
> > ECX[31:2]                     unused/#GP if set
> > EAX                           unused (reserve for hint)
> > EBX[31:0]       unused             |            max wait time (loops)
> > 
> >                 MONITOR                         MONITORX
> > opcode          0f 01 c8           |            0f 01 fa
> > EAX                     (logical) address to monitor
> > ECX                     #GP if not zero
> > 
> > The software P0 frequency is the same as the TSC frequency.
> > 
> > Max timeout = EBX/(TSC frequency)
> 
> As suggested last time, please put the description of MWAITX over
> __mwaitx() in the code as comments below. See below.
> 

Apology to miss that. Will do it at once.

Thanks,
Rui

[PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Huang Rui]

MWAITX can enable a timer and a corresponding timer value specified in SW
P0 clocks. The SW P0 frequency is the same with TSC. The timer provides an
upper bound on how long the instruction waits before exiting.

The implementation of delay function in kernel can lerverage the timer of
MWAITX. This patch provides a new method (delay_mwaitx) to measure delay
time.

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Suggested-by: Borislav Petkov <bp@suse.de>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 arch/x86/include/asm/mwait.h | 11 +++++++++++
 arch/x86/lib/delay.c         | 41 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
index ece8048..9b41580 100644
--- a/arch/x86/include/asm/mwait.h
+++ b/arch/x86/include/asm/mwait.h
@@ -14,6 +14,8 @@
 #define CPUID5_ECX_INTERRUPT_BREAK	0x2
 
 #define MWAIT_ECX_INTERRUPT_BREAK	0x1
+#define MWAITX_ECX_TIMER_ENABLE		BIT(1)
+#define MWAITX_MAX_LOOPS		((u32)-1)
 
 static inline void __monitor(const void *eax, unsigned long ecx,
 			     unsigned long edx)
@@ -80,4 +82,13 @@ static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx)
 	current_clr_polling();
 }
 
+static inline void mwaitx(unsigned long eax, unsigned long loops,
+			  bool enable)
+{
+	if (enable)
+		__mwaitx(eax, loops, MWAITX_ECX_TIMER_ENABLE);
+	else
+		__mwaitx(eax, 0, 0);
+}
+
 #endif /* _ASM_X86_MWAIT_H */
diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c
index 39d6a3d..9daf94a 100644
--- a/arch/x86/lib/delay.c
+++ b/arch/x86/lib/delay.c
@@ -20,6 +20,7 @@
 #include <asm/processor.h>
 #include <asm/delay.h>
 #include <asm/timer.h>
+#include <asm/mwait.h>
 
 #ifdef CONFIG_SMP
 # include <asm/smp.h>
@@ -87,6 +88,41 @@ static void delay_tsc(unsigned long __loops)
 }
 
 /*
+ * On AMD platforms mwaitx has a configurable 32-bit timer, that counts
+ * with TSC frequency. And the input value is the loop of the counter, it
+ * will exit with the timer expired.
+ */
+static void delay_mwaitx(unsigned long __loops)
+{
+	u32 end, start, delay, addr, loops = __loops;
+
+	rdtsc_barrier();
+	rdtscl(start);
+
+	for (;;) {
+		delay = min(MWAITX_MAX_LOOPS, loops);
+
+		__monitorx(&addr, 0, 0);
+		/*
+		 * AMD, like Intel, supports the EAX hint and EAX=0xf
+		 * means, do not enter any deep C-state and we use it
+		 * here in delay() to minimize wakeup latency.
+		 */
+		mwaitx(0xf, delay, true);
+
+		rdtsc_barrier();
+		rdtscl(end);
+
+		if (loops <= end - start)
+			break;
+
+		loops -= end - start;
+
+		start = end;
+	}
+}
+
+/*
  * Since we calibrate only once at boot, this
  * function should be set once at boot and not changed
  */
@@ -108,7 +144,10 @@ int read_current_timer(unsigned long *timer_val)
 
 void __delay(unsigned long loops)
 {
-	delay_fn(loops);
+	if (!static_cpu_has_safe(X86_FEATURE_MWAITT))
+		delay_fn(loops);
+	else
+		delay_mwaitx(loops);
 }
 EXPORT_SYMBOL(__delay);
 
-- 
1.9.1

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Andy Lutomirski]

On Thu, Jun 11, 2015 at 7:14 AM, Huang Rui <ray.huang@amd.com> wrote:
> MWAITX can enable a timer and a corresponding timer value specified in SW
> P0 clocks. The SW P0 frequency is the same with TSC. The timer provides an
> upper bound on how long the instruction waits before exiting.
>
> The implementation of delay function in kernel can lerverage the timer of
> MWAITX. This patch provides a new method (delay_mwaitx) to measure delay
> time.
>
> Suggested-by: Andy Lutomirski <luto@amacapital.net>
> Suggested-by: Borislav Petkov <bp@suse.de>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  arch/x86/include/asm/mwait.h | 11 +++++++++++
>  arch/x86/lib/delay.c         | 41 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
> index ece8048..9b41580 100644
> --- a/arch/x86/include/asm/mwait.h
> +++ b/arch/x86/include/asm/mwait.h
> @@ -14,6 +14,8 @@
>  #define CPUID5_ECX_INTERRUPT_BREAK     0x2
>
>  #define MWAIT_ECX_INTERRUPT_BREAK      0x1
> +#define MWAITX_ECX_TIMER_ENABLE                BIT(1)
> +#define MWAITX_MAX_LOOPS               ((u32)-1)
>
>  static inline void __monitor(const void *eax, unsigned long ecx,
>                              unsigned long edx)
> @@ -80,4 +82,13 @@ static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx)
>         current_clr_polling();
>  }
>
> +static inline void mwaitx(unsigned long eax, unsigned long loops,
> +                         bool enable)
> +{
> +       if (enable)
> +               __mwaitx(eax, loops, MWAITX_ECX_TIMER_ENABLE);
> +       else
> +               __mwaitx(eax, 0, 0);
> +}

What's the purpose of the "enable" parameter?

--Andy

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Huang Rui]

On Thu, Jun 11, 2015 at 10:34:15AM -0700, Andy Lutomirski wrote:
> On Thu, Jun 11, 2015 at 7:14 AM, Huang Rui <ray.huang@amd.com> wrote:
> > MWAITX can enable a timer and a corresponding timer value specified in SW
> > P0 clocks. The SW P0 frequency is the same with TSC. The timer provides an
> > upper bound on how long the instruction waits before exiting.
> >
> > The implementation of delay function in kernel can lerverage the timer of
> > MWAITX. This patch provides a new method (delay_mwaitx) to measure delay
> > time.
> >
> > Suggested-by: Andy Lutomirski <luto@amacapital.net>
> > Suggested-by: Borislav Petkov <bp@suse.de>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >  arch/x86/include/asm/mwait.h | 11 +++++++++++
> >  arch/x86/lib/delay.c         | 41 ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/mwait.h b/arch/x86/include/asm/mwait.h
> > index ece8048..9b41580 100644
> > --- a/arch/x86/include/asm/mwait.h
> > +++ b/arch/x86/include/asm/mwait.h
> > @@ -14,6 +14,8 @@
> >  #define CPUID5_ECX_INTERRUPT_BREAK     0x2
> >
> >  #define MWAIT_ECX_INTERRUPT_BREAK      0x1
> > +#define MWAITX_ECX_TIMER_ENABLE                BIT(1)
> > +#define MWAITX_MAX_LOOPS               ((u32)-1)
> >
> >  static inline void __monitor(const void *eax, unsigned long ecx,
> >                              unsigned long edx)
> > @@ -80,4 +82,13 @@ static inline void mwait_idle_with_hints(unsigned long eax, unsigned long ecx)
> >         current_clr_polling();
> >  }
> >
> > +static inline void mwaitx(unsigned long eax, unsigned long loops,
> > +                         bool enable)
> > +{
> > +       if (enable)
> > +               __mwaitx(eax, loops, MWAITX_ECX_TIMER_ENABLE);
> > +       else
> > +               __mwaitx(eax, 0, 0);
> > +}
> 
> What's the purpose of the "enable" parameter?
> 

Enable mwaitx timer. Should I add comments to explain the usage?

Thanks,
Rui

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Peter Zijlstra]

On Fri, Jun 12, 2015 at 12:03:22PM +0800, Huang Rui wrote:

> > What's the purpose of the "enable" parameter?
> > 
> 
> Enable mwaitx timer. Should I add comments to explain the usage?

No, you should read your own patch, notice that the only usage site
provides a true, then realize its a redundant bit of code and kill it
dead.

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Huang Rui]

On Fri, Jun 12, 2015 at 09:38:48AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2015 at 12:03:22PM +0800, Huang Rui wrote:
> 
> > > What's the purpose of the "enable" parameter?
> > > 
> > 
> > Enable mwaitx timer. Should I add comments to explain the usage?
> 
> No, you should read your own patch, notice that the only usage site
> provides a true, then realize its a redundant bit of code and kill it
> dead.

Yeah, but I remembered at last time, someone tell me we shouldn't
force to set timer always enabled. So I add this interface to expose
more clearly. :)

So could I use __mwaitx directly like below:

__monitorx(&dummy, 0, 0);
__mwaitx(0xf, delay, MWAITX_ECX_TIMER_ENABLE);


Thanks,
Rui

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Borislav Petkov]

On Fri, Jun 12, 2015 at 03:50:33PM +0800, Huang Rui wrote:
> Yeah, but I remembered at last time, someone tell me we shouldn't
> force to set timer always enabled. So I add this interface to expose
> more clearly. :)

Yeah, but this interface would make users go and look at it and try to
remember what the enable parameter stood for.

> So could I use __mwaitx directly like below:
> 
> __monitorx(&dummy, 0, 0);
> __mwaitx(0xf, delay, MWAITX_ECX_TIMER_ENABLE);

Yes, this is more straight-forward and what the hardware instructions
take, so more intuitive.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v3 2/2] x86, mwaitt: introduce mwaix delay with a configurable timer

[Huang Rui]

On Fri, Jun 12, 2015 at 10:30:33AM +0200, Borislav Petkov wrote:
> On Fri, Jun 12, 2015 at 03:50:33PM +0800, Huang Rui wrote:
> > Yeah, but I remembered at last time, someone tell me we shouldn't
> > force to set timer always enabled. So I add this interface to expose
> > more clearly. :)
> 
> Yeah, but this interface would make users go and look at it and try to
> remember what the enable parameter stood for.
> 
> > So could I use __mwaitx directly like below:
> > 
> > __monitorx(&dummy, 0, 0);
> > __mwaitx(0xf, delay, MWAITX_ECX_TIMER_ENABLE);
> 
> Yes, this is more straight-forward and what the hardware instructions
> take, so more intuitive.
> 

OK, will do it. :)

Thanks,
Rui