Re: [PATCH 1/1] KVM: inject data abort if instruction cannot be decoded

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 09:52:36 +0100	[thread overview]
Message-ID: <86mufjrup7.wl-maz@kernel.org> (raw)
In-Reply-To: <CAFEAcA-mc6cLmRGdGNOBR0PC1f_VBjvTdAL6xYtKjApx3NoPgQ@mail.gmail.com>

On Thu, 05 Sep 2019 09:16:54 +0100,
Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On Thu, 5 Sep 2019 at 09:04, Marc Zyngier <maz@kernel.org> wrote:
> > How can you tell that the access would fault? You have no idea at that
> > stage (the kernel doesn't know about the MMIO ranges that userspace
> > handles). All you know is that you're faced with a memory access that
> > you cannot emulate in the kernel. Injecting a data abort at that stage
> > is not something that the architecture allows.
> 
> To be fair, locking up the whole CPU (which is effectively
> what the kvm_err/ENOSYS is going to do to the VM) isn't
> something the architecture allows either :-)

Hey, I didn't say things were good as they are now! ;-)

I'm definitely willing to change things in that area, but I also don't
want anyone to start relying on things that are not specified anywhere.

> 
> > Of course, the best thing would be to actually fix the guest so that
> > it doesn't use non-emulatable MMIO accesses. In general, that the sign
> > of a bug in low-level accessors.
> 
> 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).

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.

Thanks,

	M.

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