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

[Marcelo Tosatti <mtosatti@redhat.com>]

On Sat, Feb 16, 2013 at 01:56:14PM +1100, Paul Mackerras wrote:
> On Fri, Feb 15, 2013 at 05:59:11PM -0600, Scott Wood wrote:
> > On 02/15/2013 05:18:31 PM, Paul Mackerras wrote:
> > >On Fri, Feb 15, 2013 at 02:05:41PM -0600, Scott Wood wrote:
> > >> On 02/14/2013 06:01:08 PM, Paul Mackerras wrote:
> > >> >From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > >> >
> > >> >This adds in-kernel emulation of the XICS (eXternal Interrupt
> > >> >Controller Specification) interrupt controller specified by
> > >PAPR, for
> > >> >both HV and PR KVM guests.
> > >> >
> > >> >This adds a new KVM_CREATE_IRQCHIP_ARGS ioctl, which is like
> > >> >KVM_CREATE_IRQCHIP in that it indicates that the virtual machine
> > >> >should use in-kernel interrupt controller emulation, but also
> > >takes an
> > >> >argument struct that contains the type of interrupt controller
> > >> >architecture and an optional parameter.  Currently only one
> > >type value
> > >> >is defined, that which indicates the XICS architecture.
> > >>
> > >> Would the device config API I posted a couple days ago work for you?
> > >
> > >I suppose it could be made to work.  It doesn't feel like a natural
> > >fit though, because your API seems to assume (AFAICT) that a device is
> > >manipulated via registers at specific physical addresses, so I would
> > >have to invent an artificial set of registers with addresses and bit
> > >layouts, that aren't otherwise required.  The XICS is operated from
> > >the guest side via hcalls, not via emulated MMIO.
> > 
> > I don't think it makes such an assumption.  The MPIC device has
> > physical registers, so it exposes them, but it also exposes things
> > that are not physical registers (e.g. the per-IRQ input state).  The
> > generic device control layer leaves interpretation of attributes up
> > to the device.
> > 
> > I think it would be easier to fit XICS into the device control api
> > model than to fit MPIC into this model, not to mention what would
> > happen if we later want to emulate some other type of device -- x86
> > already has at least one non-irqchip emulated device (i8254).
> 
> I have no particular objection to the device control API per se, but
> I have two objections to using it as the primary interface to the XICS
> emulation.
> 
> First, I dislike the magical side-effect where creating a device of a
> particular type (e.g. MPIC or XICS) automatically attaches it to the
> interrupt lines of the vcpus.  I prefer an explicit request to do
> in-kernel interrupt control. 

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.

>  Further, the magic means that you can
> only have one instance of the device, whereas you might want to model
> the interrupt controller architecture as several devices.  You could
> do that using several device types, but then the interconnections
> between them would also be magic.
> 
> Secondly, it means that we are completely abandoning any attempt to
> define an abstract or generic interface to in-kernel interrupt
> controller emulations.  Each device will have its own unique set of
> attribute groups and its own unique userspace code to drive it, with
> no commonality between them.

<snip>