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: 3895 bytes --]

On 2012-07-21 10:54, Peter Maydell wrote:
> On 21 July 2012 07:57, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2012-07-20 21:14, Peter Maydell wrote:
>>> 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.
> 
> I agree that "irqchip in kernel?" is generic (though as you'll see
> below there's disagreement about what that ought to mean or imply).
> "irq0_override" though seems to me to be absolutely x86 specific.

Naming is x86 specific, semantic not. It means that KVM doesn't prevent
remapping of IRQs. Granted, I really hope you don't make such mistakes
in your arch.

> 
>> 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
> 
> The trouble is that at the moment QEMU assumes that "is the
> irqchip in kernel?" == "is VCPU idle management in kernel", for
> instance. For ARM, VCPU idle management is in kernel whether
> we're using the kernel's model of the VGIC or not. Alex tells
> me PPC is the same way. It's only x86 that has tied these two
> concepts together.

Hmm, and why does Power work despite this mismatch?

If cpu_thread_is_idle doesn't work for you, define something like
kvm_idle_in_kernel() to replace kvm_irqchip_in_kernel here.

> 
> The reason I want to get rid of common-code uses of kvm_irqchip_in_kernel()
> is because I think they're all similar to this -- the common code is
> using the check as a proxy for something else, and it should be directly
> asking about that something else. The only bits of code that should
> care about "is the irqchip in kernel?" are:
>  * target-specific device/machine setup code which needs to know
>    which apic/etc to instantiate
>  * target-specific x86 code which has this weird synchronous IRQ
>    delivery model for irqchip-not-in-kernel
> (Obviously I might have missed something, I'm flailing around
> trying to understand this code :-))
> 
>>  - 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.
> 
> But you can perfectly well have an in-kernel-irqchip that doesn't
> support irqfd 

You could, thought this doesn't make much sense.

> -- it just means that interrupts from devices have
> to come in via the ioctls same as anything else. Some in-kernel
> helpers obviously would depend on the existence and use of a full
> featured in-kernel irqchip (on ARM you don't get the in kernel timer
> unless you have in kernel VGIC), but I don't see why the virtio code
> should be asking "is there an in kernel irqchip?" rather than "can
> I do irqfd routing?" or whatever the question is it actually wants
> to ask.  (In fact the virtio code probably needs to do something
> more complex anyway: you could perfectly well have a system where
> there is a full-featured irqchip in the kernel but the virtio
> device is on the "wrong" side of a second interrupt controller
> which is not in-kernel. So the actual question it needs to ask
> is "does the interrupt wiring in this specific machine model mean
> I can get and use an irqfd from where I am to the main CPU
> interrupt controller?" or something similar.)

Well, same here: then define more precise generic test functions.

Jan


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