On 30.01.2024 13:06, Roger Pau Monné wrote:
> On Tue, Jan 30, 2024 at 11:57:17AM +0100, Jan Beulich wrote:
>> On 30.01.2024 10:13, Roger Pau Monne wrote:
>>> The CPUID feature bit signals the presence of the IPRED_DIS_{U,S} controls
>>> in
>>> SPEC_CTRL MSR.
>>>
>>> Note that those controls are not used by the hypervisor.
>>
>> Despite this, ...
>>
>>> --- a/xen/arch/x86/msr.c
>>> +++ b/xen/arch/x86/msr.c
>>> @@ -324,6 +324,9 @@ uint64_t msr_spec_ctrl_valid_bits(const struct
>>> cpu_policy *cp)
>>> return (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP |
>>> (ssbd ? SPEC_CTRL_SSBD : 0) |
>>> (psfd ? SPEC_CTRL_PSFD : 0) |
>>> + (cp->feat.ipred_ctrl ? (SPEC_CTRL_IPRED_DIS_U |
>>> + SPEC_CTRL_IPRED_DIS_S)
>>> + : 0) |
>>> 0);
>>> }
>>
>> ... if I'm not mistaken exposing SPEC_CTRL bits to guests is independent
>> of whether we write SPEC_CTRL on entry to Xen. Therefore I think in the
>> description it wants clarifying why it is acceptable to run Xen with the
>> guest chosen settings for at least the DIS_S bit (assuming that it is
>> okay to do so). Likely (didn't look there yet) also applicable to the
>> further two patches.
>
> "The added feature is made dependent on IBRSB, which ensures it will
> only be exposed if X86_FEATURE_SC_MSR_{PV,HVM} is available, and that
> ensures the value of SPEC_CTRL will get context switched on exit/entry
> to guest."
>
> Would adding the above to the commit message clarify the intended
> implementation?
It would improve things, at least hinting towards there being a connection
between exposure and updating on entry to Xen. I'd like to ask though to
avoid "context switch" when talking about entry from guest context. While
in a way technically correct, our normal meaning of the term is the
process of switching vCPU-s out/in on a pCPU.
Jan
