From: Andrew Jones <drjones@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: quintela@redhat.com, QEMU Developers <qemu-devel@nongnu.org>,
qemu-arm <qemu-arm@nongnu.org>,
kvmarm@lists.cs.columbia.edu,
"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH v4 1/2] target/arm: kvm: Handle DABT with no valid ISS
Date: Mon, 27 Apr 2020 08:18:27 +0200 [thread overview]
Message-ID: <20200427061827.s3fcyvdu3r3bfijj@kamzik.brq.redhat.com> (raw)
In-Reply-To: <d6f4b17c-1202-3aef-320a-247dc0295277@redhat.com>
On Sat, Apr 25, 2020 at 11:24:14AM +0200, Paolo Bonzini wrote:
> On 24/04/20 14:16, Dr. David Alan Gilbert wrote:
> >>> I was trying to work out whether we need to migrate this state,
> >>> and I'm not sure. Andrew, do you know? I think this comes down
> >>> to "at what points in QEMU's kvm run loop can migration kick in",
> >>> and specifically if we get a KVM_EXIT_ARM_NISV do we definitely
> >>> go round the loop and KVM_RUN again without ever checking
> >>> to see if we should do a migration ?
> >>>
> >> I'd prefer a migration expert confirm this, so I've CC'ed David and Juan,
> >> but afaict there's no way to break out of the KVM_RUN loop after a
> >> successful (ret=0) call to kvm_arch_handle_exit() until after the next
> >> KVM_RUN ioctl. This is because even if migration kicks the vcpus between
> >> kvm_arch_handle_exit() and the next run, the signal won't do anything
> >> other than prepare the vcpu for an immediate exit.
>
> As far as QEMU is concerned, this should be enough for Beata's patch to
> be safe. If the signal causes KVM to exit before KVM_EXIT_ARM_NISV,
> it's of course okay. If you get a KVM_EXIT_ARM_NISV, however, KVM_RUN
> will exit with return code 0 and kvm_cpu_exec will:
>
> - set env->ext_dabt_pending
>
> - go round the loop again
>
> - notice cpu->exit_request and schedule an immediate exit
>
> - call kvm_arch_put_registers
>
> - call KVM_RUN again, which will exit with -EINTR
>
> - exit the loop and allow migration to proceed
This was my understanding as well. Thanks for the confirmation.
>
> However, I'm not sure that it's a good idea to
>
> + /* Clear instantly if the call was successful */
> + env->ext_dabt_pending = 0;
>
> Rather, this should be done by the next kvm_arch_get_registers when it
> calls KVM_GET_VCPU_EVENTS. It's also possible to add an assertion in
> kvm_get_vcpu_events that it you always get zero, to justify that the
> field is not migrated.
I like the idea of a balanced API; keeping the 'set' abort pending
until the next 'get', but this event doesn't work that way.
Documentation/virt/kvm/api.rst states:
It is not possible to read back a pending external abort (injected via
KVM_SET_VCPU_EVENTS or otherwise) because such an exception is always
delivered directly to the virtual CPU).
I think clearing the userspace copy instantly after a successful
KVM_SET_VCPU_EVENTS implements that correctly.
Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next prev parent reply other threads:[~2020-04-27 6:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-23 11:32 [PATCH v4 0/2] target/arm: kvm: Support for KVM DABT with no valid ISS Beata Michalska
2020-03-23 11:32 ` [PATCH v4 1/2] target/arm: kvm: Handle " Beata Michalska
2020-03-23 12:44 ` Andrew Jones
2020-03-25 15:15 ` Beata Michalska
2020-04-17 10:39 ` Peter Maydell
2020-04-17 13:10 ` Andrew Jones
2020-04-18 22:56 ` Beata Michalska
2020-04-24 12:16 ` Dr. David Alan Gilbert
2020-04-24 12:51 ` Peter Maydell
2020-04-25 9:24 ` Paolo Bonzini
2020-04-27 6:18 ` Andrew Jones [this message]
2020-03-23 11:32 ` [PATCH v4 2/2] target/arm: kvm: Handle potential issue with dabt injection Beata Michalska
2020-03-23 18:44 ` Richard Henderson
2020-03-25 15:16 ` Beata Michalska
2020-04-03 8:44 ` Andrew Jones
2020-04-07 11:24 ` Peter Maydell
2020-04-07 11:32 ` Beata Michalska
2020-04-07 11:31 ` Beata Michalska
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=20200427061827.s3fcyvdu3r3bfijj@kamzik.brq.redhat.com \
--to=drjones@redhat.com \
--cc=dgilbert@redhat.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=pbonzini@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@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 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).