lkml.org 
[lkml]   [2016]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled
From
Date


On 29/03/2016 12:48, Roman Kagan wrote:
> On Tue, Mar 29, 2016 at 11:34:56AM +0200, Paolo Bonzini wrote:
>> If SynIC is disabled, there is nothing that userspace can do to
>> handle these exits; on the other hand, userspace probably will
>> not know about KVM_EXIT_HYPERV_HCALL and complain about it or
>> even exit. Just prevent anything bad from happening by handling
>> the hypercall in KVM and returning an "invalid hypercall" code.
>
> I wonder if this has been encountered in real life or just found by code
> inspection?

Code inspection; I was looking for an excus^Wreason to submit your patch
for hypercall stubs during QEMU hard freeze, and it occurred to me that
you'd need the stubs even on older QEMU releases. This patch avoids
that regression.

Paolo

>> Fixes: 83326e43f27e9a8a501427a0060f8af519a39bb2
>> Cc: Andrey Smetanin <asmetanin@virtuozzo.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> arch/x86/kvm/hyperv.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
>> index b960d9ea171f..9e2becd20a59 100644
>> --- a/arch/x86/kvm/hyperv.c
>> +++ b/arch/x86/kvm/hyperv.c
>> @@ -1237,14 +1237,17 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>> break;
>> case HVCALL_POST_MESSAGE:
>> case HVCALL_SIGNAL_EVENT:
>> - vcpu->run->exit_reason = KVM_EXIT_HYPERV;
>> - vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
>> - vcpu->run->hyperv.u.hcall.input = param;
>> - vcpu->run->hyperv.u.hcall.params[0] = ingpa;
>> - vcpu->run->hyperv.u.hcall.params[1] = outgpa;
>> - vcpu->arch.complete_userspace_io =
>> + if (vcpu_to_synic(vcpu)->active) {
>> + vcpu->run->exit_reason = KVM_EXIT_HYPERV;
>> + vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
>> + vcpu->run->hyperv.u.hcall.input = param;
>> + vcpu->run->hyperv.u.hcall.params[0] = ingpa;
>> + vcpu->run->hyperv.u.hcall.params[1] = outgpa;
>> + vcpu->arch.complete_userspace_io =
>> kvm_hv_hypercall_complete_userspace;
>> - return 0;
>> + return 0;
>> + }
>> + /* fall through */
>
> I'd rather put it the other way around, with explicit error path and a
> comment:
>
>
> /* don't bother userspace if it has no way to handle it */
> if (!vcpu_to_synic(vcpu)->active) {
> res = HV_STATUS_INVALID_HYPERCALL_CODE;
> break;
> }
>
> but that's just nitpicking, so
>
> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>

Your version is better.

> [ Andrey has left the company so I'm not sure he even receives the
> messages sent to this address ]

He noticed and informed me. I'll Cc both you and his gmail address in
the future.

Paolo

\
 
 \ /
  Last update: 2016-03-29 13:21    [W:0.032 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site