[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Christian Gmeiner]

Fixes performance related issue when running vxWorks 5/7 images.
The overall memory performance (L1, L2 cache and ram) was measured
with Bandwidth [0].

Without this patch we get following numbers:
 - sequential 128-bit reads:  ~5.2 GB/s
 - sequential 128-bit copy:   ~2.1 GB/s
 - random 32-bit writes:      ~1.2 GB/s

With this patch patch we get the following numbers:
 - sequential 128-bit reads: ~18.0 GB/s
 - sequential 128-bit copy:   ~9.5 GB/s
 - random 32-bit writes:      ~5.0 GB/s

[0] https://zsmith.co/bandwidth.html

v1 -> v2:
 - incorporate feedback from Bin Meng

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 arch/x86/cpu/queensbay/Makefile |  2 +-
 arch/x86/cpu/queensbay/cpu.c    | 58 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/cpu/queensbay/cpu.c

diff --git a/arch/x86/cpu/queensbay/Makefile b/arch/x86/cpu/queensbay/Makefile
index c0681995bd..3dd23465d4 100644
--- a/arch/x86/cpu/queensbay/Makefile
+++ b/arch/x86/cpu/queensbay/Makefile
@@ -5,4 +5,4 @@
 #
 
 obj-y += fsp_configs.o irq.o
-obj-y += tnc.o
+obj-y += tnc.o cpu.o
diff --git a/arch/x86/cpu/queensbay/cpu.c b/arch/x86/cpu/queensbay/cpu.c
new file mode 100644
index 0000000000..805a94cc27
--- /dev/null
+++ b/arch/x86/cpu/queensbay/cpu.c
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2018, Bachmann electronic GmbH
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <cpu.h>
+#include <dm.h>
+#include <asm/cpu.h>
+#include <asm/cpu_x86.h>
+#include <asm/msr.h>
+
+static void set_max_freq(void)
+{
+	msr_t msr;
+
+	/* Enable enhanced speed step */
+	msr = msr_read(MSR_IA32_MISC_ENABLES);
+	msr.lo |= (1 << 16);
+	msr_write(MSR_IA32_MISC_ENABLES, msr);
+
+	/* Set new performance state */
+	msr = msr_read(MSR_IA32_PERF_CTL);
+	msr.lo = 0x101f;
+	msr_write(MSR_IA32_PERF_CTL, msr);
+}
+
+static int cpu_x86_tunnelcreek_probe(struct udevice *dev)
+{
+	if (!ll_boot_init())
+		return 0;
+	debug("Init TunnelCreek core\n");
+
+	/* Set core to max frequency ratio */
+	set_max_freq();
+
+	return 0;
+}
+
+static const struct cpu_ops cpu_x86_tunnelcreek_ops = {
+	.get_desc	= cpu_x86_get_desc,
+	.get_count	= cpu_x86_get_count,
+};
+
+static const struct udevice_id cpu_x86_tunnelcreek_ids[] = {
+	{ .compatible = "intel,tunnelcreek-cpu" },
+	{ }
+};
+
+U_BOOT_DRIVER(cpu_x86_tunnelcreek_drv) = {
+	.name		= "cpu_x86_tunnelcreek",
+	.id		= UCLASS_CPU,
+	.of_match	= cpu_x86_tunnelcreek_ids,
+	.bind		= cpu_x86_bind,
+	.probe		= cpu_x86_tunnelcreek_probe,
+	.ops		= &cpu_x86_tunnelcreek_ops,
+};
-- 
2.14.3

[U-Boot] [PATCH v2 2/2] x86: Intel Crown Bay board: switch to TunnelCreek cpu driver

[Christian Gmeiner]

Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---
 arch/x86/dts/crownbay.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/dts/crownbay.dts b/arch/x86/dts/crownbay.dts
index 78a1ef415c..bb39a895f6 100644
--- a/arch/x86/dts/crownbay.dts
+++ b/arch/x86/dts/crownbay.dts
@@ -32,14 +32,14 @@
 
 		cpu at 0 {
 			device_type = "cpu";
-			compatible = "cpu-x86";
+			compatible = "intel,tunnelcreek-cpu";
 			reg = <0>;
 			intel,apic-id = <0>;
 		};
 
 		cpu at 1 {
 			device_type = "cpu";
-			compatible = "cpu-x86";
+			compatible = "intel,tunnelcreek-cpu";
 			reg = <1>;
 			intel,apic-id = <1>;
 		};
-- 
2.14.3

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Hi Christian,

On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
> Fixes performance related issue when running vxWorks 5/7 images.

nits: vxWorks -> VxWorks

> The overall memory performance (L1, L2 cache and ram) was measured
> with Bandwidth [0].
>
> Without this patch we get following numbers:
>  - sequential 128-bit reads:  ~5.2 GB/s
>  - sequential 128-bit copy:   ~2.1 GB/s
>  - random 32-bit writes:      ~1.2 GB/s
>
> With this patch patch we get the following numbers:
>  - sequential 128-bit reads: ~18.0 GB/s
>  - sequential 128-bit copy:   ~9.5 GB/s
>  - random 32-bit writes:      ~5.0 GB/s
>
> [0] https://zsmith.co/bandwidth.html
>
> v1 -> v2:
>  - incorporate feedback from Bin Meng

This should not show in the commit message.

>
> Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>  arch/x86/cpu/queensbay/Makefile |  2 +-
>  arch/x86/cpu/queensbay/cpu.c    | 58 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+), 1 deletion(-)
>  create mode 100644 arch/x86/cpu/queensbay/cpu.c
>
> diff --git a/arch/x86/cpu/queensbay/Makefile b/arch/x86/cpu/queensbay/Makefile
> index c0681995bd..3dd23465d4 100644
> --- a/arch/x86/cpu/queensbay/Makefile
> +++ b/arch/x86/cpu/queensbay/Makefile
> @@ -5,4 +5,4 @@
>  #
>
>  obj-y += fsp_configs.o irq.o
> -obj-y += tnc.o
> +obj-y += tnc.o cpu.o
> diff --git a/arch/x86/cpu/queensbay/cpu.c b/arch/x86/cpu/queensbay/cpu.c
> new file mode 100644
> index 0000000000..805a94cc27
> --- /dev/null
> +++ b/arch/x86/cpu/queensbay/cpu.c
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright (C) 2018, Bachmann electronic GmbH
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <cpu.h>
> +#include <dm.h>
> +#include <asm/cpu.h>
> +#include <asm/cpu_x86.h>
> +#include <asm/msr.h>
> +
> +static void set_max_freq(void)
> +{
> +       msr_t msr;
> +
> +       /* Enable enhanced speed step */
> +       msr = msr_read(MSR_IA32_MISC_ENABLES);
> +       msr.lo |= (1 << 16);
> +       msr_write(MSR_IA32_MISC_ENABLES, msr);
> +
> +       /* Set new performance state */
> +       msr = msr_read(MSR_IA32_PERF_CTL);
> +       msr.lo = 0x101f;
> +       msr_write(MSR_IA32_PERF_CTL, msr);
> +}

I tried to find any documentation that describes the performance state
values of the TunnelCreek processor, but in vain. However when I read
the doc, I do have a question here:

The enhanced speedstep technology is set to disabled by the processor
after power-on, that means we don't need set the performance state
(P-state) via the MSR_IA32_PERF_CTL and the processor itself should
work under its maximum base frequency. So I believe this whole
set_max_freq() is not needed. Can you clarify this?

> +
> +static int cpu_x86_tunnelcreek_probe(struct udevice *dev)
> +{
> +       if (!ll_boot_init())
> +               return 0;
> +       debug("Init TunnelCreek core\n");
> +
> +       /* Set core to max frequency ratio */
> +       set_max_freq();
> +
> +       return 0;
> +}
> +
> +static const struct cpu_ops cpu_x86_tunnelcreek_ops = {
> +       .get_desc       = cpu_x86_get_desc,
> +       .get_count      = cpu_x86_get_count,
> +};
> +
> +static const struct udevice_id cpu_x86_tunnelcreek_ids[] = {
> +       { .compatible = "intel,tunnelcreek-cpu" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(cpu_x86_tunnelcreek_drv) = {
> +       .name           = "cpu_x86_tunnelcreek",
> +       .id             = UCLASS_CPU,
> +       .of_match       = cpu_x86_tunnelcreek_ids,
> +       .bind           = cpu_x86_bind,
> +       .probe          = cpu_x86_tunnelcreek_probe,
> +       .ops            = &cpu_x86_tunnelcreek_ops,
> +};
> --

Regards,
Bin

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Christian, I got some time to look at this old patch, and I see my
last question remained unanswered.

+Andy,

Hi Andy,

On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Christian,
>
> On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
> > Fixes performance related issue when running vxWorks 5/7 images.
>
> nits: vxWorks -> VxWorks
>
> > The overall memory performance (L1, L2 cache and ram) was measured
> > with Bandwidth [0].
> >
> > Without this patch we get following numbers:
> >  - sequential 128-bit reads:  ~5.2 GB/s
> >  - sequential 128-bit copy:   ~2.1 GB/s
> >  - random 32-bit writes:      ~1.2 GB/s
> >
> > With this patch patch we get the following numbers:
> >  - sequential 128-bit reads: ~18.0 GB/s
> >  - sequential 128-bit copy:   ~9.5 GB/s
> >  - random 32-bit writes:      ~5.0 GB/s
> >
> > [0] https://zsmith.co/bandwidth.html
> >
> > v1 -> v2:
> >  - incorporate feedback from Bin Meng
>
> This should not show in the commit message.
>
> >
> > Signed-off-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> > ---
> >  arch/x86/cpu/queensbay/Makefile |  2 +-
> >  arch/x86/cpu/queensbay/cpu.c    | 58 +++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/x86/cpu/queensbay/cpu.c
> >
> > diff --git a/arch/x86/cpu/queensbay/Makefile b/arch/x86/cpu/queensbay/Makefile
> > index c0681995bd..3dd23465d4 100644
> > --- a/arch/x86/cpu/queensbay/Makefile
> > +++ b/arch/x86/cpu/queensbay/Makefile
> > @@ -5,4 +5,4 @@
> >  #
> >
> >  obj-y += fsp_configs.o irq.o
> > -obj-y += tnc.o
> > +obj-y += tnc.o cpu.o
> > diff --git a/arch/x86/cpu/queensbay/cpu.c b/arch/x86/cpu/queensbay/cpu.c
> > new file mode 100644
> > index 0000000000..805a94cc27
> > --- /dev/null
> > +++ b/arch/x86/cpu/queensbay/cpu.c
> > @@ -0,0 +1,58 @@
> > +/*
> > + * Copyright (C) 2018, Bachmann electronic GmbH
> > + *
> > + * SPDX-License-Identifier:    GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <cpu.h>
> > +#include <dm.h>
> > +#include <asm/cpu.h>
> > +#include <asm/cpu_x86.h>
> > +#include <asm/msr.h>
> > +
> > +static void set_max_freq(void)
> > +{
> > +       msr_t msr;
> > +
> > +       /* Enable enhanced speed step */
> > +       msr = msr_read(MSR_IA32_MISC_ENABLES);
> > +       msr.lo |= (1 << 16);
> > +       msr_write(MSR_IA32_MISC_ENABLES, msr);
> > +
> > +       /* Set new performance state */
> > +       msr = msr_read(MSR_IA32_PERF_CTL);
> > +       msr.lo = 0x101f;
> > +       msr_write(MSR_IA32_PERF_CTL, msr);
> > +}
>
> I tried to find any documentation that describes the performance state
> values of the TunnelCreek processor, but in vain. However when I read
> the doc, I do have a question here:
>
> The enhanced speedstep technology is set to disabled by the processor
> after power-on, that means we don't need set the performance state
> (P-state) via the MSR_IA32_PERF_CTL and the processor itself should
> work under its maximum base frequency. So I believe this whole
> set_max_freq() is not needed. Can you clarify this?
>

I hope you can give some clarification about Enhanced Intel Speedstep
(EIST) technology. I read Intel 64 and IA-32 Architectures SDM volume
3: chapter 14 "POWER AND THERMAL MANAGEMENT" and it does not say much
about this EIST. Especially I want to know whether the process is
running at the highest frequency if EIST is disabled which is the case
after power-on (MSR_IA32_MISC_ENABLES bit[16] is 0).

A related discussion after Googling is:
https://forums.anandtech.com/threads/actual-difference-between-speedstep-and-turbo-boost.2226820/
It says: "The base frequency is the highest P-state called P0, and
every step below that using EIST goes P1, P2, etc. Turbo Mode only
activates when the CPU is at P0 state, then every speed above the base
clock is decided by the CPU."

So to me this seems to match my understanding about EIST. If this is
true, then I can't explain why Christian's patch is needed since the
EIST is disabled on TunnelCreek by default and the processor should
already run at the highest performance.

Regards,
Bin

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Andy Shevchenko]

On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > <christian.gmeiner@gmail.com> wrote:

> So to me this seems to match my understanding about EIST. If this is
> true, then I can't explain why Christian's patch is needed since the
> EIST is disabled on TunnelCreek by default and the processor should
> already run at the highest performance.

The some internal documents I found suggesting that first what one needs to do
is to be sure that EIST is enabled / disabled by reading a bit from CPUID.

(There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)

It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
(MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
(MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
accordingly.

Hope this helps.

P.S. All names are implying Linux kernel source code.

-- 
With Best Regards,
Andy Shevchenko

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Hi Andy,

On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > <christian.gmeiner@gmail.com> wrote:
>
> > So to me this seems to match my understanding about EIST. If this is
> > true, then I can't explain why Christian's patch is needed since the
> > EIST is disabled on TunnelCreek by default and the processor should
> > already run at the highest performance.
>
> The some internal documents I found suggesting that first what one needs to do
> is to be sure that EIST is enabled / disabled by reading a bit from CPUID.
>

Correct. This is documented in the public Intel SDM too.

> (There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)
>
> It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
> (MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
> (MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
> accordingly.
>
> Hope this helps.
>

Thanks for the help. But I was looking for clarification on what
frequency the CPU is running at if EIST is disabled (the default
power-up state). Especially I suspect the Intel CPU is running at the
highest frequency when ESIT is disabled hence the highest performance.
Such details are not documented in the public Intel SDM. :(

> P.S. All names are implying Linux kernel source code.

Regards,
Bin

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Christian Gmeiner]

Hi all,

Am Mo., 11. März 2019 um 15:41 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
>
> Hi Andy,
>
> On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > > <christian.gmeiner@gmail.com> wrote:
> >
> > > So to me this seems to match my understanding about EIST. If this is
> > > true, then I can't explain why Christian's patch is needed since the
> > > EIST is disabled on TunnelCreek by default and the processor should
> > > already run at the highest performance.
> >
> > The some internal documents I found suggesting that first what one needs to do
> > is to be sure that EIST is enabled / disabled by reading a bit from CPUID.
> >
>
> Correct. This is documented in the public Intel SDM too.
>
> > (There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)
> >
> > It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
> > (MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
> > (MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
> > accordingly.
> >
> > Hope this helps.
> >
>
> Thanks for the help. But I was looking for clarification on what
> frequency the CPU is running at if EIST is disabled (the default
> power-up state). Especially I suspect the Intel CPU is running at the
> highest frequency when ESIT is disabled hence the highest performance.
> Such details are not documented in the public Intel SDM. :(
>
> > P.S. All names are implying Linux kernel source code.
>

I think I need to clarify how I came to this patch.

Back in the days when we used an Intel BLDK based BIOS solution we did
some performance
measurements and found out that the same device with an vendor BIOS is
much faster. So we started
to dig into the issue and used bandwidth to do our measurements. We
contacted Intel to get support
and after lot of ping-pong mails we got a solution.

We need to set these two msr registers to the values they provided to us.
These msr register should be described in #29324.

Bin do you have hardware to reproduce this issue I have and what gets
fixed with this patch?

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Hi Christian,

On Wed, Mar 13, 2019 at 5:27 PM Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
>
> Hi all,
>
> Am Mo., 11. März 2019 um 15:41 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> >
> > Hi Andy,
> >
> > On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > > > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > > > <christian.gmeiner@gmail.com> wrote:
> > >
> > > > So to me this seems to match my understanding about EIST. If this is
> > > > true, then I can't explain why Christian's patch is needed since the
> > > > EIST is disabled on TunnelCreek by default and the processor should
> > > > already run at the highest performance.
> > >
> > > The some internal documents I found suggesting that first what one needs to do
> > > is to be sure that EIST is enabled / disabled by reading a bit from CPUID.
> > >
> >
> > Correct. This is documented in the public Intel SDM too.
> >
> > > (There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)
> > >
> > > It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
> > > (MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
> > > (MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
> > > accordingly.
> > >
> > > Hope this helps.
> > >
> >
> > Thanks for the help. But I was looking for clarification on what
> > frequency the CPU is running at if EIST is disabled (the default
> > power-up state). Especially I suspect the Intel CPU is running at the
> > highest frequency when ESIT is disabled hence the highest performance.
> > Such details are not documented in the public Intel SDM. :(
> >
> > > P.S. All names are implying Linux kernel source code.
> >
>
> I think I need to clarify how I came to this patch.
>
> Back in the days when we used an Intel BLDK based BIOS solution we did
> some performance
> measurements and found out that the same device with an vendor BIOS is
> much faster. So we started
> to dig into the issue and used bandwidth to do our measurements. We
> contacted Intel to get support
> and after lot of ping-pong mails we got a solution.
>
> We need to set these two msr registers to the values they provided to us.
> These msr register should be described in #29324.

Could you please provide details about these 2 MSR registers? And is
there any official document from Intel about what is the behavior of
having EIST disabled?

>
> Bin do you have hardware to reproduce this issue I have and what gets
> fixed with this patch?

Yes, I have the hardware to test. Is the test case able to run in
U-Boot? I can have a test.

Regards,
Bin

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Christian Gmeiner]

Hi Bin,

Am Fr., 15. März 2019 um 09:03 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
>
> Hi Christian,
>
> On Wed, Mar 13, 2019 at 5:27 PM Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
> >
> > Hi all,
> >
> > Am Mo., 11. März 2019 um 15:41 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> > >
> > > Hi Andy,
> > >
> > > On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > > > > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > > > > <christian.gmeiner@gmail.com> wrote:
> > > >
> > > > > So to me this seems to match my understanding about EIST. If this is
> > > > > true, then I can't explain why Christian's patch is needed since the
> > > > > EIST is disabled on TunnelCreek by default and the processor should
> > > > > already run at the highest performance.
> > > >
> > > > The some internal documents I found suggesting that first what one needs to do
> > > > is to be sure that EIST is enabled / disabled by reading a bit from CPUID.
> > > >
> > >
> > > Correct. This is documented in the public Intel SDM too.
> > >
> > > > (There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)
> > > >
> > > > It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
> > > > (MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
> > > > (MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
> > > > accordingly.
> > > >
> > > > Hope this helps.
> > > >
> > >
> > > Thanks for the help. But I was looking for clarification on what
> > > frequency the CPU is running at if EIST is disabled (the default
> > > power-up state). Especially I suspect the Intel CPU is running at the
> > > highest frequency when ESIT is disabled hence the highest performance.
> > > Such details are not documented in the public Intel SDM. :(
> > >
> > > > P.S. All names are implying Linux kernel source code.
> > >
> >
> > I think I need to clarify how I came to this patch.
> >
> > Back in the days when we used an Intel BLDK based BIOS solution we did
> > some performance
> > measurements and found out that the same device with an vendor BIOS is
> > much faster. So we started
> > to dig into the issue and used bandwidth to do our measurements. We
> > contacted Intel to get support
> > and after lot of ping-pong mails we got a solution.
> >
> > We need to set these two msr registers to the values they provided to us.
> > These msr register should be described in #29324.
>
> Could you please provide details about these 2 MSR registers? And is
> there any official document from Intel about what is the behavior of
> having EIST disabled?
>

I only got an excerpt of two pages describing these 2 MSR and I am quite sure
I am not allowed to share these. As I wrote these MSR should be documented
in #29324 (which I do not have access to).

> >
> > Bin do you have hardware to reproduce this issue I have and what gets
> > fixed with this patch?
>
> Yes, I have the hardware to test. Is the test case able to run in
> U-Boot? I can have a test.

Boot into linux and run https://zsmith.co/bandwidth.html

-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Bin Meng]

Hi Christian,

On Mon, Apr 1, 2019 at 3:48 PM Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
>
> Hi Bin,
>
> Am Fr., 15. März 2019 um 09:03 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> >
> > Hi Christian,
> >
> > On Wed, Mar 13, 2019 at 5:27 PM Christian Gmeiner
> > <christian.gmeiner@gmail.com> wrote:
> > >
> > > Hi all,
> > >
> > > Am Mo., 11. März 2019 um 15:41 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> > > >
> > > > Hi Andy,
> > > >
> > > > On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > >
> > > > > On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > > > > > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > > > > > <christian.gmeiner@gmail.com> wrote:
> > > > >
> > > > > > So to me this seems to match my understanding about EIST. If this is
> > > > > > true, then I can't explain why Christian's patch is needed since the
> > > > > > EIST is disabled on TunnelCreek by default and the processor should
> > > > > > already run at the highest performance.
> > > > >
> > > > > The some internal documents I found suggesting that first what one needs to do
> > > > > is to be sure that EIST is enabled / disabled by reading a bit from CPUID.
> > > > >
> > > >
> > > > Correct. This is documented in the public Intel SDM too.
> > > >
> > > > > (There is no mention of the exact bit, I'm guessing it might be X86_FEATURE_EST)
> > > > >
> > > > > It also refers to IA32_MISC_ENABLE MSR, i.e. bit 20
> > > > > (MSR_IA32_MISC_ENABLE_SPEEDSTEP_LOCK_BIT) and bit 16
> > > > > (MSR_IA32_MISC_ENABLE_ENHANCED_SPEEDSTEP_BIT), that firmware can set up
> > > > > accordingly.
> > > > >
> > > > > Hope this helps.
> > > > >
> > > >
> > > > Thanks for the help. But I was looking for clarification on what
> > > > frequency the CPU is running at if EIST is disabled (the default
> > > > power-up state). Especially I suspect the Intel CPU is running at the
> > > > highest frequency when ESIT is disabled hence the highest performance.
> > > > Such details are not documented in the public Intel SDM. :(
> > > >
> > > > > P.S. All names are implying Linux kernel source code.
> > > >
> > >
> > > I think I need to clarify how I came to this patch.
> > >
> > > Back in the days when we used an Intel BLDK based BIOS solution we did
> > > some performance
> > > measurements and found out that the same device with an vendor BIOS is
> > > much faster. So we started
> > > to dig into the issue and used bandwidth to do our measurements. We
> > > contacted Intel to get support
> > > and after lot of ping-pong mails we got a solution.
> > >
> > > We need to set these two msr registers to the values they provided to us.
> > > These msr register should be described in #29324.
> >
> > Could you please provide details about these 2 MSR registers? And is
> > there any official document from Intel about what is the behavior of
> > having EIST disabled?
> >
>
> I only got an excerpt of two pages describing these 2 MSR and I am quite sure
> I am not allowed to share these. As I wrote these MSR should be documented
> in #29324 (which I do not have access to).
>

Thanks for sharing the information.

Andy, do you have access to the #29324, and check whether it has some
explanation about my assumption about Intel EIST?

> > >
> > > Bin do you have hardware to reproduce this issue I have and what gets
> > > fixed with this patch?
> >
> > Yes, I have the hardware to test. Is the test case able to run in
> > U-Boot? I can have a test.
>
> Boot into linux and run https://zsmith.co/bandwidth.html

Thanks. I will need run some testing when I get the board.

Regards,
Bin

[U-Boot] [PATCH v2 1/2] x86: TunnelCreek: switch P state to the highest freq

[Andy Shevchenko]

On Mon, Apr 01, 2019 at 04:08:33PM +0800, Bin Meng wrote:
> On Mon, Apr 1, 2019 at 3:48 PM Christian Gmeiner
> <christian.gmeiner@gmail.com> wrote:
> > Am Fr., 15. März 2019 um 09:03 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> > > On Wed, Mar 13, 2019 at 5:27 PM Christian Gmeiner
> > > <christian.gmeiner@gmail.com> wrote:
> > > > Am Mo., 11. März 2019 um 15:41 Uhr schrieb Bin Meng <bmeng.cn@gmail.com>:
> > > > > On Wed, Mar 6, 2019 at 7:09 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Thu, Feb 28, 2019 at 11:29:50AM +0800, Bin Meng wrote:
> > > > > > > On Thu, May 24, 2018 at 12:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > On Thu, Apr 12, 2018 at 4:07 PM, Christian Gmeiner
> > > > > > > > <christian.gmeiner@gmail.com> wrote:

> > > > We need to set these two msr registers to the values they provided to us.
> > > > These msr register should be described in #29324.
> > >
> > > Could you please provide details about these 2 MSR registers? And is
> > > there any official document from Intel about what is the behavior of
> > > having EIST disabled?
> > >
> >
> > I only got an excerpt of two pages describing these 2 MSR and I am quite sure
> > I am not allowed to share these. As I wrote these MSR should be documented
> > in #29324 (which I do not have access to).
> >
> 
> Thanks for sharing the information.
> 
> Andy, do you have access to the #29324, and check whether it has some
> explanation about my assumption about Intel EIST?

I have no idea what that number means.
Christian might sent me (*) at least a title of the document for easy search.

*) privately to @intel.com address, for example.

-- 
With Best Regards,
Andy Shevchenko