KVM: nVMX: Fix content of MSR_IA32_VMX_ENTRY/EXIT_CTLS
PATCH 1/1 KVM: nVMX: Fix content of MSR_IA32_VMX_ENTRY/EXIT_CTLS
From: Jan Kiszka <[email protected]>
Properly set those bits to 1 that the spec demands in case bit 55 of VMX_BASIC is 0 - like in our case.
--- arch/x86/kvm/vmx.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)
arch/x86/kvm/vmx.c +10 -3
@@ -2050,21 +2050,28 @@ static __init void nested_vmx_setup_ctls_msrs(void)PIN_BASED_EXT_INTR_MASK |PIN_BASED_NMI_EXITING |PIN_BASED_VIRTUAL_NMIS ; -/* exit controls */ - nested_vmx_exit_ctls_low =0 ; +/* +* Exit controls +* If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and +* 17 must be 1. +*/ + nested_vmx_exit_ctls_low =0x36dff ;/* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */ #ifdef CONFIG_X86_64 nested_vmx_exit_ctls_high =VM_EXIT_HOST_ADDR_SPACE_SIZE ;#else nested_vmx_exit_ctls_high =0 ;#endif + nested_vmx_exit_ctls_high |=0x36dff ;/* entry controls */ rdmsr (MSR_IA32_VMX_ENTRY_CTLS , nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); - nested_vmx_entry_ctls_low =0 ; +/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ + nested_vmx_entry_ctls_low =0x11ff ; nested_vmx_entry_ctls_high &=VM_ENTRY_LOAD_IA32_PAT |VM_ENTRY_IA32E_MODE ; + nested_vmx_entry_ctls_high |=0x11ff ;/* cpu-based controls */ rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
Signature · 1 line
1.7.3.4
Paolo Bonzini
maintainerIl 03/03/2013 13:04, Jan Kiszka ha scritto:
Quoted 23 lines
From: Jan Kiszka <[email protected]>
Properly set those bits to 1 that the spec demands in case bit 55 of
VMX_BASICis 0 - like in our case.Signed-off-by Jan Kiszka <[email protected]>--- arch/x86/kvm/vmx.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)arch/x86/kvm/vmx.c +3
-2@@ -2050,21 +2050,28 @@ static __init void nested_vmx_setup_ctls_msrs(void)PIN_BASED_EXT_INTR_MASK |PIN_BASED_NMI_EXITING |PIN_BASED_VIRTUAL_NMIS ; -/* exit controls */ - nested_vmx_exit_ctls_low =0 ; +/* +* Exit controls +* If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2065
Quoted 10 lines
arch/x86/kvm/vmx.c +4
-0+* 17 must be 1. +*/ + nested_vmx_exit_ctls_low =0x36dff ;/* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */ #ifdef CONFIG_X86_64 nested_vmx_exit_ctls_high =VM_EXIT_HOST_ADDR_SPACE_SIZE ;#else nested_vmx_exit_ctls_high =0 ;#endif + nested_vmx_exit_ctls_high |=0x36dff ;
Can you use nested_vmx_exit_ctls_low on the RHS, or define a constant?
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2074
Quoted 9 lines
arch/x86/kvm/vmx.c +3
-1/* entry controls */ rdmsr (MSR_IA32_VMX_ENTRY_CTLS , nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); - nested_vmx_entry_ctls_low =0 ; +/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ + nested_vmx_entry_ctls_low =0x11ff ; nested_vmx_entry_ctls_high &=VM_ENTRY_LOAD_IA32_PAT |VM_ENTRY_IA32E_MODE ; + nested_vmx_entry_ctls_high |=0x11ff ;
Same here.
Paolo
Quoted 3 lines
arch/x86/kvm/vmx.c +0
-0/* cpu-based controls */ rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
Quoted 28 lines · Paolo Bonzini
Il 03/03/2013 13:04, Jan Kiszka ha scritto:
Quoted 27 lines
From: Jan Kiszka <[email protected]>
Properly set those bits to 1 that the spec demands in case bit 55 of
VMX_BASICis 0 - like in our case.Signed-off-by Jan Kiszka <[email protected]>--- arch/x86/kvm/vmx.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-)arch/x86/kvm/vmx.c +6
-2@@ -2050,21 +2050,28 @@ static __init void nested_vmx_setup_ctls_msrs(void)PIN_BASED_EXT_INTR_MASK |PIN_BASED_NMI_EXITING |PIN_BASED_VIRTUAL_NMIS ; -/* exit controls */ - nested_vmx_exit_ctls_low =0 ; +/* +* Exit controls +* If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and +* 17 must be 1. +*/ + nested_vmx_exit_ctls_low =0x36dff ;/* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */
Quoted 8 lines
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2065
Quoted 6 lines
arch/x86/kvm/vmx.c +1
-0#ifdef CONFIG_X86_64 nested_vmx_exit_ctls_high =VM_EXIT_HOST_ADDR_SPACE_SIZE ;#else nested_vmx_exit_ctls_high =0 ;#endif + nested_vmx_exit_ctls_high |=0x36dff ;Can you use
nested_vmx_exit_ctls_lowon the RHS, or define a constant?
I'll define constants.
Jan
Quoted 19 lines
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2074
Quoted 10 lines
arch/x86/kvm/vmx.c +3
-1/* entry controls */ rdmsr (MSR_IA32_VMX_ENTRY_CTLS , nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); - nested_vmx_entry_ctls_low =0 ; +/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ + nested_vmx_entry_ctls_low =0x11ff ; nested_vmx_entry_ctls_high &=VM_ENTRY_LOAD_IA32_PAT |VM_ENTRY_IA32E_MODE ; + nested_vmx_entry_ctls_high |=0x11ff ;Same here.
Paolo
Quoted 3 lines
arch/x86/kvm/vmx.c +0
-0/* cpu-based controls */ rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux
On Sun, Mar 03, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Fix content of MSR_IA32_VMX_ENTRY/EXIT_CTLS":
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2065
Quoted 7 lines
arch/x86/kvm/vmx.c +1
-0/* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */ #ifdef CONFIG_X86_64 nested_vmx_exit_ctls_high =VM_EXIT_HOST_ADDR_SPACE_SIZE ;#else nested_vmx_exit_ctls_high =0 ;#endif + nested_vmx_exit_ctls_high |=0x36dff ;
Can you please compose this 0x36dff out of constants? Is VM_EXIT_HOST_ADDR_SPACE_SIZE one of them?
It's important to verify that we actually support all these bits - even if we *should* support them, it doesn't mean we actually do (but if we do, we should say we do).
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2071
Quoted 3 lines
arch/x86/kvm/vmx.c +2
-1- nested_vmx_entry_ctls_low =0 ; +/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ + nested_vmx_entry_ctls_low =0x11ff ;
Setting nested_vmx_entry_ctls_low = 0 just means that although the spec says only 1 setting is supported, we *also* support 0 setting. I'm not sure why this is a bad thing. Our VMX will be even better than the real processors' ;-)
Signature · 4 lines
Nadav Har'El | Monday, Mar 4 2013, 22 Adar 5773 [email protected] |----------------------------------------- Phone +972-523-790466, ICQ 13349191 |My opinions may have changed, but not the http://nadav.harel.org.il |fact that I am right.
Quoted 11 lines · Nadav Har'El
On Sun, Mar 03, 2013, Jan Kiszka wrote about "[PATCH] KVM: nVMX: Fix content of MSR_IA32_VMX_ENTRY/EXIT_CTLS":
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2065
Quoted 7 lines
arch/x86/kvm/vmx.c +1
-0/* Note that guest use of VM_EXIT_ACK_INTR_ON_EXIT is not supported. */ #ifdef CONFIG_X86_64 nested_vmx_exit_ctls_high =VM_EXIT_HOST_ADDR_SPACE_SIZE ;#else nested_vmx_exit_ctls_high =0 ;#endif + nested_vmx_exit_ctls_high |=0x36dff ;Can you please compose this
0x36dffout of constants? IsVM_EXIT_HOST_ADDR_SPACE_SIZEone of them?
Nope. These are bits enumerated by the spec to be always 1 unless that bit 55 is 1 - what happens to them then remains unclear to me, but is not important right now. I'm about to stick them all into a constant.
Quoted 4 lines
It's important to verify that we actually support all these bits - even if we *should* support them, it doesn't mean we actually do (but if we do, we should say we do).
No, we just implement what the spec demands here. Leaving any of them 0 would make our implementation non-conforming, not just "special" as it is right now.
Quoted 9 lines
arch/x86/kvm/vmx.c static __init void nested_vmx_setup_ctls_msrs(void) · line 2071
Quoted 4 lines
arch/x86/kvm/vmx.c +2
-1- nested_vmx_entry_ctls_low =0 ; +/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ + nested_vmx_entry_ctls_low =0x11ff ;Setting
nested_vmx_entry_ctls_low= 0 just means that although the spec says only 1 setting is supported, we *also* support 0 setting. I'm not sure why this is a bad thing. Our VMX will be even better than the real processors' ;-)
Bad idea. We first need to make it correct, then we can think about optimizations. This is very important if you want to support hypervisors for which you have no code, just the knowledge that they work on real hw.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux