On 23 July 2012 13:26, Avi Kivity <a...@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]
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
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
Second, current uses of kvm_irqchip_in_kernel():
hw/kvmvapic.c, hw/pc.c, hw/pc_piix.c, target-i386/kvm.c:
-- these are all x86 specific and can continue to use
kvm_irqchip_in_kernel()
cpus.c:cpu_thread_is_idle()
-- should use !kvm_interrupts_are_async() [because halt is
in userspace iff we're using the synchronous interrupt model]
kvm-all.c:kvm_irqchip_set_irq():
-- (just an assert) should be kvm_interrupts_are_async()
kvm-all.c:kvm_irqchip_add_msi_route():
-- should be kvm_have_msi_routing()
kvm-all.c:kvm_irqchip_assign_irqfd():
-- should be true if kvm_has_irqfds()
kvm-all.c:kvm_allows_irq0_override():
-- this still seems to me to be a completely x86 specific concept;
it should move to a source file in target-x86 and then it
can continue to use kvm_irqchip_in_kernel()
hw/virtio-pci.c:virtio_pci_set_guest_notifiers()
-- not entirely sure about this one but I think it
should be testing kvm_has_msi_routing().
Any mistakes/countersuggestions?
thanks
-- PMM
