| Message ID | a0af2c5ef19c0520f0832f7270fc7285a458267c.1402919981.git.jan.kiszka@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jan Kiszka <jan.kiszka@siemens.com> writes: > We already implemented them but failed to advertise them. Currently they > all return the identical values to the capability MSRs they are > augmenting. So there is no change in exposed features yet. > > Drop related comments at this chance that are partially incorrect and > redundant anyway. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > arch/x86/include/uapi/asm/msr-index.h | 1 + > arch/x86/kvm/vmx.c | 13 ++----------- > 2 files changed, 3 insertions(+), 11 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h > index fcf2b3a..eaefcc6 100644 > --- a/arch/x86/include/uapi/asm/msr-index.h > +++ b/arch/x86/include/uapi/asm/msr-index.h > @@ -558,6 +558,7 @@ > > /* VMX_BASIC bits and bitmasks */ > #define VMX_BASIC_VMCS_SIZE_SHIFT 32 > +#define VMX_BASIC_TRUE_CTLS (1ULL << 55) Curiosity question - know of any case that failed since we didn't set this bit ? > #define VMX_BASIC_64 0x0001000000000000LLU > #define VMX_BASIC_MEM_TYPE_SHIFT 50 > #define VMX_BASIC_MEM_TYPE_MASK 0x003c000000000000LLU > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 801332e..536f341 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2265,21 +2265,13 @@ static __init void nested_vmx_setup_ctls_msrs(void) > /* pin-based controls */ > rdmsr(MSR_IA32_VMX_PINBASED_CTLS, > nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high); > - /* > - * According to the Intel spec, if bit 55 of VMX_BASIC is off (as it is > - * in our case), bits 1, 2 and 4 (i.e., 0x16) must be 1 in this MSR. > - */ > nested_vmx_pinbased_ctls_low |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR; > nested_vmx_pinbased_ctls_high &= PIN_BASED_EXT_INTR_MASK | > PIN_BASED_NMI_EXITING | PIN_BASED_VIRTUAL_NMIS; > nested_vmx_pinbased_ctls_high |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR | > PIN_BASED_VMX_PREEMPTION_TIMER; > > - /* > - * Exit controls > - * If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and > - * 17 must be 1. > - */ > + /* exit controls */ > rdmsr(MSR_IA32_VMX_EXIT_CTLS, > nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high); > nested_vmx_exit_ctls_low = VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR; > @@ -2299,7 +2291,6 @@ static __init void nested_vmx_setup_ctls_msrs(void) > /* entry controls */ > rdmsr(MSR_IA32_VMX_ENTRY_CTLS, > nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); > - /* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ > nested_vmx_entry_ctls_low = VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR; > nested_vmx_entry_ctls_high &= > #ifdef CONFIG_X86_64 > @@ -2394,7 +2385,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) > * guest, and the VMCS structure we give it - not about the > * VMX support of the underlying hardware. > */ > - *pdata = VMCS12_REVISION | > + *pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS | > ((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) | > (VMX_BASIC_MEM_TYPE_WB << VMX_BASIC_MEM_TYPE_SHIFT); > break; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2014-06-16 18:54, Bandan Das wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> We already implemented them but failed to advertise them. Currently they >> all return the identical values to the capability MSRs they are >> augmenting. So there is no change in exposed features yet. >> >> Drop related comments at this chance that are partially incorrect and >> redundant anyway. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> arch/x86/include/uapi/asm/msr-index.h | 1 + >> arch/x86/kvm/vmx.c | 13 ++----------- >> 2 files changed, 3 insertions(+), 11 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h >> index fcf2b3a..eaefcc6 100644 >> --- a/arch/x86/include/uapi/asm/msr-index.h >> +++ b/arch/x86/include/uapi/asm/msr-index.h >> @@ -558,6 +558,7 @@ >> >> /* VMX_BASIC bits and bitmasks */ >> #define VMX_BASIC_VMCS_SIZE_SHIFT 32 >> +#define VMX_BASIC_TRUE_CTLS (1ULL << 55) > > Curiosity question - know of any case that failed since we didn't > set this bit ? None. But that is likely due to the fact that we exposed a broken PROCBASED_CTLS value. Jan > >> #define VMX_BASIC_64 0x0001000000000000LLU >> #define VMX_BASIC_MEM_TYPE_SHIFT 50 >> #define VMX_BASIC_MEM_TYPE_MASK 0x003c000000000000LLU >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 801332e..536f341 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2265,21 +2265,13 @@ static __init void nested_vmx_setup_ctls_msrs(void) >> /* pin-based controls */ >> rdmsr(MSR_IA32_VMX_PINBASED_CTLS, >> nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high); >> - /* >> - * According to the Intel spec, if bit 55 of VMX_BASIC is off (as it is >> - * in our case), bits 1, 2 and 4 (i.e., 0x16) must be 1 in this MSR. >> - */ >> nested_vmx_pinbased_ctls_low |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR; >> nested_vmx_pinbased_ctls_high &= PIN_BASED_EXT_INTR_MASK | >> PIN_BASED_NMI_EXITING | PIN_BASED_VIRTUAL_NMIS; >> nested_vmx_pinbased_ctls_high |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR | >> PIN_BASED_VMX_PREEMPTION_TIMER; >> >> - /* >> - * Exit controls >> - * If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and >> - * 17 must be 1. >> - */ >> + /* exit controls */ >> rdmsr(MSR_IA32_VMX_EXIT_CTLS, >> nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high); >> nested_vmx_exit_ctls_low = VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR; >> @@ -2299,7 +2291,6 @@ static __init void nested_vmx_setup_ctls_msrs(void) >> /* entry controls */ >> rdmsr(MSR_IA32_VMX_ENTRY_CTLS, >> nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); >> - /* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ >> nested_vmx_entry_ctls_low = VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR; >> nested_vmx_entry_ctls_high &= >> #ifdef CONFIG_X86_64 >> @@ -2394,7 +2385,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) >> * guest, and the VMCS structure we give it - not about the >> * VMX support of the underlying hardware. >> */ >> - *pdata = VMCS12_REVISION | >> + *pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS | >> ((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) | >> (VMX_BASIC_MEM_TYPE_WB << VMX_BASIC_MEM_TYPE_SHIFT); >> break; > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/arch/x86/include/uapi/asm/msr-index.h b/arch/x86/include/uapi/asm/msr-index.h index fcf2b3a..eaefcc6 100644 --- a/arch/x86/include/uapi/asm/msr-index.h +++ b/arch/x86/include/uapi/asm/msr-index.h @@ -558,6 +558,7 @@ /* VMX_BASIC bits and bitmasks */ #define VMX_BASIC_VMCS_SIZE_SHIFT 32 +#define VMX_BASIC_TRUE_CTLS (1ULL << 55) #define VMX_BASIC_64 0x0001000000000000LLU #define VMX_BASIC_MEM_TYPE_SHIFT 50 #define VMX_BASIC_MEM_TYPE_MASK 0x003c000000000000LLU diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 801332e..536f341 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2265,21 +2265,13 @@ static __init void nested_vmx_setup_ctls_msrs(void) /* pin-based controls */ rdmsr(MSR_IA32_VMX_PINBASED_CTLS, nested_vmx_pinbased_ctls_low, nested_vmx_pinbased_ctls_high); - /* - * According to the Intel spec, if bit 55 of VMX_BASIC is off (as it is - * in our case), bits 1, 2 and 4 (i.e., 0x16) must be 1 in this MSR. - */ nested_vmx_pinbased_ctls_low |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR; nested_vmx_pinbased_ctls_high &= PIN_BASED_EXT_INTR_MASK | PIN_BASED_NMI_EXITING | PIN_BASED_VIRTUAL_NMIS; nested_vmx_pinbased_ctls_high |= PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR | PIN_BASED_VMX_PREEMPTION_TIMER; - /* - * Exit controls - * If bit 55 of VMX_BASIC is off, bits 0-8 and 10, 11, 13, 14, 16 and - * 17 must be 1. - */ + /* exit controls */ rdmsr(MSR_IA32_VMX_EXIT_CTLS, nested_vmx_exit_ctls_low, nested_vmx_exit_ctls_high); nested_vmx_exit_ctls_low = VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR; @@ -2299,7 +2291,6 @@ static __init void nested_vmx_setup_ctls_msrs(void) /* entry controls */ rdmsr(MSR_IA32_VMX_ENTRY_CTLS, nested_vmx_entry_ctls_low, nested_vmx_entry_ctls_high); - /* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */ nested_vmx_entry_ctls_low = VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR; nested_vmx_entry_ctls_high &= #ifdef CONFIG_X86_64 @@ -2394,7 +2385,7 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) * guest, and the VMCS structure we give it - not about the * VMX support of the underlying hardware. */ - *pdata = VMCS12_REVISION | + *pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS | ((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) | (VMX_BASIC_MEM_TYPE_WB << VMX_BASIC_MEM_TYPE_SHIFT); break;
We already implemented them but failed to advertise them. Currently they all return the identical values to the capability MSRs they are augmenting. So there is no change in exposed features yet. Drop related comments at this chance that are partially incorrect and redundant anyway. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- arch/x86/include/uapi/asm/msr-index.h | 1 + arch/x86/kvm/vmx.c | 13 ++----------- 2 files changed, 3 insertions(+), 11 deletions(-)