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

[Avi Kivity]

On 07/21/2012 11:54 AM, Peter Maydell wrote:
> On 21 July 2012 07:57, Jan Kiszka <address@hidden> 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.
> 
>> 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.

Really, "irqchip in kernel" means asynchronous interrupts - you can
inject an interrupt from outside the vcpu thread.  Obviously if the vcpu
is sleeping you need to wake it up and that pulls in idle management.

"irqchip" for x86 really means the local APIC, which has a synchronous
interface with the cpu core.  "local APIC in kernel" really is
equivalent to "kernel idle management", "KVM_IRQ_LINE", and irqfd.  The
ioapic and pit, on the other hand, don't contribute anything to this
(just performance).

So yes, ARM with and without GIC are irqchip_in_kernel, since the
ARM<->GIC interface is asynchronous.  Whether to emulate the GIC or not
is just a performance question.

> 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 :-))

Agree naming should be improved.  In fact the early series I pushed to
decompose local apic, ioapic, and pic, but that didn't happen.  If it
did we'd probably not have this conversation.


-- 
error compiling committee.c: too many arguments to function