On 2012-07-23 17:19, Peter Maydell wrote: > On 23 July 2012 13:26, Avi Kivity <avi@xxxxxxxxxx> 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 You should reject kernel_irqchip=off as long as you only support an in-kernel GIC model. > 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 GSI, not MSI. > set true only if x86 and kvm_irqchip_in_kernel It means that the target architecture supports routing of various interrupt sources (userspace, irqfds, in-kernel device models) to different in-kernel IRQ sinks (CPU cores, irqchip models, whatever). Interrupt messages via (binary-state) irqfd depend on it. > > kvm_has_irqfds() > -- true if kernel supports IRQFDs > currently true only if x86 and kvm_irqchip_in_kernel Note that this and the above are currently static feature tests, not mode checks (i.e. they are true even if kernel_irqchip=off). The "kvm_has" namespace is reserved for such tests. > > > 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() The name kvm_irqchip_set_irq implies so far that it injects into an in-kernel irqchip model. Either different functions for archs that don't follow this concept need to be provided, or this function requires renaming (kvm_set_irq_async or so). > > kvm-all.c:kvm_irqchip_add_msi_route(): > -- should be kvm_have_msi_routing() Only if you change the semantics of kvm_has_gsi_routing (and rename it). > > kvm-all.c:kvm_irqchip_assign_irqfd(): > -- should be true if kvm_has_irqfds() The same issue. Plus there is that naming conflict again if we should ever see irqfd without some in-kernel irqchip. But even s390 would have a logical "irqchip" for me at the point it may route interrupt messages from devices directly to the CPUs. > > 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(). It depends on full irqfd support, which includes IRQ routing to allow MSI via irqfd. Something like kvm_msi_via_irqfd_available. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
