diff mbox series

[v12,13/46] KVM: x86: Prepare kvm_hv_flush_tlb() to handle L2's GPAs

Message ID 20221021153521.1216911-14-vkuznets@redhat.com
State New
Headers
Series KVM: x86: hyper-v: Fine-grained TLB flush + L2 TLB flush features |

Commit Message

Vitaly Kuznetsov Oct. 21, 2022, 3:34 p.m. UTC 
  To handle L2 TLB flush requests, KVM needs to translate the specified
L2 GPA to L1 GPA to read hypercall arguments from there.

No functional change as KVM doesn't handle VMCALL/VMMCALL from L2 yet.

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 arch/x86/kvm/hyperv.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Sean Christopherson Oct. 26, 2022, 10:26 p.m. UTC | #1 
On Fri, Oct 21, 2022, Vitaly Kuznetsov wrote:
> To handle L2 TLB flush requests, KVM needs to translate the specified
> L2 GPA to L1 GPA to read hypercall arguments from there.
> 
> No functional change as KVM doesn't handle VMCALL/VMMCALL from L2 yet.
> 
> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  arch/x86/kvm/hyperv.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index fca9c51891f5..df1efb821eb0 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -23,6 +23,7 @@
>  #include "ioapic.h"
>  #include "cpuid.h"
>  #include "hyperv.h"
> +#include "mmu.h"
>  #include "xen.h"
>  
>  #include <linux/cpu.h>
> @@ -1908,6 +1909,12 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>  	 */
>  	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
>  
> +	if (!hc->fast && is_guest_mode(vcpu)) {

Please add a comment explaining why only "slow" hypercalls need to translate the
GPA from L2=>L1.

With a comment (and assuming this isn't a bug),

Reviewed-by: Sean Christopherson <seanjc@google.com>
Vitaly Kuznetsov Oct. 27, 2022, 8:18 a.m. UTC | #2 
Sean Christopherson <seanjc@google.com> writes:

> On Fri, Oct 21, 2022, Vitaly Kuznetsov wrote:
>> To handle L2 TLB flush requests, KVM needs to translate the specified
>> L2 GPA to L1 GPA to read hypercall arguments from there.
>> 
>> No functional change as KVM doesn't handle VMCALL/VMMCALL from L2 yet.
>> 
>> Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  arch/x86/kvm/hyperv.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>> 
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index fca9c51891f5..df1efb821eb0 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -23,6 +23,7 @@
>>  #include "ioapic.h"
>>  #include "cpuid.h"
>>  #include "hyperv.h"
>> +#include "mmu.h"
>>  #include "xen.h"
>>  
>>  #include <linux/cpu.h>
>> @@ -1908,6 +1909,12 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>>  	 */
>>  	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
>>  
>> +	if (!hc->fast && is_guest_mode(vcpu)) {
>
> Please add a comment explaining why only "slow" hypercalls need to translate the
> GPA from L2=>L1.
>
> With a comment (and assuming this isn't a bug),

This is intended,

For "slow" hypercalls 'hc->ingpa' is the GPA (or an 'nGPA' -- thus the
patch) in guest memory where hypercall parameters are placed, kvm reads
them with kvm_read_guest() later. For "fast" hypercalls 'ingpa' is a
misnomer as it is not an address but the first parameter (in the 'tlb
flush' case it's 'address space id' which we currently don't
analyze). We may want to add a union in 'struct kvm_hv_hcall' to make
this explicit.

The comment I'm thinking of would be:

"
/*
 * 'Slow' hypercall's first parameter is the address in guest's memory where
 * hypercall parameters are placed. This is either a GPA or a nested GPA when
 * KVM is handling the call from L2 ('direct' TLB flush), translate the address
 * here so the memory can be uniformly read with kvm_read_guest().
 */
"

>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
>
Sean Christopherson Oct. 27, 2022, 9:39 p.m. UTC | #3 
On Thu, Oct 27, 2022, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > On Fri, Oct 21, 2022, Vitaly Kuznetsov wrote:
> >> @@ -1908,6 +1909,12 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
> >>  	 */
> >>  	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
> >>  
> >> +	if (!hc->fast && is_guest_mode(vcpu)) {
> >
> > Please add a comment explaining why only "slow" hypercalls need to translate the
> > GPA from L2=>L1.
> >
> > With a comment (and assuming this isn't a bug),
> 
> This is intended,
> 
> For "slow" hypercalls 'hc->ingpa' is the GPA (or an 'nGPA' -- thus the
> patch) in guest memory where hypercall parameters are placed, kvm reads
> them with kvm_read_guest() later. For "fast" hypercalls 'ingpa' is a
> misnomer as it is not an address but the first parameter (in the 'tlb
> flush' case it's 'address space id' which we currently don't
> analyze). We may want to add a union in 'struct kvm_hv_hcall' to make
> this explicit.

Ya, a union would be helpful.  I'm pretty sure at some point I knew the "fast"
ingpa isn't actually a GPA, but obviously forgot that detail.
diff mbox series

Patch

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index fca9c51891f5..df1efb821eb0 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -23,6 +23,7 @@ 
 #include "ioapic.h"
 #include "cpuid.h"
 #include "hyperv.h"
+#include "mmu.h"
 #include "xen.h"
 
 #include <linux/cpu.h>
@@ -1908,6 +1909,12 @@  static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
 	 */
 	BUILD_BUG_ON(KVM_HV_MAX_SPARSE_VCPU_SET_BITS > 64);
 
+	if (!hc->fast && is_guest_mode(vcpu)) {
+		hc->ingpa = translate_nested_gpa(vcpu, hc->ingpa, 0, NULL);
+		if (unlikely(hc->ingpa == INVALID_GPA))
+			return HV_STATUS_INVALID_HYPERCALL_INPUT;
+	}
+
 	if (hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST ||
 	    hc->code == HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE) {
 		if (hc->fast) {