[PATCH] x86: support Atom Tremont

[PATCH] x86: support Atom Tremont

[Jan Beulich]

Add model 0x86 to relevant switch() statements, as per SDM 069 Vol 4.
Take the liberty and also change Gemini Lake comments to say Goldmont
Plus. to match the SDM.

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

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -224,6 +224,8 @@ static void do_get_hw_residencies(void *
     case 0x5F:
     /* Goldmont Plus */
     case 0x7A:
+    /* Tremont */
+    case 0x86:
         GET_PC2_RES(hw_res->pc2);
         GET_PC3_RES(hw_res->pc3);
         GET_PC6_RES(hw_res->pc6);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2767,6 +2767,8 @@ static const struct lbr_info *last_branc
         case 0x66:
         /* Goldmont Plus */
         case 0x7a:
+        /* Tremont */
+        case 0x86:
         /* Kaby Lake */
         case 0x8e: case 0x9e:
             return sk_lbr;
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -443,7 +443,8 @@ static bool __init should_use_eager_fpu(
     case 0x5a: /* Moorefield */
     case 0x5c: /* Goldmont */
     case 0x5f: /* Denverton */
-    case 0x7a: /* Gemini Lake */
+    case 0x7a: /* Goldmont Plus */
+    case 0x86: /* Tremont */
         return false;
 
         /*
@@ -530,7 +531,8 @@ static __init void l1tf_calculations(uin
         case 0x5a: /* Moorefield */
         case 0x5c: /* Goldmont */
         case 0x5f: /* Denverton */
-        case 0x7a: /* Gemini Lake */
+        case 0x7a: /* Goldmont Plus */
+        case 0x86: /* Tremont */
             break;
 
             /*





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86: support Atom Tremont

[Andrew Cooper]

On 11/03/2019 16:38, Jan Beulich wrote:
> Add model 0x86 to relevant switch() statements, as per SDM 069 Vol 4.
> Take the liberty and also change Gemini Lake comments to say Goldmont
> Plus. to match the SDM.

Goldmont+ (Goldmont Refresh in some places) is the name of the
Microarchitecture.  Gemini Lake is the name of the SoC platform which
uses the GMT+

In reality, the terms appear to be used interchangeably in the literature.

>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -224,6 +224,8 @@ static void do_get_hw_residencies(void *
>      case 0x5F:
>      /* Goldmont Plus */
>      case 0x7A:
> +    /* Tremont */
> +    case 0x86:
>          GET_PC2_RES(hw_res->pc2);
>          GET_PC3_RES(hw_res->pc3);
>          GET_PC6_RES(hw_res->pc6);
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2767,6 +2767,8 @@ static const struct lbr_info *last_branc
>          case 0x66:
>          /* Goldmont Plus */
>          case 0x7a:
> +        /* Tremont */
> +        case 0x86:
>          /* Kaby Lake */
>          case 0x8e: case 0x9e:
>              return sk_lbr;

These two hunks are fine.  However...

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -443,7 +443,8 @@ static bool __init should_use_eager_fpu(
>      case 0x5a: /* Moorefield */
>      case 0x5c: /* Goldmont */
>      case 0x5f: /* Denverton */
> -    case 0x7a: /* Gemini Lake */
> +    case 0x7a: /* Goldmont Plus */
> +    case 0x86: /* Tremont */
>          return false;

... this whitelist was put in place at the time because we had no idea
what the effect would be.  As it turns out, eagerFPU was a performance
improvement across the board.

When we get some performance numbers from AMD, if they come back
indicating an improvement across the board (which is the expected
outcome), then I will be following through on Wei's original patch and
removing lazyFPU from Xen entirely.

However, in the short term, the prevailing evidence would suggest that
eager is the way to go, not lazy.

>  
>          /*
> @@ -530,7 +531,8 @@ static __init void l1tf_calculations(uin
>          case 0x5a: /* Moorefield */
>          case 0x5c: /* Goldmont */
>          case 0x5f: /* Denverton */
> -        case 0x7a: /* Gemini Lake */
> +        case 0x7a: /* Goldmont Plus */
> +        case 0x86: /* Tremont */

Where has this information come from?  I see no update in the L1TF guidance.

Given that Tremont isn't actually released yet, it should be enumerating
RDCL_NO and not hit this whitelist to begin with.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] x86: support Atom Tremont

[Jan Beulich]

>>> On 11.03.19 at 19:18, <andrew.cooper3@citrix.com> wrote:
> On 11/03/2019 16:38, Jan Beulich wrote:
>> Add model 0x86 to relevant switch() statements, as per SDM 069 Vol 4.
>> Take the liberty and also change Gemini Lake comments to say Goldmont
>> Plus. to match the SDM.
> 
> Goldmont+ (Goldmont Refresh in some places) is the name of the
> Microarchitecture.  Gemini Lake is the name of the SoC platform which
> uses the GMT+
> 
> In reality, the terms appear to be used interchangeably in the literature.

Well, imo we should be using CPU micro-architecture names here.

And no, the SDM does not say Goldmont+, it uses Goldmont Plus.
Very consistently at least in Vol 4.

>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -443,7 +443,8 @@ static bool __init should_use_eager_fpu(
>>      case 0x5a: /* Moorefield */
>>      case 0x5c: /* Goldmont */
>>      case 0x5f: /* Denverton */
>> -    case 0x7a: /* Gemini Lake */
>> +    case 0x7a: /* Goldmont Plus */
>> +    case 0x86: /* Tremont */
>>          return false;
> 
> ... this whitelist was put in place at the time because we had no idea
> what the effect would be.  As it turns out, eagerFPU was a performance
> improvement across the board.
> 
> When we get some performance numbers from AMD, if they come back
> indicating an improvement across the board (which is the expected
> outcome), then I will be following through on Wei's original patch and
> removing lazyFPU from Xen entirely.
> 
> However, in the short term, the prevailing evidence would suggest that
> eager is the way to go, not lazy.

Well, if so, then this should be consistently made so for earlier Atoms
as well. I see no reason why we should treat Tremont differently from
Goldmont etc.

>> @@ -530,7 +531,8 @@ static __init void l1tf_calculations(uin
>>          case 0x5a: /* Moorefield */
>>          case 0x5c: /* Goldmont */
>>          case 0x5f: /* Denverton */
>> -        case 0x7a: /* Gemini Lake */
>> +        case 0x7a: /* Goldmont Plus */
>> +        case 0x86: /* Tremont */
> 
> Where has this information come from?  I see no update in the L1TF guidance.

Implication from Tremont being a successor to Goldmont. I don't
think you're suggesting you suspect Tremont to be vulnerable?

> Given that Tremont isn't actually released yet, it should be enumerating
> RDCL_NO and not hit this whitelist to begin with.

In which case adding the case here would in the worst case be harmless
albeit useless. To be honest I'd prefer to have the entry here until we
have positive confirmation of RDCL_NO being set there unconditionally.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel