From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 3/9] KVM: PPC: Book3S: Add kernel emulation for the XICS interrupt controller Date: Wed, 20 Feb 2013 19:45:15 -0600 Message-ID: <1361411115.31212.17@snotra> References: <20130221010955.GA30853@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Paul Mackerras , Alexander Graf , , To: Marcelo Tosatti Return-path: In-Reply-To: <20130221010955.GA30853@amt.cnet> (from mtosatti@redhat.com on Wed Feb 20 19:09:55 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 02/20/2013 07:09:55 PM, Marcelo Tosatti wrote: > On Wed, Feb 20, 2013 at 06:20:51PM -0600, Scott Wood wrote: > > 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? > > I have no idea. Is mapping from one integer linear space (GSIs) > to real interrupts suitable for you? I can live with it. > > Where does the code live to manage this table, and how APICy is it > (looks like the > > answer is "irq_comm.c, and very")? > > Thinking faster than typing? Not sure what you mean. Sorry... The code to manage the table lives in virt/kvm/irq_comm.c. That code is very APIC-specific and not currently in a state that invites sharing. > > 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? > > Can't follow this sentence. Suppose, for the sake of irqfd (and maillist tranquility) MPIC uses existing KVM_IRQ_LINE and such. Will there be objection to being able to use KVM_GET_DEVICE_ATTR to *get* the irq line status for debugging purposes (maybe also useful for migration)? If there's no objection to that, would there be any actual reason, beyond saving a few lines of glue code, to make it a read-only attribute? > > 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 > > - ACK on 512 bytes not sufficient. Add another ioctl, SET_IRQCHIP2? Well, that's what KVM_SET_DEVICE_ATTR is. > - Agree on poor extensibility of interface. Adding a reserved amount > of space as padding and versioning such as has been done so far > is not acceptable? That's exactly what we did with SREGS, and it got declared a mess and replaced with ONE_REG. I'm trying to learn from my mistakes. :-) > - Debugging: why is reading entire register state not acceptable? Yes, > its slow. For one, it's more work. The current way works by simulating a guest MMIO access. No blob format to design, implement, and keep compatible. -Scott