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

[Peter Maydell]

On 23 July 2012 13:26, Avi Kivity <address@hidden> 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