kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoffer Dall <christoffer.dall@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	kvm-devel <kvm@vger.kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	open list <linux-kernel@vger.kernel.org>,
	Gleb Natapov <gleb@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	arm-mail-list <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: KVM: export current vcpu->pause state via pseudo regs
Date: Mon, 4 Aug 2014 14:11:31 +0200	[thread overview]
Message-ID: <20140804121131.GC524@cbox> (raw)
In-Reply-To: <87egx0ilkg.fsf@linaro.org>

On Fri, Aug 01, 2014 at 10:48:36AM +0100, Alex Bennée wrote:
> 
> Christoffer Dall writes:
> 
> > On Thu, Jul 31, 2014 at 05:45:28PM +0100, Peter Maydell wrote:
> >> On 31 July 2014 17:38, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >> >> > If we are not complaining when setting the pause value to false if it
> >> >> > was true before, then we probably also need to wake up the thread in
> >> >> > case this is called from another thread, right?
> >> >> >
> >> >> > or perhaps we should just return an error if you're trying to un-pause a
> >> >> > CPU through this interface, hmmmm.
> >> >>
> >> >> Wouldn't it be an error to mess with any register when the system is not
> >> >> in a quiescent state? I was assuming that the wake state is dealt with
> >> >> when the run loop finally restarts.
> >> >>
> >> >
> >> > The ABI doesn't really define it as an error (the ABI doesn't enforce
> >> > anything right now) so the question is, does it ever make sense to clear
> >> > the pause flag through this ioctl?  If not, I think we should just err
> >> > on the side of caution and specify in the docs that this is not
> >> > supported and return an error.
> >> 
> >> Consider the case where the reset state of the system is
> >> "CPU 0 running, CPUs 1..N stopped", and we're doing an
> >> incoming migration to a state where all CPUs are running.
> >> In that case we'll be using this ioctl to clear the pause flag,
> >> right? (We'll also obviously need to set the PC and other
> >> register state correctly before resuming the guest.)
> >> 
> > Doh, you're right, I somehow had it in my mind that when you send the
> > thread a signal, the pause flag would be cleared, but that goes against
> > the whole idea of a CPU being turned off for KVM.
> >
> > But wouldn't we then have to also wake up the thread when clearing the
> > pause flag?  It feels strange that the ioctl can clear the pause flag,
> > but keep the thread on a wake-queue, and then userspace has to send the
> > thread a signal of some sort to wake it up?
> <snip>
> 
> Isn't the vCPU off the wait-queue by definition if the ioctl exits and
> you go through the KVM_SET_ONE_REG stuff?
> 
> Once you re-enter the KVM_RUN ioctl it sees the pause_flag as cleared
> and falls straight through into kvm_guest_enter() otherwise it will
> again wait on wait_event_interruptible(*wq, !vcpu->arch.pause).
> 
Yeah, you're right, I forgot we grab the vcpu->mutex.

-Christoffer

  reply	other threads:[~2014-08-04 12:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-09 13:55 [PATCH] arm64: KVM: export current vcpu->pause state via pseudo regs Alex Bennée
2014-07-31 14:35 ` Christoffer Dall
2014-07-31 15:14   ` Alex Bennée
2014-07-31 16:38     ` Christoffer Dall
2014-07-31 16:45       ` Peter Maydell
2014-07-31 16:50         ` Christoffer Dall
2014-07-31 16:53           ` Peter Maydell
2014-08-01  9:48           ` Alex Bennée
2014-08-04 12:11             ` Christoffer Dall [this message]
2014-08-01  9:11       ` Alex Bennée
2014-08-04 12:13         ` Christoffer Dall
2014-07-31 16:57 ` Paolo Bonzini
2014-07-31 17:04   ` Peter Maydell
2014-07-31 17:21     ` Paolo Bonzini
2014-07-31 17:36       ` Peter Maydell
2014-07-31 17:44         ` Will Deacon
2014-08-04 12:16       ` Christoffer Dall
2014-08-04 12:35         ` Alex Bennée

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=20140804121131.GC524@cbox \
    --to=christoffer.dall@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=gleb@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=will.deacon@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).