Re: [PATCH v3 4/4] KVM: VMX: Allow direct access to MSR_IA32_SPEC_CTRL

[KarimAllah Ahmed]

On 01/30/2018 11:49 PM, Jim Mattson wrote:
> On Tue, Jan 30, 2018 at 1:00 PM, KarimAllah Ahmed <karahmed@xxxxxxxxxx> wrote:
> > Ooops! I did not think at all about nested :)
> >
> > This should be addressed now, I hope:
> >
> > http://git.infradead.org/linux-retpoline.git/commitdiff/f7f0cbba3e0cffcee050a8a5a9597a162d57e572
>
> +               if (cpu_has_vmx_msr_bitmap() && data &&
> +                   !vmx->save_spec_ctrl_on_exit) {
> +                       vmx->save_spec_ctrl_on_exit = true;
> +
> +                       msr_bitmap = is_guest_mode(vcpu) ?
> vmx->nested.vmcs02.msr_bitmap :
> +
> vmx->vmcs01.msr_bitmap;
> +                       vmx_disable_intercept_for_msr(msr_bitmap,
> +                                                     MSR_IA32_SPEC_CTRL,
> +                                                     MSR_TYPE_RW);
> +               }
>
> There are two ways to get to this point in vmx_set_msr while
> is_guest_mode(vcpu) is true:
> 1) L0 is processing vmcs12's VM-entry MSR load list on emulated
> VM-entry (see enter_vmx_non_root_mode).
> 2) L2 tried to execute WRMSR, writes to the MSR are intercepted in
> vmcs02's MSR permission bitmap, and writes to the MSR are not
> intercepted in vmcs12's MSR permission bitmap.
>
> In the first case, disabling the intercepts for the MSR in
> vmx->nested.vmcs02.msr_bitmap is incorrect, because we haven't yet
> determined that the intercepts are clear in vmcs12's MSR permission
> bitmap.
> In the second case, disabling *both* of the intercepts for the MSR in
> vmx->nested.vmcs02.msr_bitmap is incorrect, because we don't know that
> the read intercept is clear in vmcs12's MSR permission bitmap.
> Furthermore, disabling the write intercept for the MSR in
> vmx->nested.vmcs02.msr_bitmap is somewhat fruitless, because
> nested_vmx_merge_msr_bitmap is just going to undo that change on the
> next emulated VM-entry.

Okay, I took a second look at the code (specially 
nested_vmx_merge_msr_bitmap).

This means that I simply should not touch the MSR bitmap in set_msr in
case of nested, I just need to properly update the l02 msr_bitmap in
nested_vmx_merge_msr_bitmap. As in here:

http://git.infradead.org/linux-retpoline.git/commitdiff/d90eedebdd16bb00741a2c93bc13c5e444c99c2b

or am I still missing something? (sorry, did not actually look at the
nested code before!)

>
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B