On Mon, Oct 22, 2012 at 1:45 PM, Will Deacon <will.dea...@arm.com> wrote: > On Mon, Oct 22, 2012 at 04:09:06AM +0100, Rusty Russell wrote: >> Christoffer Dall <c.d...@virtualopensystems.com> writes: >> > On Fri, Oct 19, 2012 at 2:19 AM, Rusty Russell <ru...@rustcorp.com.au> >> > wrote: >> >> Wait, what? kvm/arm isn't in kvm-next? >> >> Christoffer, is there anything I can help with? >> > >> > Specifically there are worries about the instruction decoding for the >> > mmio instructions. My cycles are unfortunately too limited to change >> > this right now and I'm also not sure I agree things will turn out >> > nicer by unifying all decoding into a large complicated space ship, >> > but it would be great if you could take a look. This discussion seems >> > to be a good place to start: >> > >> > https://lists.cs.columbia.edu/pipermail/kvmarm/2012-September/003447.html >> >> They're still asking you to boil that ocean?? >> >> I could create a struct and do simple decode into it for limited cases >> (ie. for kvm). Will, do you want to see that? > > Yes, I think that would be great! Basically, I'd like the code to be > reusable so that other subsystems (including uprobes as of last week) can > plug into if it possible. The actual plugging in of those subsystems is > obviously not up to you. > > Dave posted an idea here: > > > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-October/123464.html > > which could form a basic starting block for something like load/store > decoding. It looks like Christoffer has actually done a bunch of this for > v3. >
The issue is that the decoding assumes that you're only going to decode instructions that don't carry decode information in the HSR. For example, we don't do anything special about unprivileged load/store, because they would have failed in their stage 1 translation and we wouldn't even get to the decoding in KVM. There are also a number of corner cases such as loading into the PC from an MMIO address that we don't need to worry about as we simply dismiss it as being insane guest behavior. Another example is all the checking of the write-back cases, since we can simply assume that there will be a write-back in case we're decoding anything. We also don't decode load/store multiples (although Antonios Motakis did work up a patch for this some time ago for the ARM mode), since the user space ABI to communicate the MMIO operations don't support multiple registers and reworking that on the architecture-generic level for some theoretical non-existing guest is simply not something worth the time. The point is that we cannot just take the code that is there now and make it available generically within arch/arm without a lot of work, including coming up with a test framework and verify it, and as Rusty says, even then we don't know if the whole thing will look so complicated that we deem it not worth the effort end. And, to repeat my favorite argument, this can always be changed after merging KVM. Alternatively, we can remove the mmio encoding completely from the patch series and rely on your patch for mmio accessors in any guest kernel, but I think this is a shame for people wanting to try slightly more exotic things like an older kernel, when we have code that's working. Please, please, consider signing off on the current stages of the patches if nothing else ground breaking pops up. >> But unifying them all is a much larger task, and only when that's all >> done can you judge whether it was worthwhile. I've spend half an hour >> looking and each case is subtly different, and the conversion has to be >> incredibly careful not to break them. And converting opcodes.c is just >> ideology; it's great as it is. > > opcodes.c may be where this stuff ultimately ends up. In the meantime, you > could try moving what you have for v3 into common ARM code so that other > people can try to use it. In fact, if you don't even want to do that, just > put it in its own file under arch/arm/kvm/ so that the interface to > emulate.c doesn't make it too hard to move around in future. > >> I'm interested in the 64-bit ARM kvm, because it would be nice to unify >> the two implementations. But the ABI will be different anyway (64 bit >> regs get their own id space even if nothing else changes). > > Assumedly you'll need a wrapper around the 32-bit ABI in order to run 32-bit > guests under a 64-bit kernel, so unification is definitely a good idea. > > Will -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
