| Message ID | 20171122102340.7110-3-arbel.moshe@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I believe this change is incorrect, for a couple of reasons. (1) The VMX capability MSRs are static. Once they have been observed by L1, they cannot be changed. Doing so would be a violation of the architectural specification. IIUC, vmx_refresh_apicv_exec_ctrl() may be executed after L1 has observed the values of the VMX capability MSRs, because there are no restrictions on when userspace can make the KVM_ENABLE_CAP ioctl. (2) I don't believe that enabling Hyper-V SynIC support for L1 precludes the use of the advanced APIC virtualization features for the execution of L2. On Wed, Nov 22, 2017 at 2:23 AM, Arbel Moshe <arbel.moshe@oracle.com> wrote: > Fix bug of not updating nested MSRs regarding APICv, when refreshing > apicv exec ctrl. > > Before this commit, enabling Hyper-V SynIC would disable APICv controls > in VMCS but still expose APICv controls to nested guest. > > Signed-off-by: Arbel Moshe <arbel.moshe@oracle.com> > Reviewed-by: Liran Alon <liran.alon@oracle.com> > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > --- > arch/x86/kvm/vmx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 84ccd3b2762c..0450fbdb97be 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -5253,6 +5253,8 @@ static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu) > > if (cpu_has_vmx_msr_bitmap()) > vmx_set_msr_bitmap(vcpu); > + > + nested_vmx_setup_ctls_msrs(vmx); > } > > static u32 vmx_exec_control(struct vcpu_vmx *vmx) > -- > 2.14.1 >
On 22/11/2017 19:04, Jim Mattson wrote: > I believe this change is incorrect, for a couple of reasons. > (1) The VMX capability MSRs are static. Once they have been observed > by L1, they cannot be changed. Doing so would be a violation of the > architectural specification. IIUC, vmx_refresh_apicv_exec_ctrl() may > be executed after L1 has observed the values of the VMX capability > MSRs, because there are no restrictions on when userspace can make the > KVM_ENABLE_CAP ioctl. This is technically true, but I think we can say comfortably that it makes little sense and userspace Should Not Have Done It. > (2) I don't believe that enabling Hyper-V SynIC support for L1 > precludes the use of the advanced APIC virtualization features for the > execution of L2. This is true. Paolo
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 84ccd3b2762c..0450fbdb97be 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -5253,6 +5253,8 @@ static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu) if (cpu_has_vmx_msr_bitmap()) vmx_set_msr_bitmap(vcpu); + + nested_vmx_setup_ctls_msrs(vmx); } static u32 vmx_exec_control(struct vcpu_vmx *vmx)