Re: [PATCH 4/6] x86/cpu-policy: MSR_ARCH_CAPS feature names

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/05/2023 1:27 pm, Jan Beulich wrote:
> On 15.05.2023 16:42, Andrew Cooper wrote:
>> Seed the default visibility from the dom0 special case, which for the most
>> part just exposes the *_NO bits.
> EIBRS and SKIP_L1DFL are outliers here, in not presently being exposed
> to Dom0. If (latent) exposing of them wasn't an oversight, then this would
> imo want justifying here. They'll get exposed, after all, ...

EIBRS is exposed to dom0.  I've intentionally renamed it from
ARCH_CAPS_IBRS_ALL because EIBRS is by far the more recognisable name now.


SKIP_L1DFL is more complicated, but on yet more consideration I think
it's probably wrong here.

The confusion is regarding L1 Terminal Fault, where RDCL_NO was
retroactively declared to mean "also fixes L1TF".  SKIP_L1DFL means "you
don't need to flush on vmentry", which is advertised by real hardware
that also advertises RDCL_NO, but should also be advertised on
vulnerable hardware by the L0 hypervisor to an L1 to say "don't worry,
I'm already taking care of that for you".

So on consideration, I think SKIP_L1DFL should not be visible by
default, and when we start doing nested virt, should be reintroduced
with a dependency on the VMX feature.

>> +    [12] = "doitm",               [13] = "sbdr-ssdp-no",
>> +    [14] = "fbsdp-no",            [15] = "psdp-no",
>> +    /* 16 */                      [17] = "fb-clear",
>> +    [18] = "fb-clear-ctrl",       [19] = "rrsba",
>> +    [20] = "bhi-no",              [21] = "xapic-status",
>> +    /* 22 */                      [23] = "ovrclk-status",
>> +    [24] = "pbrsb-no",
>>  };
>>  
>>  static const char *const str_10Ah[32] =
>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>> @@ -308,6 +308,29 @@ XEN_CPUFEATURE(AVX_NE_CONVERT,     15*32+ 5) /*A  
>> AVX-NE-CONVERT Instructions */
>>  XEN_CPUFEATURE(CET_SSS,            15*32+18) /*   CET Supervisor Shadow 
>> Stacks safe to use */
>>  
>>  /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.eax, word 16 */
>> +XEN_CPUFEATURE(RDCL_NO,            16*32+ 0) /*A  No Rogue Data Cache Load 
>> (Meltdown) */
>> +XEN_CPUFEATURE(EIBRS,              16*32+ 1) /*A  Enhanced IBRS */
>> +XEN_CPUFEATURE(RSBA,               16*32+ 2) /*!A RSB Alternative 
>> (Retpoline not safe) */
>> +XEN_CPUFEATURE(SKIP_L1DFL,         16*32+ 3) /*A  Don't need to flush L1D 
>> on VMEntry */
>> +XEN_CPUFEATURE(INTEL_SSB_NO,       16*32+ 4) /*A  No Speculative Store 
>> Bypass */
>> +XEN_CPUFEATURE(MDS_NO,             16*32+ 5) /*A  No Microarchitectural 
>> Data Sampling */
>> +XEN_CPUFEATURE(IF_PSCHANGE_MC_NO,  16*32+ 6) /*A  No Instruction fetch #MC 
>> */
>> +XEN_CPUFEATURE(TSX_CTRL,           16*32+ 7) /*   MSR_TSX_CTRL */
>> +XEN_CPUFEATURE(TAA_NO,             16*32+ 8) /*A  No TSX Async Abort */
>> +XEN_CPUFEATURE(MCU_CTRL,           16*32+ 9) /*   MSR_MCU_CTRL */
>> +XEN_CPUFEATURE(MISC_PKG_CTRL,      16*32+10) /*   MSR_MISC_PKG_CTRL */
>> +XEN_CPUFEATURE(ENERGY_FILTERING,   16*32+11) /*   
>> MSR_MISC_PKG_CTRL.ENERGY_FILTERING */
> These last two aren't exactly in sync with the SDM; I assume that's
> intended?

Yeah.  I'm bored of Intel failing with naming consistency.  This one, I
was assured that the draft was going to be fixed before publishing it,
and yet...

>
>> +XEN_CPUFEATURE(DOITM,              16*32+12) /*   Data Operand Invariant 
>> Timing Mode */
>> +XEN_CPUFEATURE(SBDR_SSBD_NO,       16*32+13) /*A  No Shared Buffer Data 
>> Read or Sideband Stale Data Propagation */
> SBDR_SSDP_NO?
>
>> +XEN_CPUFEATURE(FBDSP_NO,           16*32+14) /*A  No Fill Buffer Stale Data 
>> Propagation */
> FBSDP_NO?

Oops.  I hate the MMIO vulns especially because they're too similar to
other vulns.  Will fix.

~Andrew