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

Marcelo Tosatti
2013-02-20T19:58:54Z

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>