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 <[email protected]>
>
> --- 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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to