Re: [Xen-devel] [PATCH] x86: support Atom Tremont

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.03.19 at 19:18, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel