| Message ID | 1473013395-12214-1-git-send-email-jan.dakinevich@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Sep 04, 2016 at 09:23:15PM +0300, Jan Dakinevich wrote: > Expose the feature to L1 hypervisor if host CPU supports it, since > certain hypervisors requires it for own purposes. > > According to Intel SDM A.1, if CPU supports the feature, > VMX_INSTRUCTION_INFO field of VMCS will contain detailed information > about INS/OUTS instructions handling. This field is already copied to > VMCS12 for L1 hypervisor (see prepare_vmcs12 routine) independently > feature presence. > > Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com> > --- > arch/x86/kvm/vmx.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index a4bb2bd..4fd22a6 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -939,6 +939,7 @@ static DEFINE_SPINLOCK(vmx_vpid_lock); > static struct vmcs_config { > int size; > int order; > + u32 basic_cap; > u32 revision_id; > u32 pin_based_exec_ctrl; > u32 cpu_based_exec_ctrl; > @@ -1215,6 +1216,11 @@ static inline bool cpu_has_vmx_ple(void) > SECONDARY_EXEC_PAUSE_LOOP_EXITING; > } > > +static inline bool cpu_has_vmx_basic_inout(void) inline is useless. See Documentation/CodingStyle, Chapter 15 > +{ > + return (((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT); You can shift VMX_BASIC_INOUT (at compile time) and avoid the shifting of vmcs_config.basic_cap at runtime. > +} > + > static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu) > { > return flexpriority_enabled && lapic_in_kernel(vcpu); > @@ -2877,6 +2883,8 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) > *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); > + if (cpu_has_vmx_basic_inout()) > + *pdata |= VMX_BASIC_INOUT; > break; > case MSR_IA32_VMX_TRUE_PINBASED_CTLS: > case MSR_IA32_VMX_PINBASED_CTLS: > @@ -3458,6 +3466,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) > > vmcs_conf->size = vmx_msr_high & 0x1fff; > vmcs_conf->order = get_order(vmcs_config.size); > + vmcs_conf->basic_cap = vmx_msr_high; > vmcs_conf->revision_id = vmx_msr_low; > > vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control; > -- > 1.9.1 -- 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 05/09/2016 11:19, Yury Norov wrote: >> > >> > +static inline bool cpu_has_vmx_basic_inout(void) > inline is useless. See Documentation/CodingStyle, Chapter 15 No, it's not. See Documentation/CodingStyle, Chapter 15: "While the use of inlines can be appropriate (for example as a means of replacing macros, see Chapter 12), it very often is not". >> > +{ >> > + return (((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT); > You can shift VMX_BASIC_INOUT (at compile time) and avoid the shifting of > vmcs_config.basic_cap at runtime. Not really since VMX_BASIC_INOUT comes from the processor manual. It's ironic that your first remark places a lot of trust in the compiler, while the second places none. The compiler is happy to optimize away the left-shift/bitwise-AND pair to this: movq %rdi, %rax shrq $22, %rax andl $1, %eax where 22 is the order of VMX_BASIC_INOUT's only set bit, minus 32. Paolo -- 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 04/09/2016 20:23, Jan Dakinevich wrote: > Expose the feature to L1 hypervisor if host CPU supports it, since > certain hypervisors requires it for own purposes. > > According to Intel SDM A.1, if CPU supports the feature, > VMX_INSTRUCTION_INFO field of VMCS will contain detailed information > about INS/OUTS instructions handling. This field is already copied to > VMCS12 for L1 hypervisor (see prepare_vmcs12 routine) independently > feature presence. > > Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com> > --- > arch/x86/kvm/vmx.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index a4bb2bd..4fd22a6 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -939,6 +939,7 @@ static DEFINE_SPINLOCK(vmx_vpid_lock); > static struct vmcs_config { > int size; > int order; > + u32 basic_cap; > u32 revision_id; > u32 pin_based_exec_ctrl; > u32 cpu_based_exec_ctrl; > @@ -1215,6 +1216,11 @@ static inline bool cpu_has_vmx_ple(void) > SECONDARY_EXEC_PAUSE_LOOP_EXITING; > } > > +static inline bool cpu_has_vmx_basic_inout(void) > +{ > + return (((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT); > +} > + > static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu) > { > return flexpriority_enabled && lapic_in_kernel(vcpu); > @@ -2877,6 +2883,8 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) > *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); > + if (cpu_has_vmx_basic_inout()) > + *pdata |= VMX_BASIC_INOUT; > break; > case MSR_IA32_VMX_TRUE_PINBASED_CTLS: > case MSR_IA32_VMX_PINBASED_CTLS: > @@ -3458,6 +3466,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) > > vmcs_conf->size = vmx_msr_high & 0x1fff; > vmcs_conf->order = get_order(vmcs_config.size); > + vmcs_conf->basic_cap = vmx_msr_high; Bits 32-44 are already stored in vmcs_conf->size, so please make this "vmx_msr_high & ~0x1fff". Otherwise looks fine! Paolo > vmcs_conf->revision_id = vmx_msr_low; > > vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control; > -- 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/kvm/vmx.c b/arch/x86/kvm/vmx.c index a4bb2bd..4fd22a6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -939,6 +939,7 @@ static DEFINE_SPINLOCK(vmx_vpid_lock); static struct vmcs_config { int size; int order; + u32 basic_cap; u32 revision_id; u32 pin_based_exec_ctrl; u32 cpu_based_exec_ctrl; @@ -1215,6 +1216,11 @@ static inline bool cpu_has_vmx_ple(void) SECONDARY_EXEC_PAUSE_LOOP_EXITING; } +static inline bool cpu_has_vmx_basic_inout(void) +{ + return (((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT); +} + static inline bool cpu_need_virtualize_apic_accesses(struct kvm_vcpu *vcpu) { return flexpriority_enabled && lapic_in_kernel(vcpu); @@ -2877,6 +2883,8 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata) *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); + if (cpu_has_vmx_basic_inout()) + *pdata |= VMX_BASIC_INOUT; break; case MSR_IA32_VMX_TRUE_PINBASED_CTLS: case MSR_IA32_VMX_PINBASED_CTLS: @@ -3458,6 +3466,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) vmcs_conf->size = vmx_msr_high & 0x1fff; vmcs_conf->order = get_order(vmcs_config.size); + vmcs_conf->basic_cap = vmx_msr_high; vmcs_conf->revision_id = vmx_msr_low; vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
Expose the feature to L1 hypervisor if host CPU supports it, since certain hypervisors requires it for own purposes. According to Intel SDM A.1, if CPU supports the feature, VMX_INSTRUCTION_INFO field of VMCS will contain detailed information about INS/OUTS instructions handling. This field is already copied to VMCS12 for L1 hypervisor (see prepare_vmcs12 routine) independently feature presence. Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com> --- arch/x86/kvm/vmx.c | 9 +++++++++ 1 file changed, 9 insertions(+)