Re: [PATCH v5 0/8] In-kernel XICS interrupt controller emulation

Scott Wood
2013-04-26T17:13:33Z

On 04/26/2013 09:30:37 AM, Alexander Graf wrote:
>
> On 18.04.2013, at 08:29, Paul Mackerras wrote:
>
> > This is a repost of my patch series implementing in-kernel emulation
> > of the XICS interrupt controller architecture defined in PAPR (Power
> > Architecture Platform Requirements, the document that defines IBM's
> > pSeries platform architecture). This version of the patch series
> uses
> > the latest device API as posted by Scott Wood, that is, i.e., the
> > version where the core device code provides the file descriptor and
> > ioctl handler. I have structured the series so that the API is
> added
> > by the last two patches, so as to be able to accommodate any future
> > revisions to the device API with minimal changes.
> >
> > The series is based on Alex Graf's kvm-ppc-queue branch with Scott
> > Wood's recent patch series applied on top, together with the patch
> > below to allow it to build with CONFIG_KVM_MPIC=n.
> >
> > The API defined here uses KVM_CREATE_DEVICE to create the XICS,
> > KVM_DEVICE_SET_ATTR/KVM_DEVICE_GET_ATTR to manipulate the interrupt
> > sources (for initialization and migration), a new KVM_CAP_IRQ_XICS
> > capability to connect vcpus to the XICS, a new identifier
> > KVM_REG_PPC_ICP_STATE for the one-reg interface to get and set
> > per-vcpu state, and the existing KVM_IRQ_LINE ioctl to assert and
> > deassert interrupt sources.
> >
> > This version also cleans up some checkpatch.pl errors and clarifies
> > the lifetime rules for the various objects. There are two
> checkpatch
> > warnings for long lines, but they are long because they have long
> > strings in them, and if I break the strings over two lines then
> > checkpatch warns about that.
>
> Very nice patch set. I've applie 1-7 of it to kvm-ppc-queue. So they
> will hopefully make it to 3.10.
>
> Please check for 8/8 whether
>
> a) You want to have a released kernel version without irq routing
> (irqfd) support. It makes user space's life harder, because you need
> to maintain backwards compatibility.
>
> b) Please rebase on top of the current state of things, especially
> the changed lifecycle assumptions. Devices should now just live until
> the vm gets destroyed. It gives me way less headaches.

Also please note that we no longer hold kvm->lock during device
creation, so your EEXIST check looks racy.

-Scott