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

From: Jim Mattson
Date: Tue Jan 30 2018 - 19:16:31 EST


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).