[PATCH 1/2] x86/intel: insert Ice Lake X (server) model numbers

[PATCH 1/2] x86/intel: insert Ice Lake X (server) model numbers

[Igor Druzhinin]

LBR, C-state MSRs and if_pschange_mc erratum applicability should correspond
to Ice Lake desktop according to External Design Specification vol.2.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
 xen/arch/x86/acpi/cpu_idle.c | 1 +
 xen/arch/x86/hvm/vmx/vmx.c   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
index 27e0b52..7ad726a 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -183,6 +183,7 @@ static void do_get_hw_residencies(void *arg)
     /* Ice Lake */
     case 0x7D:
     case 0x7E:
+    case 0x6A:
     /* Kaby Lake */
     case 0x8E:
     case 0x9E:
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 86b8916..bce8b99 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2429,6 +2429,7 @@ static bool __init has_if_pschange_mc(void)
     case 0x55: /* Skylake-X / Cascade Lake */
     case 0x7d: /* Ice Lake */
     case 0x7e: /* Ice Lake */
+    case 0x6a: /* Ice Lake-X */
     case 0x8e: /* Kaby / Coffee / Whiskey Lake M */
     case 0x9e: /* Kaby / Coffee / Whiskey Lake D */
     case 0xa5: /* Comet Lake H/S */
@@ -2775,7 +2776,7 @@ static const struct lbr_info *last_branch_msr_get(void)
         /* Goldmont Plus */
         case 0x7a:
         /* Ice Lake */
-        case 0x7d: case 0x7e:
+        case 0x7d: case 0x7e: case 0x6a:
         /* Tremont */
         case 0x86:
         /* Kaby Lake */
-- 
2.7.4

[PATCH 2/2] x86/mwait-idle: Customize IceLake server support

[Igor Druzhinin]

From: Chen Yu <yu.c.chen@intel.com>

On ICX platform, the C1E auto-promotion is enabled by default.
As a result, the CPU might fall into C1E more offen than previous
platforms. So disable C1E auto-promotion and expose C1E as a separate
idle state.

Beside C1 and C1E, the exit latency of C6 was measured
by a dedicated tool. However the exit latency(41us) exposed
by _CST is much smaller than the one we measured(128us). This
is probably due to the _CST uses the exit latency when woken
up from PC0+C6, rather than PC6+C6 when C6 was measured. Choose
the latter as we need the longest latency in theory.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[Linux commit a472ad2bcea479ba068880125d7273fc95c14b70]
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
Applying this gives almost 100% boost in sysbench cpu test on Whitley SDP
---
 xen/arch/x86/cpu/mwait-idle.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/xen/arch/x86/cpu/mwait-idle.c b/xen/arch/x86/cpu/mwait-idle.c
index 8add13d..f0c6ff9 100644
--- a/xen/arch/x86/cpu/mwait-idle.c
+++ b/xen/arch/x86/cpu/mwait-idle.c
@@ -554,6 +554,28 @@ static const struct cpuidle_state skx_cstates[] = {
 	{}
 };
 
+static const struct cpuidle_state icx_cstates[] = {
+       {
+               .name = "C1-ICX",
+               .flags = MWAIT2flg(0x00),
+               .exit_latency = 1,
+               .target_residency = 1,
+       },
+       {
+               .name = "C1E-ICX",
+               .flags = MWAIT2flg(0x01),
+               .exit_latency = 4,
+               .target_residency = 4,
+       },
+       {
+               .name = "C6-ICX",
+               .flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED,
+               .exit_latency = 128,
+               .target_residency = 384,
+       },
+       {}
+};
+
 static const struct cpuidle_state atom_cstates[] = {
 	{
 		.name = "C1E-ATM",
@@ -904,6 +926,11 @@ static const struct idle_cpu idle_cpu_skx = {
 	.disable_promotion_to_c1e = 1,
 };
 
+static const struct idle_cpu idle_cpu_icx = {
+       .state_table = icx_cstates,
+       .disable_promotion_to_c1e = 1,
+};
+
 static const struct idle_cpu idle_cpu_avn = {
 	.state_table = avn_cstates,
 	.disable_promotion_to_c1e = 1,
@@ -958,6 +985,7 @@ static const struct x86_cpu_id intel_idle_ids[] __initconstrel = {
 	ICPU(0x8e, skl),
 	ICPU(0x9e, skl),
 	ICPU(0x55, skx),
+	ICPU(0x6a, icx),
 	ICPU(0x57, knl),
 	ICPU(0x85, knl),
 	ICPU(0x5c, bxt),
-- 
2.7.4

Re: [PATCH 2/2] x86/mwait-idle: Customize IceLake server support

[Jan Beulich]

On 13.10.2020 05:02, Igor Druzhinin wrote:
> From: Chen Yu <yu.c.chen@intel.com>
> 
> On ICX platform, the C1E auto-promotion is enabled by default.
> As a result, the CPU might fall into C1E more offen than previous
> platforms. So disable C1E auto-promotion and expose C1E as a separate
> idle state.
> 
> Beside C1 and C1E, the exit latency of C6 was measured
> by a dedicated tool. However the exit latency(41us) exposed
> by _CST is much smaller than the one we measured(128us). This
> is probably due to the _CST uses the exit latency when woken
> up from PC0+C6, rather than PC6+C6 when C6 was measured. Choose
> the latter as we need the longest latency in theory.
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> [Linux commit a472ad2bcea479ba068880125d7273fc95c14b70]
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 1/2] x86/intel: insert Ice Lake X (server) model numbers

[Jan Beulich]

On 13.10.2020 05:02, Igor Druzhinin wrote:
> LBR, C-state MSRs and if_pschange_mc erratum applicability should correspond
> to Ice Lake desktop according to External Design Specification vol.2.

Could you tell me where this is publicly available? Even after spending
quite a bit of time on searching for it, I can't seem to be able to
find it. And the SDM doesn't have enough information (yet).

> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -183,6 +183,7 @@ static void do_get_hw_residencies(void *arg)
>      /* Ice Lake */
>      case 0x7D:
>      case 0x7E:
> +    case 0x6A:
>      /* Kaby Lake */
>      case 0x8E:
>      case 0x9E:

Here and below please honor the (partial) sorting that's in effect.

Jan

Re: [PATCH 1/2] x86/intel: insert Ice Lake X (server) model numbers

[Igor Druzhinin]

On 14/10/2020 16:47, Jan Beulich wrote:
> On 13.10.2020 05:02, Igor Druzhinin wrote:
>> LBR, C-state MSRs and if_pschange_mc erratum applicability should correspond
>> to Ice Lake desktop according to External Design Specification vol.2.
> 
> Could you tell me where this is publicly available? Even after spending
> quite a bit of time on searching for it, I can't seem to be able to
> find it. And the SDM doesn't have enough information (yet).

True that SDM doesn't have this data. As I mentioned that data is taken from
External Design Specification for Ice Lake server which is accessed using Intel
account. I'm not completely sure it is right to make changes in open source
project like Linux or Xen based on information which is not publicly available
yet. But Intel is frequently doing this with Linux : even my second patch uses
data taken from one of these documents and was committed by Intel to Linux first.

Do we need the information publicly available to commit these changes as well?
If not, we can run with these changes in our patchqueue until it gets out properly.

Igor

Re: [PATCH 1/2] x86/intel: insert Ice Lake X (server) model numbers

[Jan Beulich]

On 14.10.2020 18:42, Igor Druzhinin wrote:
> On 14/10/2020 16:47, Jan Beulich wrote:
>> On 13.10.2020 05:02, Igor Druzhinin wrote:
>>> LBR, C-state MSRs and if_pschange_mc erratum applicability should correspond
>>> to Ice Lake desktop according to External Design Specification vol.2.
>>
>> Could you tell me where this is publicly available? Even after spending
>> quite a bit of time on searching for it, I can't seem to be able to
>> find it. And the SDM doesn't have enough information (yet).
> 
> True that SDM doesn't have this data. As I mentioned that data is taken from
> External Design Specification for Ice Lake server which is accessed using Intel
> account. I'm not completely sure it is right to make changes in open source
> project like Linux or Xen based on information which is not publicly available
> yet. But Intel is frequently doing this with Linux : even my second patch uses
> data taken from one of these documents and was committed by Intel to Linux first.
> 
> Do we need the information publicly available to commit these changes as well?

Not necessarily, but it means this patch needs to be acked by someone
having access to the doc, which hence isn't me. Given the last SDM
update was in May, I'm expecting a refresh any day now. Iirc updates
where frequently done on a roughly quarterly basis.

> If not, we can run with these changes in our patchqueue until it gets out properly.

Well, I'm all for having such changes upstream as early as possible.

Jan