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

[Avi Kivity <avi@redhat.com>]

On 07/23/2012 06:19 PM, Peter Maydell wrote:
> On 23 July 2012 13:26, Avi Kivity <avi@redhat.com> wrote:
>> On 07/21/2012 11:54 AM, Peter Maydell wrote:
>>> 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.
> 
> OK, let's see if we can get some agreement about naming here.
> 
> First, some test-functions I think we definitely need:
> 
>  kvm_interrupts_are_async()
>    -- true if interrupt delivery is asynchronous
>       default false in kvm_init, set true in kvm_irqchip_create,
>       architectures may set it true in kvm_arch_init [ARM will
>       do so; PPC might want to do so]

Interrupts are by nature async.  I'd say kvm_async_interrupt_injection()
to make it clearer.

> 
>  kvm_irqchip_in_kernel()
>    -- the user-settable option, actual behaviour is arch specific
>       on x86, true means (as it does now) LAPIC,IOAPIC,PIT in kernel
>       on ARM, we ignore this setting and just DTRT
>       on PPC, used as a convenience setting for whether to use
>       an in-kernel model of the interrupt controller
>       Shouldn't be used in non-target-specific code

If it's 100% arch specific, the name can/should be arch specific since
it will never be used in generic core.  So kvm_ioapic_in_kernel(),
kvm_gic_in_kernel() (or even kvm_ioapic(), kvm_gic(), since "kvm"
already implies the kernel (that's the k in kvm, after all).

> 
> and two I'm not quite so sure about:
> 
>  kvm_has_msi_routing()
>    -- true if we can do routing of MSIs
>       set true only if x86 and kvm_irqchip_in_kernel
> 
>  kvm_has_irqfds()
>    -- true if kernel supports IRQFDs
>       currently true only if x86 and kvm_irqchip_in_kernel

Will be true for everyone, eventually.


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