On Mon, Jan 14, 2013 at 5:36 PM, Will Deacon <will.deacon@xxxxxxx> wrote: > On Mon, Jan 14, 2013 at 07:12:49PM +0000, Christoffer Dall wrote: >> On Mon, Jan 14, 2013 at 2:00 PM, Will Deacon <will.deacon@xxxxxxx> wrote: >> > On Mon, Jan 14, 2013 at 06:53:14PM +0000, Alexander Graf wrote: >> >> On 01/14/2013 07:50 PM, Will Deacon wrote: >> >> > 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. >> > 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. >> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
