Oops, I missed these.
On 09/01/2018 15:22, Konrad Rzeszutek Wilk wrote:
>> + if (have_spec_ctrl) {
>> + rdmsrl(MSR_IA32_SPEC_CTRL, svm->spec_ctrl);
>> + if (svm->spec_ctrl != 0)
> Perhaps just :
>
> if (svm->spec_ctrl) ?
>
> And above too?
These will become != SPEC_CTRL_ENABLE_IBRS or something like that.
>> + wrmsrl(MSR_IA32_SPEC_CTRL, 0);
>> + }
>> + /*
>> + * Speculative execution past the above wrmsrl might encounter
>> + * an indirect branch and use guest-controlled contents of the
>> + * indirect branch predictor; block it.
>> + */
>> + asm("lfence");
> Don't you want this to be part of the if () .. else part?
Not right now, because the processor could speculate that have_spec_ctrl
== 0 and skip the wrmsrl. After it becomes a static_cpu_has, it could
move inside, but only if the kernel is compiled with static keys enabled.
> Meaning:
>
> if (have_spec_ctrl && svm->spec_ctrl)
> wrmsrl(MSR_IA32_SPEC_CTRL, 0);
> else
> asm("lfence");
>
> But .. I am missing something - AMD don't expose 0x48. They expose only 0x49.
>
> That is only the IPBP is needed on AMD? (I haven't actually seen any official
> docs from AMD).
AMD is not exposing 0x48 yet, but they plan to based on my information
from a few weeks ago.
Paolo
