From: "Radim Krčmář" <rkrcmar@redhat.com>
To: "Jan H. Schönherr" <jschoenh@amazon.de>
Cc: David Hildenbrand <david@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Fix handling of pending signal on uninitialized AP
Date: Wed, 13 Sep 2017 17:31:10 +0200 [thread overview]
Message-ID: <20170913153109.GE2673@flask> (raw)
In-Reply-To: <7954a734-3eff-1ff0-f65b-899bde5d69b2@amazon.de>
2017-09-06 17:33+0200, Jan H. Schönherr:
> On 09/06/2017 02:33 PM, David Hildenbrand wrote:
> > On 06.09.2017 00:27, Jan H. Schönherr wrote:
> > > KVM API says that KVM_RUN will return with -EINTR when a signal is
> > > pending. However, if a vCPU is in KVM_MP_STATE_UNINITIALIZED, then
> > > the return value is unconditionally -EAGAIN.
> > >
> > > Copy over some code from vcpu_run(), so that the case of a pending
> > > signal results in the expected return value.
> > >
> > > Signed-off-by: Jan H. Schönherr <jschoenh@amazon.de>
> > > ---
> > > arch/x86/kvm/x86.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > index 272320e..40039cd 100644
> > > --- a/arch/x86/kvm/x86.c
> > > +++ b/arch/x86/kvm/x86.c
> > > @@ -7203,6 +7203,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> > > kvm_apic_accept_events(vcpu);
> > > kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> > > r = -EAGAIN;
> > > + if (signal_pending(current)) {
> > > + r = -EINTR;
> > > + vcpu->run->exit_reason = KVM_EXIT_INTR;
> > > + ++vcpu->stat.signal_exits;
> > > + }
> > > goto out;
> > > }
> > >
> >
> > I am not sure if this is the right thing to do. E.g. also on s390x a
> > -EINVAL is indicated if the VCPU is stopped.
> >
> > If the documentation is unclear, maybe that one should be fixed. I don't
> > see this to be relevant in practice, or is it?
> >
> > -EINTR will only be returned if nothing else hinders the VCPU from running.
> >
>
> In practice, in allows me to distinguish, whether I can reenter KVM_RUN immediately
> (-EAGAIN), or whether I have to check for signals first (-EINTR), or whether I need
> to have a look at the actual exit reason (0).
Makes sense and the worst thing I found was ugliness of the code ...
Queued, thanks.
prev parent reply other threads:[~2017-09-13 15:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-05 22:27 [PATCH] KVM: x86: Fix handling of pending signal on uninitialized AP Jan H. Schönherr
[not found] ` <a41dda44-9d6d-100f-d900-ef8f10bd07ee@redhat.com>
2017-09-06 15:33 ` Jan H. Schönherr
2017-09-13 15:31 ` Radim Krčmář [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170913153109.GE2673@flask \
--to=rkrcmar@redhat.com \
--cc=david@redhat.com \
--cc=jschoenh@amazon.de \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.