All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"James Morse" <james.morse@arm.com>,
	"Julien Thierry" <julien.thierry@arm.com>,
	"Suzuki K Pouloze" <suzuki.poulose@arm.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"\"Daniel P . Berrangé\"" <berrange@redhat.com>,
	arm-mail-list <linux-arm-kernel@lists.infradead.org>,
	kvmarm@lists.cs.columbia.edu,
	"lkml - Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded
Date: Thu, 05 Sep 2019 10:15:49 +0100	[thread overview]
Message-ID: <86k1anrtmi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFEAcA9qkqkOTqSVrhTpt-NkZSNXomSBNiWo_D6Kr=QKYRRf=w@mail.gmail.com>

On Thu, 05 Sep 2019 09:56:44 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Thu, 5 Sep 2019 at 09:52, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Thu, 05 Sep 2019 09:16:54 +0100,
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > This is true, but the problem is that barfing out to userspace
> > > makes it harder to debug the guest because it means that
> > > the VM is immediately destroyed, whereas AIUI if we
> > > inject some kind of exception then (assuming you're set up
> > > to do kernel-debug via gdbstub) you can actually examine
> > > the offending guest code with a debugger because at least
> > > your VM is still around to inspect...
> >
> > To Christoffer's point, I find the benefit a bit dubious. Yes, you get
> > an exception, but the instruction that caused it may be completely
> > legal (store with post-increment, for example), leading to an even
> > more puzzled developer (that exception should never have been
> > delivered the first place).
> 
> Right, but the combination of "host kernel prints a message
> about an unsupported load/store insn" and "within-guest debug
> dump/stack trace/etc" is much more useful than just having
> "host kernel prints message" and "QEMU exits"; and it requires
> about 3 lines of code change...

Which is wrong, and creates a new behaviour that isn't specified
anywhere.

> 
> > I'm far more in favour of dumping the state of the access in the run
> > structure (much like we do for a MMIO access) and let userspace do
> > something about it (such as dumping information on the console or
> > breaking). It could even inject an exception *if* the user has asked
> > for it.
> 
> ...whereas this requires agreement on a kernel-userspace API,
> larger changes in the kernel, somebody to implement the userspace
> side of things, and the user to update both the kernel and QEMU.
> It's hard for me to see that the benefit here over the 3-line
> approach really outweighs the extra effort needed.

3 lines that already require the host kernel to be updated, and create
a legacy that we'll never be able to get rid of.

> In practice saying "we should do this" is saying "we're going to do
> nothing", based on the historical record.

Thanks for the vote of confidence...

	M.

-- 
Jazz is not dead, it just smells funny.

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "\"Daniel P . Berrangé\"" <berrange@redhat.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"lkml - Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	kvmarm@lists.cs.columbia.edu,
	arm-mail-list <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded
Date: Thu, 05 Sep 2019 10:15:49 +0100	[thread overview]
Message-ID: <86k1anrtmi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFEAcA9qkqkOTqSVrhTpt-NkZSNXomSBNiWo_D6Kr=QKYRRf=w@mail.gmail.com>

On Thu, 05 Sep 2019 09:56:44 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Thu, 5 Sep 2019 at 09:52, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Thu, 05 Sep 2019 09:16:54 +0100,
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > This is true, but the problem is that barfing out to userspace
> > > makes it harder to debug the guest because it means that
> > > the VM is immediately destroyed, whereas AIUI if we
> > > inject some kind of exception then (assuming you're set up
> > > to do kernel-debug via gdbstub) you can actually examine
> > > the offending guest code with a debugger because at least
> > > your VM is still around to inspect...
> >
> > To Christoffer's point, I find the benefit a bit dubious. Yes, you get
> > an exception, but the instruction that caused it may be completely
> > legal (store with post-increment, for example), leading to an even
> > more puzzled developer (that exception should never have been
> > delivered the first place).
> 
> Right, but the combination of "host kernel prints a message
> about an unsupported load/store insn" and "within-guest debug
> dump/stack trace/etc" is much more useful than just having
> "host kernel prints message" and "QEMU exits"; and it requires
> about 3 lines of code change...

Which is wrong, and creates a new behaviour that isn't specified
anywhere.

> 
> > I'm far more in favour of dumping the state of the access in the run
> > structure (much like we do for a MMIO access) and let userspace do
> > something about it (such as dumping information on the console or
> > breaking). It could even inject an exception *if* the user has asked
> > for it.
> 
> ...whereas this requires agreement on a kernel-userspace API,
> larger changes in the kernel, somebody to implement the userspace
> side of things, and the user to update both the kernel and QEMU.
> It's hard for me to see that the benefit here over the 3-line
> approach really outweighs the extra effort needed.

3 lines that already require the host kernel to be updated, and create
a legacy that we'll never be able to get rid of.

> In practice saying "we should do this" is saying "we're going to do
> nothing", based on the historical record.

Thanks for the vote of confidence...

	M.

-- 
Jazz is not dead, it just smells funny.
_______________________________________________
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: Marc Zyngier <maz@kernel.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "\"Daniel P . Berrangé\"" <berrange@redhat.com>,
	"Suzuki K Pouloze" <suzuki.poulose@arm.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"Julien Thierry" <julien.thierry@arm.com>,
	"lkml - Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"James Morse" <james.morse@arm.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	kvmarm@lists.cs.columbia.edu,
	arm-mail-list <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded
Date: Thu, 05 Sep 2019 10:15:49 +0100	[thread overview]
Message-ID: <86k1anrtmi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFEAcA9qkqkOTqSVrhTpt-NkZSNXomSBNiWo_D6Kr=QKYRRf=w@mail.gmail.com>

On Thu, 05 Sep 2019 09:56:44 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Thu, 5 Sep 2019 at 09:52, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Thu, 05 Sep 2019 09:16:54 +0100,
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > This is true, but the problem is that barfing out to userspace
> > > makes it harder to debug the guest because it means that
> > > the VM is immediately destroyed, whereas AIUI if we
> > > inject some kind of exception then (assuming you're set up
> > > to do kernel-debug via gdbstub) you can actually examine
> > > the offending guest code with a debugger because at least
> > > your VM is still around to inspect...
> >
> > To Christoffer's point, I find the benefit a bit dubious. Yes, you get
> > an exception, but the instruction that caused it may be completely
> > legal (store with post-increment, for example), leading to an even
> > more puzzled developer (that exception should never have been
> > delivered the first place).
> 
> Right, but the combination of "host kernel prints a message
> about an unsupported load/store insn" and "within-guest debug
> dump/stack trace/etc" is much more useful than just having
> "host kernel prints message" and "QEMU exits"; and it requires
> about 3 lines of code change...

Which is wrong, and creates a new behaviour that isn't specified
anywhere.

> 
> > I'm far more in favour of dumping the state of the access in the run
> > structure (much like we do for a MMIO access) and let userspace do
> > something about it (such as dumping information on the console or
> > breaking). It could even inject an exception *if* the user has asked
> > for it.
> 
> ...whereas this requires agreement on a kernel-userspace API,
> larger changes in the kernel, somebody to implement the userspace
> side of things, and the user to update both the kernel and QEMU.
> It's hard for me to see that the benefit here over the 3-line
> approach really outweighs the extra effort needed.

3 lines that already require the host kernel to be updated, and create
a legacy that we'll never be able to get rid of.

> In practice saying "we should do this" is saying "we're going to do
> nothing", based on the historical record.

Thanks for the vote of confidence...

	M.

-- 
Jazz is not dead, it just smells funny.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2019-09-05  9:15 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04 18:07 [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded Heinrich Schuchardt
2019-09-04 18:07 ` Heinrich Schuchardt
2019-09-04 18:07 ` Heinrich Schuchardt
2019-09-05  1:35 ` Guoheyi
2019-09-05  1:35   ` Guoheyi
2019-09-05  8:03 ` Marc Zyngier
2019-09-05  8:03   ` Marc Zyngier
2019-09-05  8:03   ` Marc Zyngier
2019-09-05  8:16   ` Peter Maydell
2019-09-05  8:16     ` Peter Maydell
2019-09-05  8:16     ` Peter Maydell
2019-09-05  8:25     ` Christoffer Dall
2019-09-05  8:25       ` Christoffer Dall
2019-09-05  8:25       ` Christoffer Dall
2019-09-05  8:32       ` Peter Maydell
2019-09-05  8:32         ` Peter Maydell
2019-09-05  8:32         ` Peter Maydell
2019-09-05  8:48     ` Heinrich Schuchardt
2019-09-05  8:48       ` Heinrich Schuchardt
2019-09-05  8:48       ` Heinrich Schuchardt
2019-09-05  8:52     ` Marc Zyngier
2019-09-05  8:52       ` Marc Zyngier
2019-09-05  8:52       ` Marc Zyngier
2019-09-05  8:56       ` Peter Maydell
2019-09-05  8:56         ` Peter Maydell
2019-09-05  8:56         ` Peter Maydell
2019-09-05  9:15         ` Marc Zyngier [this message]
2019-09-05  9:15           ` Marc Zyngier
2019-09-05  9:15           ` Marc Zyngier
2019-09-05  9:22         ` Christoffer Dall
2019-09-05  9:22           ` Christoffer Dall
2019-09-05  9:22           ` Christoffer Dall
2019-09-05 13:09           ` Marc Zyngier
2019-09-05 13:09             ` Marc Zyngier
2019-09-05 13:09             ` Marc Zyngier
2019-09-06  8:00             ` Christoffer Dall
2019-09-06  8:00               ` Christoffer Dall
2019-09-06  8:00               ` Christoffer Dall
2019-09-06 12:08               ` Alexander Graf
2019-09-06 12:08                 ` Alexander Graf
2019-09-06 12:08                 ` Alexander Graf
2019-09-06 12:34                 ` Marc Zyngier
2019-09-06 12:34                   ` Marc Zyngier
2019-09-06 12:34                   ` Marc Zyngier
2019-09-06 13:02                   ` [UNVERIFIED SENDER] " Alexander Graf
2019-09-06 13:02                     ` Alexander Graf
2019-09-06 13:02                     ` Alexander Graf
2019-09-06 13:12                 ` Christoffer Dall
2019-09-06 13:12                   ` Christoffer Dall
2019-09-06 13:12                   ` Christoffer Dall
2019-09-06 13:16                   ` Alexander Graf
2019-09-06 13:16                     ` Alexander Graf
2019-09-06 13:16                     ` Alexander Graf
2019-09-06 13:31                   ` Peter Maydell
2019-09-06 13:31                     ` Peter Maydell
2019-09-06 13:31                     ` Peter Maydell
2019-09-06 13:41                     ` Alexander Graf
2019-09-06 13:41                       ` Alexander Graf
2019-09-06 13:41                       ` Alexander Graf
2019-09-06 13:50                       ` Peter Maydell
2019-09-06 13:50                         ` Peter Maydell
2019-09-06 13:50                         ` Peter Maydell
2019-09-06 14:12                         ` Alexander Graf
2019-09-06 14:12                           ` Alexander Graf
2019-09-06 14:12                           ` Alexander Graf
2019-09-06 13:44                     ` Christoffer Dall
2019-09-06 13:44                       ` Christoffer Dall
2019-09-06 13:44                       ` Christoffer Dall
2019-09-05 13:25           ` Heinrich Schuchardt
2019-09-05 13:25             ` Heinrich Schuchardt
2019-09-05 13:25             ` Heinrich Schuchardt
2019-09-06  7:58             ` Christoffer Dall
2019-09-06  7:58               ` Christoffer Dall
2019-09-06  7:58               ` Christoffer Dall
2019-09-05  8:28   ` Heinrich Schuchardt
2019-09-05  8:28     ` Heinrich Schuchardt
2019-09-05  8:28     ` Heinrich Schuchardt
2019-09-05  9:11     ` Marc Zyngier
2019-09-05  9:11       ` Marc Zyngier
2019-09-05  9:11       ` Marc Zyngier
2019-09-05  9:20 ` Stefan Hajnoczi
2019-09-05  9:20   ` Stefan Hajnoczi
2019-09-05  9:20   ` Stefan Hajnoczi
2019-09-05  9:23   ` Daniel P. Berrangé
2019-09-05  9:23     ` Daniel P. Berrangé
2019-09-05  9:23     ` Daniel P. Berrangé
2019-09-05 12:01   ` Heinrich Schuchardt
2019-09-05 12:01     ` Heinrich Schuchardt
2019-09-05 12:01     ` Heinrich Schuchardt
2019-09-05 12:16     ` Christoffer Dall
2019-09-05 12:16       ` Christoffer Dall
2019-09-05 12:16       ` Christoffer Dall

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=86k1anrtmi.wl-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=berrange@redhat.com \
    --cc=james.morse@arm.com \
    --cc=julien.thierry@arm.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.maydell@linaro.org \
    --cc=stefanha@redhat.com \
    --cc=suzuki.poulose@arm.com \
    --cc=xypron.glpk@gmx.de \
    /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.