On Wed, Jan 20, 2016 at 02:43:01PM +0100, Paolo Bonzini wrote:
>
>
> On 12/01/2016 11:50, Andrey Smetanin wrote:
> > The patch implements KVM_EXIT_HV_HCALL functionality
> > for Hyper-V VMBus hypercalls: HV_X64_HCALL_POST_MESSAGE,
> > HV_X64_HCALL_SIGNAL_EVENT.
> >
> > Signed-off-by: Andrey Smetanin <asmetanin@xxxxxxxxxxxxx>
> > Reviewed-by: Roman Kagan <rkagan@xxxxxxxxxxxxx>
> > CC: Gleb Natapov <gleb@xxxxxxxxxx>
> > CC: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> > CC: Joerg Roedel <joro@xxxxxxxxxx>
> > CC: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>
> > CC: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> > CC: Roman Kagan <rkagan@xxxxxxxxxxxxx>
> > CC: Denis V. Lunev <den@xxxxxxxxxx>
> > CC: qemu-devel@xxxxxxxxxx
> > ---
> > Documentation/virtual/kvm/api.txt | 8 ++++++++
> > arch/x86/kvm/hyperv.c | 28 +++++++++++++++++++++-------
> > arch/x86/kvm/hyperv.h | 1 +
> > arch/x86/kvm/x86.c | 3 +++
> > include/uapi/linux/kvm.h | 7 +++++++
> > 5 files changed, 40 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index 053f613..23d4b9d 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -3359,6 +3359,14 @@ Hyper-V SynIC state change. Notification is used to remap SynIC
> > event/message pages and to enable/disable SynIC messages/events processing
> > in userspace.
> >
> > + /* KVM_EXIT_HYPERV_HCALL */
> > + struct {
> > + __u64 input;
> > + __u64 params[2];
> > + __u64 result;
> > + } hv_hcall;
> > +Indicates that the VCPU exits into userspace to process some guest
> > +Hyper-V hypercalls.
>
> Is this something other than KVM_EXIT_HYPERV because of the previous
> discussion about MSRs? Otherwise, it definitely should be a separate
> type of the existing KVM_EXIT_HYPERV exit.
Mostly. Besides, it wasn't obvious why we needed another level of
dispatch.
> (In fact, I do not think anymore that I will add the MSR exit in 4.5.
> It's kind of against the KVM design to do "CPU things" outside the
> kernel. My conjecture is that MSRs are either simple and thus not
> security sensitive, or complicated and thus require state that resides
> in the kernel).
>
> > /* Fix the size of the union. */
> > char padding[256];
> > };
> > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> > index 0e7c90f..76c9ec4 100644
> > --- a/arch/x86/kvm/hyperv.c
> > +++ b/arch/x86/kvm/hyperv.c
> > @@ -1087,18 +1087,32 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
> > case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
> > kvm_vcpu_on_spin(vcpu);
> > break;
> > + case HV_X64_HCALL_POST_MESSAGE:
> > + case HV_X64_HCALL_SIGNAL_EVENT:
> > + vcpu->run->exit_reason = KVM_EXIT_HYPERV_HCALL;
> > + vcpu->run->hv_hcall.input = param;
> > + vcpu->run->hv_hcall.params[0] = ingpa;
> > + vcpu->run->hv_hcall.params[1] = outgpa;
> > + return 0;
> > default:
> > res = HV_STATUS_INVALID_HYPERCALL_CODE;
> > break;
> > }
> >
> > ret = res | (((u64)rep_done & 0xfff) << 32);
> > - if (longmode) {
> > - kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
> > - } else {
> > - kvm_register_write(vcpu, VCPU_REGS_RDX, ret >> 32);
> > - kvm_register_write(vcpu, VCPU_REGS_RAX, ret & 0xffffffff);
> > - }
> > -
> > + kvm_hv_hypercall_set_result(vcpu, ret);
> > return 1;
> > }
> > +
> > +void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
> > +{
> > + bool longmode;
> > +
> > + longmode = is_64_bit_mode(vcpu);
> > + if (longmode)
> > + kvm_register_write(vcpu, VCPU_REGS_RAX, result);
> > + else {
> > + kvm_register_write(vcpu, VCPU_REGS_RDX, result >> 32);
> > + kvm_register_write(vcpu, VCPU_REGS_RAX, result & 0xffffffff);
> > + }
> > +}
> > diff --git a/arch/x86/kvm/hyperv.h b/arch/x86/kvm/hyperv.h
> > index 60eccd4..64a4a3b 100644
> > --- a/arch/x86/kvm/hyperv.h
> > +++ b/arch/x86/kvm/hyperv.h
> > @@ -52,6 +52,7 @@ int kvm_hv_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata);
> >
> > bool kvm_hv_hypercall_enabled(struct kvm *kvm);
> > int kvm_hv_hypercall(struct kvm_vcpu *vcpu);
> > +void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result);
> >
> > void kvm_hv_irq_routing_update(struct kvm *kvm);
> > int kvm_hv_synic_set_irq(struct kvm *kvm, u32 vcpu_id, u32 sint);
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index fad1d09..6ad3599 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -6886,6 +6886,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> > } else
> > WARN_ON(vcpu->arch.pio.count || vcpu->mmio_needed);
> >
> > + if (unlikely(kvm_run->exit_reason == KVM_EXIT_HYPERV_HCALL))
> > + kvm_hv_hypercall_set_result(vcpu, kvm_run->hv_hcall.result);
> > +
> > r = vcpu_run(vcpu);
> >
> > out:
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 9da9051..a62c4fb 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -199,6 +199,7 @@ struct kvm_hyperv_exit {
> > #define KVM_EXIT_S390_STSI 25
> > #define KVM_EXIT_IOAPIC_EOI 26
> > #define KVM_EXIT_HYPERV 27
> > +#define KVM_EXIT_HYPERV_HCALL 28
> >
> > /* For KVM_EXIT_INTERNAL_ERROR */
> > /* Emulate instruction failed. */
> > @@ -355,6 +356,12 @@ struct kvm_run {
> > } eoi;
> > /* KVM_EXIT_HYPERV */
> > struct kvm_hyperv_exit hyperv;
> > + /* KVM_EXIT_HYPERV_HCALL */
> > + struct {
> > + __u64 input;
> > + __u64 params[2];
> > + __u64 result;
>
> Please put result before params, so that it's possible to add more
> parameters in the future.
The number of parameters is fixed in the Hyper-V spec: they are either
physical addresses of the input and output parameter structures, or (for
"fast" hypercalls) up to two input parameters.
That said, there's also another variant called "extended fast
hypercalls" when extra parameters are placed in XMM registers so if we
ever decide to implement those we'll need another field in this
structure, so it indeed looks sensible to swap result and params now.
Thanks,
Roman.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
