On Mon, Dec 07, 2020 at 10:06:55PM +0100, Claudio Fontana wrote: > On 12/7/20 9:56 PM, Peter Maydell wrote: > > On Mon, 7 Dec 2020 at 18:28, Eduardo Habkost <ehabk...@redhat.com> wrote: > >> All signs seem to indicate that CPUClass.do_interrupt is > >> TCG-specific, except for those two lines of code in > >> target/arm/kvm64.c. The point of this patch would be to allow us > >> to separate TCG-specific code from accel-independent code later. > > > > So it's TCG-specific except that we call it from KVM. > > That doesn't sound very TCG-specific :-) > > > >> Maybe those signs are misleading us, and CPUClass.do_interrupt > >> shouldn't be TCG-specific. If that's the case, why arm is the > >> only architecture that uses CPUClass.do_interrupt outside > >> TCG-specific code? > > > > So, the purpose of the do_interrupt method is "set the guest > > CPU state up in the way that the architecture specifies > > happens when an interrupt is taken" (set the program counter, > > set things like the syndrome register that says what type > > of exception happens, etc, etc). For TCG we obviously need > > to do this for every interrupt/exception that happens. > > For KVM, in most cases the kernel is responsible for > > delivering an exception to the guest, because it's the > > kernel that determines that it needs to do this. > > The two oddball cases[*] in target/arm are for situations > > where it is userspace code that has identified that it > > needs to deliver an exception to the guest. The kernel's > > KVM API doesn't provide a "please deliver an exception to > > the guest" function, on the grounds that userspace could > > do the work itself. So we need to do that work (setting the > > PC, setting syndrome register, etc, etc). In theory we > > could have a special version of the function for KVM > > CPUs only, but since in fact the same code works just > > fine in KVM and TCG we reuse it. > > > > I know that the macOS Hypervisor.Framework APIs are rather > > lower-level than KVM (they do less work in the kernel and > > push more of it onto userspace); it's possible that there > > we might find more situations where userspace needs to do > > "make the guest CPU take an exception"; I haven't investigated. > > > > [*] The two special cases are: > > (1) the vcpu thread got a SIGBUS indicating a memory error, > > and we need to deliver a synchronous external abort > > exception to the guest to let it know about the error > > (2) the kernel told us about a debug exception (breakpoint, > > watchpoint, etc) but it turns out not to be for one of > > QEMU's own gdbstub breakpoints/watchpoints, so it > > must be one the guest itself has set up, and so we need > > to deliver it to the guest > > > > These are fairly obscure, and it wouldn't surprise me if > > other target archs had picked a different separation of > > concerns between userspace and the kernel such that userspace > > didn't need to manually deliver an exception. > > > > thanks > > -- PMM > > > > Hello Peter, > > thank you for the explanation, interesting read. > > As I understand it, for the purpose of code separation, > we could: > > 1) skip do_interrupt move to the separate tcg_ops structure, > wait until KVM/ARM uses another approach (if ever)
My understanding is that there's no reason for ARM KVM to use another approach, and that CPUClass.do_interrupt is not really TCG-specific. Do we have any case where the CPUClass.do_interrupt implementation is really TCG-specific, or it is just a coincidence that most other accelerators simply don't to call the method? It looks like the only cases where the CPUClass.do_interrupt assignment is conditional on CONFIG_TCG are i386 and s390x. > 2) do the move, and just call arm_cpu_do_interrupt() directly, > since for KVM64 it is the only one that can be assigned to > cc->do_interrupt(). > > Which way would you suggest? > > Thanks, > > Claudio > > > > > -- Eduardo
