Re: [RFC PATCH 01/19] cpus: Add argument to qemu_get_cpu() to filter CPUs by QOM type

[David Woodhouse]

On Fri, 2023-10-20 at 18:36 +0200, Philippe Mathieu-Daudé wrote:
> Heterogeneous machines have different type of CPU.
> qemu_get_cpu() returning unfiltered CPUs doesn't make
> sense anymore. Add a 'type' argument to filter CPU by
> QOM type.
> 
> Type in "hw/core/cpu.h" and implementation in cpu-common.c
> modified manually, then convert all call sites by passing
> a NULL argument using the following coccinelle script:
> 
>   @@
>   expression index;
>   @@
>   -   qemu_get_cpu(index)
>   +   qemu_get_cpu(index, NULL)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> RFC: Is this hot path code? What is the cost of this QOM cast check?

A bunch of them in the Xen emulation code are during interrupt
delivery. So yes, kind of hot path :)

In hw/i386/kvm/xen_evtchn.c I they're almost all just looking up a CPU
with qemu_get_cpu() so that they can call kvm_arch_vcpu_id() to get its
APIC ID. Mostly to send an MSI there or otherwise talk to the kernel
about it.

Perhaps it could keep a simple array of { vCPU ID, APIC ID } instead.

There's one case in valid_vcpu() where it's checking that a given vCPU
does exist, which could use that same array.

In target/i386/kvm/xen-emu.c there are some slow paths which are less
interesting (xen_set_shared_info(), kvm_xen_has_vcpu_callback_vector(),
kvm_xen_set_vcpu_virq(), kvm_xen_hcall_evtchn_upcall_vector()

kvm_xen_hcall_vcpu_op() does have a fast path (the singleshot timer
handling, although most guests seem to use a different hypercall for
that). But it doesn't actually call qemu_get_cpu() for that fast path
anyway, since it'll use the CPU it was *called* on.

kvm_xen_get_vcpu_info_hva() wants a pointer stored in cpu->env, and is
called in *one* fast path in xen_evtchn.c, again interrupt delivery (in
set_port_pending). But only in the case where we're running on an old
kernel and don't just ask the kernel to deliver the event. So I can
live with not optimising that.

kvm_xen_set_callback_asserted() is only for the case where event
channels are being delivered to the guest via emulated GSI, so not the
high-performance case. And I still do want to hook that up properly to
the ack on the *interrupt controllers* anyway (qv¹).

Finally, kvm_xen_injecft_vcpu_callback_vector() is also only for older
kernels where we don't ask the kernel to deliver the event. 

tl;dr: we don't care about anything in xen.emu.c. So I think we could
live with just a fast lookup from vCPU# → APIC ID for the cases in
xen_evtchn.c.



¹ 
https://lore.kernel.org/all/70eb35a08a7c48993812b7f088fa9ae3f2c8b925.camel@infradead.org/T/

smime.p7s
Description: S/MIME cryptographic signature