diff mbox

KVM: nVMX: emulate the INVVPID instruction

Message ID BLU436-SMTP5413CD8F74C5A28A5A178880440@phx.gbl (mailing list archive)
State New, archived
Headers show

Commit Message

Wanpeng Li Sept. 23, 2015, 7:59 a.m. UTC 
Add the INVVPID instruction emulation.

Reviewed-by: Wincy Van <fanwenyi0529@gmail.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/include/asm/vmx.h |  1 +
 arch/x86/kvm/vmx.c         | 23 ++++++++++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini Sept. 23, 2015, 8:39 a.m. UTC | #1 
On 23/09/2015 09:59, Wanpeng Li wrote:
> Add the INVVPID instruction emulation.
> 
> Reviewed-by: Wincy Van <fanwenyi0529@gmail.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/include/asm/vmx.h |  1 +
>  arch/x86/kvm/vmx.c         | 23 ++++++++++++++++++++++-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index d25f32a..69f3d71 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -397,6 +397,7 @@ enum vmcs_field {
>  #define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	(KVM_USER_MEM_SLOTS + 2)
>  
>  #define VMX_NR_VPIDS				(1 << 16)
> +#define VMX_VPID_EXTENT_INDIVIDUAL_ADDR 	0
>  #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1
>  #define VMX_VPID_EXTENT_ALL_CONTEXT		2
>  
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 6ad991a..794c529 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7189,7 +7189,28 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>  
>  static int handle_invvpid(struct kvm_vcpu *vcpu)
>  {
> -	kvm_queue_exception(vcpu, UD_VECTOR);
> +	u32 vmx_instruction_info;
> +	unsigned long type;
> +
> +	if (!nested_vmx_check_permission(vcpu))
> +		return 1;
> +
> +	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> +	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
> +
> +	switch (type) {
> +	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
> +	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
> +	case VMX_VPID_EXTENT_ALL_CONTEXT:
> +		vmx_flush_tlb(vcpu);
> +		nested_vmx_succeed(vcpu);
> +		break;
> +	default:
> +		nested_vmx_failInvalid(vcpu);
> +		break;
> +	}
> +
> +	skip_emulated_instruction(vcpu);
>  	return 1;
>  }
>  
> 

This is not enough.  You need to add a VPID argument to
vpid_sync_vcpu_single, and inline vmx_flush_tlb in handle_invvpid so
that it can use the new VPID argument of vpid_sync_vcpu_single.

Note that the "all context" variant can be mapped to
vpid_sync_vcpu_single with vpid02 as the argument (a nice side effect of
your vpid02 design).

However, I have applied the patch to kvm/queue.  Please send the changes
separately, and I will squash them in the existing VPID patch.

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
Wanpeng Li Sept. 23, 2015, 10:30 a.m. UTC | #2 
On 9/23/15 4:39 PM, Paolo Bonzini wrote:
>
> On 23/09/2015 09:59, Wanpeng Li wrote:
>> Add the INVVPID instruction emulation.
>>
>> Reviewed-by: Wincy Van <fanwenyi0529@gmail.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>   arch/x86/include/asm/vmx.h |  1 +
>>   arch/x86/kvm/vmx.c         | 23 ++++++++++++++++++++++-
>>   2 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
>> index d25f32a..69f3d71 100644
>> --- a/arch/x86/include/asm/vmx.h
>> +++ b/arch/x86/include/asm/vmx.h
>> @@ -397,6 +397,7 @@ enum vmcs_field {
>>   #define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	(KVM_USER_MEM_SLOTS + 2)
>>   
>>   #define VMX_NR_VPIDS				(1 << 16)
>> +#define VMX_VPID_EXTENT_INDIVIDUAL_ADDR 	0
>>   #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1
>>   #define VMX_VPID_EXTENT_ALL_CONTEXT		2
>>   
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 6ad991a..794c529 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -7189,7 +7189,28 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>>   
>>   static int handle_invvpid(struct kvm_vcpu *vcpu)
>>   {
>> -	kvm_queue_exception(vcpu, UD_VECTOR);
>> +	u32 vmx_instruction_info;
>> +	unsigned long type;
>> +
>> +	if (!nested_vmx_check_permission(vcpu))
>> +		return 1;
>> +
>> +	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
>> +	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
>> +
>> +	switch (type) {
>> +	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
>> +	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>> +	case VMX_VPID_EXTENT_ALL_CONTEXT:
>> +		vmx_flush_tlb(vcpu);
>> +		nested_vmx_succeed(vcpu);
>> +		break;
>> +	default:
>> +		nested_vmx_failInvalid(vcpu);
>> +		break;
>> +	}
>> +
>> +	skip_emulated_instruction(vcpu);
>>   	return 1;
>>   }
>>   
>>
> This is not enough.  You need to add a VPID argument to
> vpid_sync_vcpu_single, and inline vmx_flush_tlb in handle_invvpid so
> that it can use the new VPID argument of vpid_sync_vcpu_single.
>
> Note that the "all context" variant can be mapped to
> vpid_sync_vcpu_single with vpid02 as the argument (a nice side effect of
> your vpid02 design).
>

Got it. Just send out patches to handle this. :-)

Regards,
Wanpeng Li
--
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
Bandan Das Sept. 24, 2015, 3:45 p.m. UTC | #3 
Paolo Bonzini <pbonzini@redhat.com> writes:
...
>> @@ -7189,7 +7189,28 @@ static int handle_invept(struct kvm_vcpu *vcpu)
>>  
>>  static int handle_invvpid(struct kvm_vcpu *vcpu)
>>  {
>> -	kvm_queue_exception(vcpu, UD_VECTOR);
>> +	u32 vmx_instruction_info;
>> +	unsigned long type;
>> +
>> +	if (!nested_vmx_check_permission(vcpu))
>> +		return 1;
>> +
>> +	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
>> +	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
>> +
>> +	switch (type) {
>> +	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
>> +	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>> +	case VMX_VPID_EXTENT_ALL_CONTEXT:
>> +		vmx_flush_tlb(vcpu);
>> +		nested_vmx_succeed(vcpu);
>> +		break;
>> +	default:
>> +		nested_vmx_failInvalid(vcpu);
>> +		break;
>> +	}
>> +
>> +	skip_emulated_instruction(vcpu);
>>  	return 1;
>>  }
>>  
>> 
>
> This is not enough.  You need to add a VPID argument to
> vpid_sync_vcpu_single, and inline vmx_flush_tlb in handle_invvpid so
> that it can use the new VPID argument of vpid_sync_vcpu_single.
>
> Note that the "all context" variant can be mapped to
> vpid_sync_vcpu_single with vpid02 as the argument (a nice side effect of
> your vpid02 design).
>
> However, I have applied the patch to kvm/queue.  Please send the changes
> separately, and I will squash them in the existing VPID patch.

Please don't do this. It's making it really difficult to review these
patches individually :( Why not let them get some review time before
applying them all together ?


> 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
--
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
Paolo Bonzini Sept. 25, 2015, 7:53 a.m. UTC | #4 
On 24/09/2015 17:45, Bandan Das wrote:
> > However, I have applied the patch to kvm/queue.  Please send the changes
> > separately, and I will squash them in the existing VPID patch.
> 
> Please don't do this. It's making it really difficult to review these
> patches individually :( Why not let them get some review time before
> applying them all together ?

Ok---I did it because it makes sense to keep this patch separate from
the others.  You can expose VPID even if vpid02 == vpid01 (in fact
that's what happens if KVM cannot find a vpid02) and in that case this
patch provides a valid implementation of INVVPID.

Do you think it would help if I posted the whole kvm/queue contents a
few days before pushing it to kvm/next?

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
Bandan Das Sept. 25, 2015, 2:54 p.m. UTC | #5 
Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/09/2015 17:45, Bandan Das wrote:
>> > However, I have applied the patch to kvm/queue.  Please send the changes
>> > separately, and I will squash them in the existing VPID patch.
>> 
>> Please don't do this. It's making it really difficult to review these
>> patches individually :( Why not let them get some review time before
>> applying them all together ?
>
> Ok---I did it because it makes sense to keep this patch separate from
> the others.  You can expose VPID even if vpid02 == vpid01 (in fact
> that's what happens if KVM cannot find a vpid02) and in that case this
> patch provides a valid implementation of INVVPID.
>
> Do you think it would help if I posted the whole kvm/queue contents a
> few days before pushing it to kvm/next?

Oh that would be great. Thank you!

> 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
diff mbox

Patch

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index d25f32a..69f3d71 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -397,6 +397,7 @@  enum vmcs_field {
 #define IDENTITY_PAGETABLE_PRIVATE_MEMSLOT	(KVM_USER_MEM_SLOTS + 2)
 
 #define VMX_NR_VPIDS				(1 << 16)
+#define VMX_VPID_EXTENT_INDIVIDUAL_ADDR 	0
 #define VMX_VPID_EXTENT_SINGLE_CONTEXT		1
 #define VMX_VPID_EXTENT_ALL_CONTEXT		2
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6ad991a..794c529 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7189,7 +7189,28 @@  static int handle_invept(struct kvm_vcpu *vcpu)
 
 static int handle_invvpid(struct kvm_vcpu *vcpu)
 {
-	kvm_queue_exception(vcpu, UD_VECTOR);
+	u32 vmx_instruction_info;
+	unsigned long type;
+
+	if (!nested_vmx_check_permission(vcpu))
+		return 1;
+
+	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
+	type = kvm_register_readl(vcpu, (vmx_instruction_info >> 28) & 0xf);
+
+	switch (type) {
+	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
+	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
+	case VMX_VPID_EXTENT_ALL_CONTEXT:
+		vmx_flush_tlb(vcpu);
+		nested_vmx_succeed(vcpu);
+		break;
+	default:
+		nested_vmx_failInvalid(vcpu);
+		break;
+	}
+
+	skip_emulated_instruction(vcpu);
 	return 1;
 }