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

[Jim Mattson]

On Tue, Jan 30, 2018 at 3:50 PM, KarimAllah Ahmed <karahmed@xxxxxxxxxx> wrote:
> 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!)

+               if (cpu_has_vmx_msr_bitmap() && data &&
+                   !vmx->save_spec_ctrl_on_exit) {
+                       vmx->save_spec_ctrl_on_exit = true;
+
+                       if (is_guest_mode(vcpu))
+                               break;

As Paolo suggested, the test for !is_guest_mode (vcpu) should just be
folded into the condition above. If you aren't clearing a 'W' bit in
the MSR permission bitmap, there's no need to set
vmx->save_spec_ctrl_on_exit.

+
+                       vmx_disable_intercept_for_msr(vmx->vmcs01.msr_bitmap,
+                                                     MSR_IA32_SPEC_CTRL,
+                                                     MSR_TYPE_RW);
+               }
+               break;

 ...

+       if (guest_cpuid_has(vcpu, X86_FEATURE_IBRS)) {
+               nested_vmx_disable_intercept_for_msr(
+                               msr_bitmap_l1, msr_bitmap_l0,
+                               MSR_IA32_SPEC_CTRL,
+                               MSR_TYPE_R | MSR_TYPE_W);
+       }
+

However, here, you should set vmx->save_spec_ctrl_on_exit if
nested_vmx_disable_intercept_for_msr clears the 'W' bit for
MSR_IA32_SPEC_CTRL in msr_bitmap_l0. Perhaps this would be easier if
nested_vmx_disable_intercept_for_msr returned something indicative of
which bits it cleared (if any).