Messages in this thread | | | Subject | Re: [PATCH v5 4/5] KVM: VMX: Allow direct access to MSR_IA32_SPEC_CTRL | From | KarimAllah Ahmed <> | Date | Thu, 1 Feb 2018 18:46:12 +0100 |
| |
On 02/01/2018 06:37 PM, KarimAllah Ahmed wrote: > On 02/01/2018 02:25 PM, David Woodhouse wrote: >> >> >> On Wed, 2018-01-31 at 23:26 -0500, Konrad Rzeszutek Wilk wrote: >>> >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>> index 6a9f4ec..bfc80ff 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -594,6 +594,14 @@ struct vcpu_vmx { >>>> #endif >>>> u64 arch_capabilities; >>>> + u64 spec_ctrl; >>>> + >>>> + /* >>>> + * This indicates that: >>>> + * 1) guest_cpuid_has(X86_FEATURE_IBRS) == true && >>>> + * 2) The guest has actually initiated a write against the MSR. >>>> + */ >>>> + bool spec_ctrl_used; >>>> /* >>>> * This indicates that: >> >> Thanks for persisting with the details here, Karim. In addition to >> Konrad's heckling at the comments, I'll add my own request to his... >> >> I'd like the comment for spec_ctrl_used to explain why it isn't >> entirely redundant with the spec_ctrl_intercepted() function. >> >> Without nesting, I believe it *would* be redundant, but the difference >> comes when an L2 is running for which L1 has not permitted the MSR to >> be passed through. That's when we have spec_ctrl_used = true but the >> MSR *isn't* actually passed through in the active msr_bitmap. >> >> Question: if spec_ctrl_used is always equivalent to the intercept bit >> in the vmcs01.msr_bitmap, just not the guest bitmap... should we ditch >> it and always use the bit from the vmcs01.msr_bitmap? > > If I used the vmcs01.msr_bitmap, spec_ctrl_used will always be true if > L0 passed it to L1. Even if L1 did not actually pass it to L2 and even > if L2 has not written to it yet (!used). > > This pretty much renders the short-circuit at > nested_vmx_merge_msr_bitmap useless: > > if (!nested_cpu_has_virt_x2apic_mode(vmcs12) && > !to_vmx(vcpu)->pred_cmd_used && > !to_vmx(vcpu)->spec_ctrl_used) > return false; > > ... and the default path will be kvm_vcpu_gpa_to_page + kmap. > > That being said, I have to admit the logic for spec_ctrl_used is not > perfect either. > > If L1 or any of the L2s touched the MSR, spec_ctrl_used will be set to > true. So if one L2 used the MSR, all other L2s will also skip the short- > circuit mentioned above and end up *always* going through > kvm_vcpu_gpa_to_page + kmap. > > Maybe all of this is over-thinking and in reality the short-circuit > above is really useless and all L2 guests are happily using x2apic :) >
hehe ..
>> if spec_ctrl_used is always equivalent to the intercept bit in the vmcs01.msr_bitmap
actually yes, we can.
I just forgot that we update the msr bitmap lazily! :)
>> >> Sorry :) >> 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
| |