All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Andrew Jones <drjones@redhat.com>
Cc: quintela@redhat.com, QEMU Developers <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v4 1/2] target/arm: kvm: Handle DABT with no valid ISS
Date: Fri, 24 Apr 2020 13:16:33 +0100	[thread overview]
Message-ID: <20200424121633.GF3106@work-vm> (raw)
In-Reply-To: <20200417131032.lcyunbjwofsn2nzz@kamzik.brq.redhat.com>

* Andrew Jones (drjones@redhat.com) wrote:
> On Fri, Apr 17, 2020 at 11:39:25AM +0100, Peter Maydell wrote:
> > On Mon, 23 Mar 2020 at 11:32, Beata Michalska
> > <beata.michalska@linaro.org> wrote:
> > >
> > > On ARMv7 & ARMv8 some load/store instructions might trigger a data abort
> > > exception with no valid ISS info to be decoded. The lack of decode info
> > > makes it at least tricky to emulate those instruction which is one of the
> > > (many) reasons why KVM will not even try to do so.
> > >
> > > Add support for handling those by requesting KVM to inject external
> > > dabt into the quest.
> > >
> > > Signed-off-by: Beata Michalska <beata.michalska@linaro.org>
> > > ---
> > >  target/arm/cpu.h     |  2 ++
> > >  target/arm/kvm.c     | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  target/arm/kvm_arm.h | 11 +++++++++++
> > >  3 files changed, 67 insertions(+)
> > >
> > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > > index 4ffd991..4f834c1 100644
> > > --- a/target/arm/cpu.h
> > > +++ b/target/arm/cpu.h
> > > @@ -560,6 +560,8 @@ typedef struct CPUARMState {
> > >          uint64_t esr;
> > >      } serror;
> > >
> > > +    uint8_t ext_dabt_pending; /* Request for injecting ext DABT */
> > 
> > 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.

This is a level I've not looked at I'm afraid.
However, the point at which we're saving the vCPU state is when the
vCPUs are stopped; so as long as your state that you save is everything
you need to restart and you migrate that then you should be OK; but in
the end fromt he migration point of view we just stop the VM and ask for
each devices state.

Dave

> Thanks,
> drew 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Andrew Jones <drjones@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Beata Michalska <beata.michalska@linaro.org>,
	quintela@redhat.com, Christoffer Dall <Christoffer.Dall@arm.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	qemu-arm <qemu-arm@nongnu.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v4 1/2] target/arm: kvm: Handle DABT with no valid ISS
Date: Fri, 24 Apr 2020 13:16:33 +0100	[thread overview]
Message-ID: <20200424121633.GF3106@work-vm> (raw)
In-Reply-To: <20200417131032.lcyunbjwofsn2nzz@kamzik.brq.redhat.com>

* Andrew Jones (drjones@redhat.com) wrote:
> On Fri, Apr 17, 2020 at 11:39:25AM +0100, Peter Maydell wrote:
> > On Mon, 23 Mar 2020 at 11:32, Beata Michalska
> > <beata.michalska@linaro.org> wrote:
> > >
> > > On ARMv7 & ARMv8 some load/store instructions might trigger a data abort
> > > exception with no valid ISS info to be decoded. The lack of decode info
> > > makes it at least tricky to emulate those instruction which is one of the
> > > (many) reasons why KVM will not even try to do so.
> > >
> > > Add support for handling those by requesting KVM to inject external
> > > dabt into the quest.
> > >
> > > Signed-off-by: Beata Michalska <beata.michalska@linaro.org>
> > > ---
> > >  target/arm/cpu.h     |  2 ++
> > >  target/arm/kvm.c     | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  target/arm/kvm_arm.h | 11 +++++++++++
> > >  3 files changed, 67 insertions(+)
> > >
> > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > > index 4ffd991..4f834c1 100644
> > > --- a/target/arm/cpu.h
> > > +++ b/target/arm/cpu.h
> > > @@ -560,6 +560,8 @@ typedef struct CPUARMState {
> > >          uint64_t esr;
> > >      } serror;
> > >
> > > +    uint8_t ext_dabt_pending; /* Request for injecting ext DABT */
> > 
> > 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.

This is a level I've not looked at I'm afraid.
However, the point at which we're saving the vCPU state is when the
vCPUs are stopped; so as long as your state that you save is everything
you need to restart and you migrate that then you should be OK; but in
the end fromt he migration point of view we just stop the VM and ask for
each devices state.

Dave

> Thanks,
> drew 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


  parent reply	other threads:[~2020-04-24 12:18 UTC|newest]

Thread overview: 36+ 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 ` Beata Michalska
2020-03-23 11:32 ` [PATCH v4 1/2] target/arm: kvm: Handle " Beata Michalska
2020-03-23 11:32   ` Beata Michalska
2020-03-23 12:44   ` Andrew Jones
2020-03-23 12:44     ` Andrew Jones
2020-03-25 15:15     ` Beata Michalska
2020-03-25 15:15       ` Beata Michalska
2020-04-17 10:39   ` Peter Maydell
2020-04-17 10:39     ` Peter Maydell
2020-04-17 13:10     ` Andrew Jones
2020-04-17 13:10       ` Andrew Jones
2020-04-18 22:56       ` Beata Michalska
2020-04-18 22:56         ` Beata Michalska
2020-04-24 12:16       ` Dr. David Alan Gilbert [this message]
2020-04-24 12:16         ` Dr. David Alan Gilbert
2020-04-24 12:51         ` Peter Maydell
2020-04-24 12:51           ` Peter Maydell
2020-04-25  9:24         ` Paolo Bonzini
2020-04-25  9:24           ` Paolo Bonzini
2020-04-27  6:18           ` Andrew Jones
2020-04-27  6:18             ` Andrew Jones
2020-03-23 11:32 ` [PATCH v4 2/2] target/arm: kvm: Handle potential issue with dabt injection Beata Michalska
2020-03-23 11:32   ` Beata Michalska
2020-03-23 18:44   ` Richard Henderson
2020-03-23 18:44     ` Richard Henderson
2020-03-25 15:16     ` Beata Michalska
2020-03-25 15:16       ` Beata Michalska
2020-04-03  8:44   ` Andrew Jones
2020-04-03  8:44     ` Andrew Jones
2020-04-07 11:24     ` Peter Maydell
2020-04-07 11:24       ` Peter Maydell
2020-04-07 11:32       ` Beata Michalska
2020-04-07 11:32         ` Beata Michalska
2020-04-07 11:31     ` 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=20200424121633.GF3106@work-vm \
    --to=dgilbert@redhat.com \
    --cc=drjones@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 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.