Re: [PATCH for-4.11] x86/spec_ctrl: Updates to retpoline-safety decision making

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 19/04/18 10:00, Jan Beulich wrote:
>>>> On 18.04.18 at 20:13, <andrew.cooper3@citrix.com> wrote:
>> @@ -151,6 +152,20 @@ static bool __init retpoline_safe(void)
>>           boot_cpu_data.x86 != 6 )
>>          return false;
>>  
>> +    if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
>> +    {
>> +        uint64_t caps;
>> +
>> +        rdmsrl(MSR_ARCH_CAPABILITIES, caps);
>> +
>> +        /*
>> +         * RBSA may be set by a hypervisor to indicate that we may move to a
>> +         * processor which isn't retpoline-safe.
>> +         */
>> +        if ( caps & ARCH_CAPS_RSBA )
>> +            return false;
>> +    }
> I assume handing this through / forcing it on for guests is meant to be
> done in a later patch?

Virtualisaing MSR_ARCH_CAPS for guests is in progress, but definitely
4.12 material.

It requires the toolstack CPUID/MSR improvements so this bit can be
levelled across resource pools, which means dusting off some fairly old
patches of mine, and getting them finished.

>
>> @@ -177,18 +192,37 @@ static bool __init retpoline_safe(void)
>>           * versions.
>>           */
>>      case 0x3d: /* Broadwell */
>> -        return ucode_rev >= 0x28;
>> +        return ucode_rev >= 0x2a;
>>      case 0x47: /* Broadwell H */
>> -        return ucode_rev >= 0x1b;
>> +        return ucode_rev >= 0x1d;
>>      case 0x4f: /* Broadwell EP/EX */
>> -        return ucode_rev >= 0xb000025;
>> +        return ucode_rev >= 0xb000021;
>>      case 0x56: /* Broadwell D */
>> -        return false; /* TBD. */
>> +        switch ( boot_cpu_data.x86_mask )
>> +        {
>> +        case 2:  return ucode_rev >= 0x15;
>> +        case 3:  return ucode_rev >= 0x7000012;
>> +        case 4:  return ucode_rev >= 0xf000011;
>> +        case 5:  return ucode_rev >= 0xe000009;
>> +        default: return false;
>> +        }
>> +        break;
> Hmm, the white paper says
> "The predictable speculative behavior of the RET instruction is the key to
>  retpoline being a robust mitigation. RET has this behavior on all processors
>  which are based on the Intel® microarchitecture codename Broadwell and
>  earlier when updated with the latest microcode."
>
> Am I to assume the text is imprecise, or else why is it that only Broadwells
> are being checked for ucode version?

Hmm yes - that does look like poor wording in the whitepaper.  It is the
case that Broadwell is the only uarch which needs the microcode check.

> Also the default case of the
> Broadwell sub-switch would perhaps better gain a printk() just like ...

Will do.

>
>>          /*
>> -         * Skylake and later processors are not retpoline-safe.
>> +         * Skylake, Kabylake and Cannonlake processors are not retpoline-safe.
>>           */
>> +    case 0x4e:
>> +    case 0x55:
>> +    case 0x5e:
>> +    case 0x66:
>> +    case 0x67:
>> +    case 0x8e:
>> +    case 0x9e:
>> +        return false;
>> +
>>      default:
>> +        printk("Unrecognised CPU model %#x - assuming not reptpoline safe\n",
>> +               boot_cpu_data.x86_model);
>>          return false;
> ... this one?
>
> Also, looking at ark.intel.com I find Coffee Lake, not Cannon Lake.

I was going from Linux's arch/x86/include/asm/intel-family.h which is
still on my TODO list to stea^W port to Xen, unless someone feels like
beating me to it.

It seems that Cannonlake may not actually be out yet, while Coffeelake
does look like it is definitely out.  I'm completely confused....

~Andrew

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