Re: [PATCH v4 2/5] KVM: x86: Add IBPB support

["Woodhouse, David" <dwmw@amazon.co.uk>]

[-- Attachment #1.1: Type: text/plain, Size: 1688 bytes --]



On Wed, 2018-01-31 at 08:50 -0800, Jim Mattson wrote:
> On Wed, Jan 31, 2018 at 5:10 AM, KarimAllah Ahmed <karahmed@amazon.de> wrote:
> 
> > 
> > +               vmx_disable_intercept_for_msr(vmx->vmcs01.msr_bitmap, MSR_IA32_PRED_CMD,
> > +                                             MSR_TYPE_W);
> Why not disable this intercept eagerly, rather than lazily? Unlike
> MSR_IA32_SPEC_CTRL, there is no guest value to save/restore, so there
> is no cost to disabling the intercept if the guest cpuid info declares
> support for it.

The original patch did that, but the problem was that the guest CPU
features weren't known yet so it was done based on boot_cpu_has(),
which isn't right either.

By doing it on first access, we can make it just like the SPEC_CTRL one
which can indeed use guest_cpu_has().

> 
> > 
> > +       if (to_vmx(vcpu)->save_spec_ctrl_on_exit) {
> > +               nested_vmx_disable_intercept_for_msr(
> > +                               msr_bitmap_l1, msr_bitmap_l0,
> > +                               MSR_IA32_PRED_CMD,
> > +                               MSR_TYPE_R);
> > +       }
> I don't think this should be predicated on
> "to_vmx(vcpu)->save_spec_ctrl_on_exit." Why not just
> "guest_cpuid_has(vcpu, X86_FEATURE_IBPB)"? 

Right... we don't even *have* save_spec_ctrl_on_exit in this patch yet,
do we? That's added in a later patch... for spec_ctrl.


> Also, the final argument to
> nested_vmx_disable_intercept_for_msr should be MSR_TYPE_W rather than
> MSR_TYPE_R.
> 

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5210 bytes --]

[-- Attachment #2.1: Type: text/plain, Size: 197 bytes --]




Amazon Web Services UK Limited. Registered in England and Wales with registration number 08650665 and which has its registered office at 60 Holborn Viaduct, London EC1A 2FD, United Kingdom.

[-- Attachment #2.2: Type: text/html, Size: 197 bytes --]