Re: [Xen-devel] [PATCH 4/9] x86/amd: Introduce CPUID/MSR definitions for per-vcpu SSBD support

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/12/2018 16:39, Jan Beulich wrote:
>>>> On 03.12.18 at 17:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>> At the time of writing, the spec is available from:
>>
>>   
>> https://developer.amd.com/wp-content/resources/124441_AMD64_SpeculativeStoreB
>>  
>> ypassDisable_Whitepaper_final.pdf
>>
>> Future hardware (Zen v2) is expect to have hardware MSR_SPEC_CTRL support,
>> including SPEC_CTRL.SSBD, and with the expectation that this will be directly
>> passed through to guests for performance.
>>
>> On currently released hardware, the only mechanism available is the legacy
>> LS_CFG option, and this is very expensive to use.  Furthermore, emulating
>> MSR_SPEC_CTRL via interception is prohibitively expensive, as certain OSes 
>> use
>> the write-discard flexibility to simplify their entry/exit logic.
> With this, ...
>
>> As an alternative, MSR_VIRT_SPEC_CTRL is specified as an architectural 
>> control
>> (with semantics equivilent to MSR_SPEC_CTRL) which is provided by the
>> hypervisor.  This abstracts away the model-specific details of the LS_CFG
>> mechanism, which allows migration safety to be retained.
> ... how is this any less expensive, when it necessarily requires
> interception?

For an individual update?  No difference.

For the case where a certain OS takes the presence of MSR_SPEC_CTRL to
mean that it needs to write MSR_SPEC_CTRL.IBRS wherever it would choose
to if IBRS was actually available, the perf difference is in the number
of writes which occur and get intercepted.

>  At least the way things are worded, I'm getting
> the impression that you consider this less expensive.
>
>> --- a/tools/libxl/libxl_cpuid.c
>> +++ b/tools/libxl/libxl_cpuid.c
>> @@ -245,6 +245,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
>> *cpuid, const char* str)
>>          {"invtsc",       0x80000007, NA, CPUID_REG_EDX,  8,  1},
>>  
>>          {"ibpb",         0x80000008, NA, CPUID_REG_EBX, 12,  1},
>> +        {"amd_ibrs",     0x80000008, NA, CPUID_REG_EBX, 14,  1},
> While of the following two the names indeed clash with Intel's, the
> above one doesn't. Any reason you still gave it an amd_ prefix?

I really really wish we didn't have duplicate bits, or that I'd started
with blanket Intel and AMD prefixes.

I deliberately chose "IBRSB" for the Intel bit due to its double
meaning, but using just IBRS here on its own is liable to get confused
with the Intel bit.

>
>> +        {"amd_stibp",    0x80000008, NA, CPUID_REG_EBX, 15,  1},
>> +        {"amd_ssbd",     0x80000008, NA, CPUID_REG_EBX, 24,  1},
>> +        {"virt_sc_ssbd", 0x80000008, NA, CPUID_REG_EBX, 25,  1},
>> +        {"amd_ssb_no",   0x80000008, NA, CPUID_REG_EBX, 26,  1},
> Since you're at it, why not also introduce names for bits 16-18
> at this occasion?

I haven't previously filled in names for the sake of it.

The reason that ibrs/stibp/ssbd are here is because they're related and
I've also got a followon few patches to support MSR_VIRT_SPEC_CTRL on
Rome hardware via MSR_SPEC_CTRL, but I need an SDP and some
experimentation time before I'd be happy posting them.

But to address your question, I can't locate those bits at all.  Not
even in the NDA docs or Linux source.

~Andrew

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