Re: [PATCH 3/9] KVM: PPC: Book3S: Add kernel emulation for the XICS interrupt controller

[Scott Wood <scottwood@freescale.com>]

On 02/20/2013 01:58:54 PM, Marcelo Tosatti wrote:
> This is probably a stupid question, but why the
> KVM_SET_IRQCHIP/KVM_SET_GSI_ROUTING interface is not appropriate for
> your purposes?
> 
> x86 sets up a default GSI->IRQCHIP PIN mapping on creation (during
> KVM_SET_IRQCHIP), but it can be modified with KVM_SET_GSI_ROUTING.

To start, the whole IRQ routing stuff is poorly documented.

Am I supposed to make up GSI numbers and use the routing thing to  
associate them with real interrupts?  Are there constraints on what  
sort of GSI numbers I can choose (I now see in the code that  
KVM_MAX_IRQ_ROUTES is returned from the capability check, but where is  
that documented?  It looks like the APIC implementation has default  
routes, where is that documented?)?  Where does the code live to manage  
this table, and how APICy is it (looks like the answer is "irq_comm.c,  
and very")?  I suppose I could write another implementation of the  
table management code for MPIC, though the placement of "irqchip"  
inside the route entry, rather than as an argument to KVM_IRQ_LINE,  
suggests the table is supposed to be global, not in the individual  
interrupt controller.

It looks like I'm going to have to do this anyway for irqfd, though  
that doesn't make the other uses of the device control api go away.   
Even KVM_DEV_MPIC_GRP_IRQ_ACTIVE would still be useful for reading the  
status for debugging (reading device registers doesn't quite do it,  
since the "active" bit won't show up if the interrupt is masked).  At  
that point, is it more offensive to make it read-only even though it  
would be trivial to make it read/write (which would allow users who  
don't need it to bypass the routing API), or to make it read/write and  
live with there being more than one way to do something?

KVM_SET_IRQCHIP is not suitable because we have more than 512 bytes of  
state, and because it doesn't allow debugging access to device  
registers (e.g. inspecting from the QEMU command line), and because  
it's hard to add new pieces of state if we realize we left something  
out.  It reminds be of GET/SET_SREGS.  With that, I did what you seem  
to want here, which is to adapt the existing interfaces, using feature  
flags to control optional state.  It ended up being a mess, and ONE_REG  
was introduced as a replacement.  The device control API is the  
equivalent of ONE_REG for things other than vcpus.

-Scott