Thread (80 messages) 80 messages, 8 authors, 2013-01-17

[kvmarm] [PATCH v5 13/14] KVM: ARM: Handle I/O aborts

From: Christoffer Dall <hidden>
Date: 2013-01-14 22:51:08
Also in: kvm

On Mon, Jan 14, 2013 at 5:36 PM, Will Deacon [off-list ref] wrote:
On Mon, Jan 14, 2013 at 07:12:49PM +0000, Christoffer Dall wrote:
quoted
On Mon, Jan 14, 2013 at 2:00 PM, Will Deacon [off-list ref] wrote:
quoted
On Mon, Jan 14, 2013 at 06:53:14PM +0000, Alexander Graf wrote:
quoted
On 01/14/2013 07:50 PM, Will Deacon wrote:
quoted
FWIW, KVM only needs this code for handling complex MMIO instructions, which
aren't even generated by recent guest kernels. I'm inclined to suggest removing
this emulation code from KVM entirely given that it's likely to bitrot as
it is executed less and less often.
That'd mean that you heavily limit what type of guests you're executing,
which I don't think is a good idea.
To be honest, I don't think we know whether that's true or not. How many
guests out there do writeback accesses to MMIO devices? Even on older
Linux guests, it was dependent on how GCC felt.
I don't think bitrot'ing is a valid argument: the code doesn't depend
on any other implementation state that's likely to change and break
this code (the instruction encoding is not exactly going to change).
And we should simply finish the selftest code to test this stuff
(which should be finished if the code is unified or not, and is on my
todo list).
Maybe `bitrot' is the wrong word. The scenario I envisage is the addition
of new instructions to the architecture which aren't handled by the current
code, then we end up with emulation code that works for some percentage of
the instruction set only. If the code is rarely used, it will likely go
untouched until it crashes somebody's VM.
How is that worse than KVM crashing all VMs that use any of these
instructions for IO?

At least the code we have now has been tested with a number of old
kernels, and we know that it works. As for correctness, it will be the
case for all implementations and this type of code absolutely requires
a test suite.

quoted
quoted
I see where you're coming from, I just don't think we can quantify it either
way outside of Linux.
FWIW, I know of at least a couple of companies wanting to use KVM for
running non-Linux guests as well.
Oh, I don't doubt that. The point is, do we have any idea how they behave
under KVM? Do they generate complex MMIO accesses? Do they expect firmware
shims, possibly sitting above hyp? Do they require a signed boot sequence?
Do they run on Cortex-A15 (the only target CPU we have at the moment)?
No we don't know. But there's a fair chance that they do use complex
mmio instructions seeing as older kernels did, without anything
explicitly being involved.
quoted
But, however a shame, I can more easily maintain this single patch
out-of-tree, so I'm willing to drop this logic for now if it gets
things moving.
I would hope that, if this code is actually required, you would consider
merging it with what we have rather than maintaining it out-of-tree.
Of course I would, and I would also make an effort to unify the code
if it were merged now, I just don't have the cycles to do the unify
work right now, since it is without doubt a lengthy process.

So from that point of view, I don't quite see how it's better to leave
the code out at this point, but that is not up to me.

-Christoffer
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help