[PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Igor Druzhinin]

LBR, C-state MSRs should correspond to Ice Lake desktop according to
External Design Specification vol.2 for both models.

Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
(confirmed on Whitley SDP) which means the erratum is fixed in hardware for
that model and therefore it shouldn't be present in has_if_pschange_mc list.
Provisionally assume the same to be the case for Ice Lake-D.

Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
---
Changes in v3:
- Add Ice Lake-D model numbers
- Drop has_if_pschange_mc hunk following Tiger Lake related discussion -
  IF_PSCHANGE_MC_NO is confimed to be exposed on Whitley SDP

---
 xen/arch/x86/acpi/cpu_idle.c | 2 ++
 xen/arch/x86/hvm/vmx/vmx.c   | 2 +-
 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 c092086..d788c8b 100644
--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -181,6 +181,8 @@ static void do_get_hw_residencies(void *arg)
     case 0x55:
     case 0x5E:
     /* Ice Lake */
+    case 0x6A:
+    case 0x6C:
     case 0x7D:
     case 0x7E:
     /* Tiger Lake */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 2d4475e..bff5979 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2775,7 +2775,7 @@ static const struct lbr_info *last_branch_msr_get(void)
         /* Goldmont Plus */
         case 0x7a:
         /* Ice Lake */
-        case 0x7d: case 0x7e:
+        case 0x6a: case 0x6c: case 0x7d: case 0x7e:
         /* Tiger Lake */
         case 0x8c: case 0x8d:
         /* Tremont */
-- 
2.7.4

Re: [PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Jan Beulich]

On 23.12.2020 21:32, Igor Druzhinin wrote:
> LBR, C-state MSRs should correspond to Ice Lake desktop according to
> External Design Specification vol.2 for both models.
> 
> Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
> (confirmed on Whitley SDP) which means the erratum is fixed in hardware for
> that model and therefore it shouldn't be present in has_if_pschange_mc list.
> Provisionally assume the same to be the case for Ice Lake-D.

I did find Ice Lake D EDS, and it confirms the respective additions.
In the course I also found the "plain" Ice Lake EDS, and it seems to
contradict SDM vol 4 in that it doesn't list CC3_RESIDENCY (0x3FC).
For now I guess we can consider this a doc error.

I didn't find Ice Lake-X EDS, though.

Jan

Re: [PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Igor Druzhinin]

On 06/01/2021 11:04, Jan Beulich wrote:
> On 23.12.2020 21:32, Igor Druzhinin wrote:
>> LBR, C-state MSRs should correspond to Ice Lake desktop according to
>> External Design Specification vol.2 for both models.
>>
>> Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
>> (confirmed on Whitley SDP) which means the erratum is fixed in hardware for
>> that model and therefore it shouldn't be present in has_if_pschange_mc list.
>> Provisionally assume the same to be the case for Ice Lake-D.
> 
> I did find Ice Lake D EDS, and it confirms the respective additions.
> In the course I also found the "plain" Ice Lake EDS, and it seems to
> contradict SDM vol 4 in that it doesn't list CC3_RESIDENCY (0x3FC).
> For now I guess we can consider this a doc error.
> 
> I didn't find Ice Lake-X EDS, though.

You may search "Ice Lake server eds volume 2".

Igor

Re: [PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Jan Beulich]

On 07.01.2021 03:17, Igor Druzhinin wrote:
> On 06/01/2021 11:04, Jan Beulich wrote:
>> On 23.12.2020 21:32, Igor Druzhinin wrote:
>>> LBR, C-state MSRs should correspond to Ice Lake desktop according to
>>> External Design Specification vol.2 for both models.
>>>
>>> Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
>>> (confirmed on Whitley SDP) which means the erratum is fixed in hardware for
>>> that model and therefore it shouldn't be present in has_if_pschange_mc list.
>>> Provisionally assume the same to be the case for Ice Lake-D.
>>
>> I did find Ice Lake D EDS, and it confirms the respective additions.
>> In the course I also found the "plain" Ice Lake EDS, and it seems to
>> contradict SDM vol 4 in that it doesn't list CC3_RESIDENCY (0x3FC).
>> For now I guess we can consider this a doc error.
>>
>> I didn't find Ice Lake-X EDS, though.
> 
> You may search "Ice Lake server eds volume 2".

Right, that's one of the many things I had tried. Yields only
Ice Lake-D EDSes ... I can only assume I'm not entitled to see
the server ones.

Jan

Re: [PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Andrew Cooper]

On 23/12/2020 20:32, Igor Druzhinin wrote:
> LBR, C-state MSRs should correspond to Ice Lake desktop according to
> External Design Specification vol.2 for both models.
>
> Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
> (confirmed on Whitley SDP) which means the erratum is fixed in hardware for
> that model and therefore it shouldn't be present in has_if_pschange_mc list.
> Provisionally assume the same to be the case for Ice Lake-D.
>
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> ---
> Changes in v3:
> - Add Ice Lake-D model numbers
> - Drop has_if_pschange_mc hunk following Tiger Lake related discussion -
>   IF_PSCHANGE_MC_NO is confimed to be exposed on Whitley SDP
>
> ---
>  xen/arch/x86/acpi/cpu_idle.c | 2 ++
>  xen/arch/x86/hvm/vmx/vmx.c   | 2 +-
>  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 c092086..d788c8b 100644
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -181,6 +181,8 @@ static void do_get_hw_residencies(void *arg)
>      case 0x55:
>      case 0x5E:
>      /* Ice Lake */
> +    case 0x6A:
> +    case 0x6C:
>      case 0x7D:
>      case 0x7E:
>      /* Tiger Lake */

So I think these changes are correct.  TGL definitely has deeper
core/package states than this interface enumerates, but I can't locate
extra MSRs.

> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2d4475e..bff5979 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2775,7 +2775,7 @@ static const struct lbr_info *last_branch_msr_get(void)
>          /* Goldmont Plus */
>          case 0x7a:
>          /* Ice Lake */
> -        case 0x7d: case 0x7e:
> +        case 0x6a: case 0x6c: case 0x7d: case 0x7e:

IceLake Server has what appear to be new aspects to LBR.  I can't find
LAST_INT_INFO (0x1e0) existing in any previous generation.

However, my investigation also found LBR_SELECT which has been around
since Nehalem, which we don't handle, and Linux *does* use.

This logic is in a terrible state.  It's no surprise it is always the
first thing to break in the field.

~Andrew

Re: [PATCH v3] x86/intel: insert Ice Lake-X (server) and Ice Lake-D model numbers

[Igor Druzhinin]

On 27/01/2021 09:52, Andrew Cooper wrote:
> On 23/12/2020 20:32, Igor Druzhinin wrote:
>> LBR, C-state MSRs should correspond to Ice Lake desktop according to
>> External Design Specification vol.2 for both models.
>>
>> Ice Lake-X is known to expose IF_PSCHANGE_MC_NO in IA32_ARCH_CAPABILITIES MSR
>> (confirmed on Whitley SDP) which means the erratum is fixed in hardware for
>> that model and therefore it shouldn't be present in has_if_pschange_mc list.
>> Provisionally assume the same to be the case for Ice Lake-D.
>>
>> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>> ---
>> Changes in v3:
>> - Add Ice Lake-D model numbers
>> - Drop has_if_pschange_mc hunk following Tiger Lake related discussion -
>>    IF_PSCHANGE_MC_NO is confimed to be exposed on Whitley SDP
>>
>> ---
>>   xen/arch/x86/acpi/cpu_idle.c | 2 ++
>>   xen/arch/x86/hvm/vmx/vmx.c   | 2 +-
>>   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 c092086..d788c8b 100644
>> --- a/xen/arch/x86/acpi/cpu_idle.c
>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>> @@ -181,6 +181,8 @@ static void do_get_hw_residencies(void *arg)
>>       case 0x55:
>>       case 0x5E:
>>       /* Ice Lake */
>> +    case 0x6A:
>> +    case 0x6C:
>>       case 0x7D:
>>       case 0x7E:
>>       /* Tiger Lake */
> So I think these changes are correct.  TGL definitely has deeper
> core/package states than this interface enumerates, but I can't locate
> extra MSRs.
>
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 2d4475e..bff5979 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -2775,7 +2775,7 @@ static const struct lbr_info *last_branch_msr_get(void)
>>           /* Goldmont Plus */
>>           case 0x7a:
>>           /* Ice Lake */
>> -        case 0x7d: case 0x7e:
>> +        case 0x6a: case 0x6c: case 0x7d: case 0x7e:
> IceLake Server has what appear to be new aspects to LBR.  I can't find
> LAST_INT_INFO (0x1e0) existing in any previous generation.
Strange.  Updates to SDM finally got published Apr 2 and those contained 
Ice Lake SP details in it - this new LBR hasn't been mentioned. Does 
this mean we can skip it for now as it appears to be undocumented 
functionality (at least publicly) and unlikely used by software?
> However, my investigation also found LBR_SELECT which has been around
> since Nehalem, which we don't handle, and Linux *does* use.

Good point, I'll make an additional patch that adds LBR_SELECT to the 
LBR logic.

Igor