Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
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. 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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
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? 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. 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). Let's get the current code shipped please, it's only going to bitrot outside the tree. Cheers, Rusty. -- 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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
Rusty Russell ru...@rustcorp.com.au writes: Avi Kivity a...@redhat.com writes: On 09/05/2012 10:58 AM, Rusty Russell wrote: This is the generic part of the KVM_SET_ONE_REG/KVM_GET_ONE_REG enhancements which ARM wants, rebased onto kvm/next. This was stalled for so long it needs rebasing again, sorry. But otherwise I'm happy to apply. Ok, will rebase and re-test against kvm-next. Wait, what? kvm/arm isn't in kvm-next? This will produce a needless clash with that, which is more important than this cleanup. I'll rebase this as soon as that is merged. Christoffer, is there anything I can help with? Cheers, Rusty. -- 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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
On Fri, Oct 19, 2012 at 2:19 AM, Rusty Russell ru...@rustcorp.com.au wrote: Rusty Russell ru...@rustcorp.com.au writes: Avi Kivity a...@redhat.com writes: On 09/05/2012 10:58 AM, Rusty Russell wrote: This is the generic part of the KVM_SET_ONE_REG/KVM_GET_ONE_REG enhancements which ARM wants, rebased onto kvm/next. This was stalled for so long it needs rebasing again, sorry. But otherwise I'm happy to apply. Ok, will rebase and re-test against kvm-next. Wait, what? kvm/arm isn't in kvm-next? This will produce a needless clash with that, which is more important than this cleanup. I'll rebase this as soon as that is merged. Christoffer, is there anything I can help with? There are some worries about duplicating functionality on the ARM side of things. 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 Thanks! -Christoffer -- 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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
On 09/05/2012 10:58 AM, Rusty Russell wrote: This is the generic part of the KVM_SET_ONE_REG/KVM_GET_ONE_REG enhancements which ARM wants, rebased onto kvm/next. This was stalled for so long it needs rebasing again, sorry. But otherwise I'm happy to apply. -- error compiling committee.c: too many arguments to function -- 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
Re: [PATCH 0/3] KVM_VCPU_GET_REG_LIST API
Avi Kivity a...@redhat.com writes: On 09/05/2012 10:58 AM, Rusty Russell wrote: This is the generic part of the KVM_SET_ONE_REG/KVM_GET_ONE_REG enhancements which ARM wants, rebased onto kvm/next. This was stalled for so long it needs rebasing again, sorry. But otherwise I'm happy to apply. Ok, will rebase and re-test against kvm-next. Cheers, Rusty. -- 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
[PATCH 0/3] KVM_VCPU_GET_REG_LIST API
This is the generic part of the KVM_SET_ONE_REG/KVM_GET_ONE_REG enhancements which ARM wants, rebased onto kvm/next. Rusty Russell (3): KVM: Move KVM_SET_ONE_REG/KVM_GET_ONE_REG to generic code. KVM: Add KVM_REG_SIZE() helper. KVM: Add KVM_VCPU_GET_REG_LIST/KVM_CAP_REG_LIST. Documentation/virtual/kvm/api.txt | 20 ++ arch/powerpc/include/asm/kvm_host.h |1 + arch/powerpc/kvm/book3s_hv.c|4 ++-- arch/powerpc/kvm/book3s_pr.c|4 ++-- arch/powerpc/kvm/booke.c|4 ++-- arch/powerpc/kvm/powerpc.c | 15 -- arch/s390/include/asm/kvm_host.h|1 + arch/s390/kvm/kvm-s390.c| 19 ++ include/linux/kvm.h | 14 + include/linux/kvm_host.h|9 - virt/kvm/kvm_main.c | 38 +++ 11 files changed, 90 insertions(+), 39 deletions(-) -- 1.7.9.5 -- 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
