linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: nVMX: expose INS/OUTS information support
@ 2016-09-04 18:23 Jan Dakinevich
  2016-09-05  9:19 ` Yury Norov
  2016-09-05 13:27 ` Paolo Bonzini
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Dakinevich @ 2016-09-04 18:23 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, rkrcmar, linux-kernel, ynorov, Jan Dakinevich

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;
 	vmcs_conf->revision_id = vmx_msr_low;
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: nVMX: expose INS/OUTS information support
  2016-09-04 18:23 [PATCH] KVM: nVMX: expose INS/OUTS information support Jan Dakinevich
@ 2016-09-05  9:19 ` Yury Norov
  2016-09-05 13:27   ` Paolo Bonzini
  2016-09-05 13:27 ` Paolo Bonzini
  1 sibling, 1 reply; 6+ messages in thread
From: Yury Norov @ 2016-09-05  9:19 UTC (permalink / raw)
  To: Jan Dakinevich; +Cc: kvm, pbonzini, rkrcmar, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: nVMX: expose INS/OUTS information support
  2016-09-05  9:19 ` Yury Norov
@ 2016-09-05 13:27   ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-09-05 13:27 UTC (permalink / raw)
  To: Yury Norov, Jan Dakinevich; +Cc: kvm, rkrcmar, linux-kernel



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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: nVMX: expose INS/OUTS information support
  2016-09-04 18:23 [PATCH] KVM: nVMX: expose INS/OUTS information support Jan Dakinevich
  2016-09-05  9:19 ` Yury Norov
@ 2016-09-05 13:27 ` Paolo Bonzini
  2016-09-05 14:47   ` [PATCH v2] " Jan Dakinevich
  1 sibling, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2016-09-05 13:27 UTC (permalink / raw)
  To: Jan Dakinevich, kvm; +Cc: rkrcmar, linux-kernel, ynorov



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;
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2] KVM: nVMX: expose INS/OUTS information support
  2016-09-05 13:27 ` Paolo Bonzini
@ 2016-09-05 14:47   ` Jan Dakinevich
  2016-09-05 14:50     ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Dakinevich @ 2016-09-05 14:47 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, rkrcmar, linux-kernel, ynorov, Jan Dakinevich

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 of
feature presence.

Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
---
v1:
 * don't copy to basic_cap bits, which are already copied to size field
 * fix commit message

 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..10f9fb5 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 & ~0x1fff;
 	vmcs_conf->revision_id = vmx_msr_low;
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] KVM: nVMX: expose INS/OUTS information support
  2016-09-05 14:47   ` [PATCH v2] " Jan Dakinevich
@ 2016-09-05 14:50     ` Paolo Bonzini
  0 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2016-09-05 14:50 UTC (permalink / raw)
  To: Jan Dakinevich, kvm; +Cc: rkrcmar, linux-kernel, ynorov



On 05/09/2016 16:47, 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 of
> feature presence.
> 
> Signed-off-by: Jan Dakinevich <jan.dakinevich@gmail.com>
> ---
> v1:
>  * don't copy to basic_cap bits, which are already copied to size field
>  * fix commit message
> 
>  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..10f9fb5 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 & ~0x1fff;
>  	vmcs_conf->revision_id = vmx_msr_low;
>  
>  	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
> 

Thanks, looks good.

Paolo

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-05 14:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-04 18:23 [PATCH] KVM: nVMX: expose INS/OUTS information support Jan Dakinevich
2016-09-05  9:19 ` Yury Norov
2016-09-05 13:27   ` Paolo Bonzini
2016-09-05 13:27 ` Paolo Bonzini
2016-09-05 14:47   ` [PATCH v2] " Jan Dakinevich
2016-09-05 14:50     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).