Re: [PATCH] target/arm: do not use cc->do_interrupt for KVM directly

[Claudio Fontana]

On 12/7/20 9:56 PM, Peter Maydell wrote:
> On Mon, 7 Dec 2020 at 18:28, Eduardo Habkost <ehabkost@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)
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