[lkml]   [2018]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 4/8] kvm: vmx: Set IBPB when running a different VCPU
On Fri, 2018-01-12 at 09:03 -0800, Jim Mattson wrote:
> The point behind the IPBP in vmx_vcpu_load is to prevent one VCPU from
> steering the speculative execution of the next. If the VMCS address is
> recycled, vmx_vcpu_load doesn't realize that the VCPUs are different,
> and so it won't issue the IPBP.

I don't understand the sequence of events that could lead to this.

If the VMCS is freed, surely per_cpu(current_vmcs, cpu) has to be
cleared? If the VMCS is freed while it's still *active* on a CPU,
that's a bug, surely? And if that CPU is later offlined and clears the
VMCS, it's going to scribble on freed (and potentially re-used) memory.

So vmx_cpu_load() *will* realise that it's different, won't it?

>> +       if (have_spec_ctrl)
>> +               wrmsrl(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);

Also, I think the same condition applies to the conditional branches
over the IBPB-frobbing, as it does to setting IBRS. You can eschew the
'else lfence' only if you put in a comment showing that you've proved
it's safe. Many of the other bits like this are being done with
alternatives, which avoids that concern completely.

But really, I don't like this series much. Don't say "let's do this
until upstream supports...". Just fix it up properly, and add the
generic X86_FEATURE_IBPB bit and use it. We have *too* many separate
tiny patch sets, and we need to be getting our act together and putting
it all in one.

[unhandled content-type:application/x-pkcs7-signature]
 \ /
  Last update: 2018-01-14 23:26    [W:0.093 / U:33.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site