Re: [PATCH] VT-x: extend LBR Broadwell errata coverage

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/05/2020 13:52, Jan Beulich wrote:
> For lbr_tsx_fixup_check() simply name a few more specific errata numbers.
>
> For bdf93_fixup_check(), however, more models are affected. Oddly enough
> despite being the same model and stepping, the erratum is listed for Xeon
> E3 but not its Core counterpart.

That is probably a documentation error.  These processors are made from
the same die, and are not going to deviate in this regard.

> With this it's of course also uncertain
> whether the absence of the erratum for Xeon D is actually meaningful.

Given BDE105, it is exceedingly unlikely that this erratum alone was
fixed, leaving the other related ones present.

The complicating factor is that the TSX errata were addressed in some
later Broadwell parts.  Both these errata groups are to do with a
mismatch of TSX metadata in LBR/LER records.

The former group affects Haswell and Broadwell, but only when microcode
has disabled TSX, and manifests in the processor rejecting
architecturally-correct last-branch-to records.  Any mode in the list
which still has TSX enabled in up-to-date microcode doesn't get the
workaround.

The latter group affects Broadwell only, and manifests as an
architecturally incorrect ler-from record, which shouldn't have any TSX
metadata to begin with.

>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2870,8 +2870,10 @@ static void __init lbr_tsx_fixup_check(v

There is a comment out of context here which is now stale.

If simply adding adding to the list is something you'd prefer to avoid,
what about /* Haswell/Broadwell LBR TSX metadata errata */ or similar?

>      case 0x45: /* HSM182 - 4th gen Core */
>      case 0x46: /* HSM182, HSD172 - 4th gen Core (GT3) */
>      case 0x3d: /* BDM127 - 5th gen Core */
> -    case 0x47: /* BDD117 - 5th gen Core (GT3) */
> -    case 0x4f: /* BDF85  - Xeon E5-2600 v4 */
> +    case 0x47: /* BDD117 - 5th gen Core (GT3)
> +                  BDW117 - Xeon E3-1200 v4 */
> +    case 0x4f: /* BDF85  - Xeon E5-2600 v4
> +                  BDX88  - Xeon E7-x800 v4 */
>      case 0x56: /* BDE105 - Xeon D-1500 */
>          break;
>      default:
> @@ -2895,15 +2897,26 @@ static void __init lbr_tsx_fixup_check(v
>  static void __init bdf93_fixup_check(void)

Seeing as this is no longer just BDF93, how about ler_tsx_fixup_check() ?

~Andrew

>  {
>      /*
> -     * Broadwell erratum BDF93:
> +     * Broadwell erratum BDF93 et al:
>       *
>       * Reads from MSR_LER_TO_LIP (MSR 1DEH) may return values for bits[63:61]
>       * that are not equal to bit[47].  Attempting to context switch this 
> value
>       * may cause a #GP.  Software should sign extend the MSR.
>       */
> -    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> -         boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x4f )
> +    if ( boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
> +         boot_cpu_data.x86 != 6 )
> +        return;
> +
> +    switch ( boot_cpu_data.x86_model )
> +    {
> +    case 0x3d: /* BDM131 - 5th gen Core */
> +    case 0x47: /* BDD??? - 5th gen Core (H-Processor line)
> +                  BDW120 - Xeon E3-1200 v4 */
> +    case 0x4f: /* BDF93  - Xeon E5-2600 v4
> +                  BDX93  - Xeon E7-x800 v4 */
>          bdf93_fixup_needed = true;
> +        break;
> +    }
>  }
>  
>  static int is_last_branch_msr(u32 ecx)