* [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled
@ 2016-03-29 9:34 Paolo Bonzini
2016-03-29 10:48 ` Roman Kagan
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2016-03-29 9:34 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Andrey Smetanin
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.
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 */
default:
res = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled
2016-03-29 9:34 [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled Paolo Bonzini
@ 2016-03-29 10:48 ` Roman Kagan
2016-03-29 10:50 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Roman Kagan @ 2016-03-29 10:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, Andrey Smetanin
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?
> 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>
[ Andrey has left the company so I'm not sure he even receives the
messages sent to this address ]
Thanks,
Roman.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled
2016-03-29 10:48 ` Roman Kagan
@ 2016-03-29 10:50 ` Paolo Bonzini
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-03-29 10:50 UTC (permalink / raw)
To: Roman Kagan, linux-kernel, kvm, Andrey Smetanin
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-03-29 12:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-29 9:34 [PATCH] KVM: Hyper-V: do not do hypercall userspace exits if SynIC is disabled Paolo Bonzini
2016-03-29 10:48 ` Roman Kagan
2016-03-29 10:50 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.