| Message ID | 20221021153521.1216911-14-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: hyper-v: Fine-grained TLB flush + L2 TLB flush features | expand |
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>
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> >
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 --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) {