Re: [Qemu-devel] [PATCH] kvm: Move kvm_allows_irq0_override() to target-i386

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1846 bytes --]

On 2012-07-20 21:14, Peter Maydell wrote:
> kvm_allows_irq0_override() is a totally x86 specific concept:
> move it to the target-specific source file where it belongs.
> This means we need a new header file for the prototype:
> kvm_i386.h, in line with the existing kvm_ppc.h.

First of all, the patch is inconsistent as it lacks something like
target-i386/kvm-stub.c (IOW, you forgot about kvm_allows_irq0_override
in kvm-stub.c). But the approach is wrong in general, see below.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I'm sure this isn't the only x86ism in the KVM generic source
> files. However the thing I'm specifically trying to do is
> nuke all the uses of kvm_irqchip_in_kernel() in common code,

No, "irqchip in kernel" is supposed to be a generic concept. We will
also have it on Power. Not sure what your plans are for ARM, maybe it
will always be true there.

That said, maybe there is room for discussion about what it means for
the general KVM code and its users if the irqchip is in the kernel. Two
things that should be common for every arch:
 - VCPU idle management is done inside the kernel
 - in-kernel KVM helpers like vhost or VFIO can inject IRQs directly

The latter point implies that irqfd is available and that interrupt
routes from virtual IRQs (*) (like the one associated with an irqfd) to
the in-kernel IRQ controller have to be established. That's pretty generic.

> as part of trying to untangle "what irqchip model do you want"
> from other aspects of the interrupt handling model.
> 
> Other than this one, there are uses in cpus.c and hw/virtio-pci.c,
> but I haven't figured out yet exactly what those bits of code
> are trying to do...

See above for the two reasons.

Jan

(*) "GSI" actually means "physical or virtual IRQ line".


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]