On 02/15/2013 10:51:16 PM, Paul Mackerras wrote:
On Fri, Feb 15, 2013 at 09:57:06PM -0600, Scott Wood wrote:
> On 02/15/2013 08:56:14 PM, Paul Mackerras wrote:
> >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.
>
> OK.  This is device-specific behavior, so you could define it
> differently for XICS than MPIC.  I suppose we could change it for
> MPIC as well, to leave an opening for the unlikely case where we'd
> want to model an MPIC that isn't directly connected to the CPUs.

You can have cascaded MPICs; I once had a powerbook that had one MPIC
cascaded into another.

OK.

> How is the explicit request made in this patchset?

The KVM_CREATE_IRQCHIP_ARGS ioctl says that you want emulation of a
specific interrupt controller architecture connected to the vcpus'
external interrupt inputs.  In that sense it's explicit, compared to a
generic "create device" ioctl that could be for any device.

Hooking up to the CPU's interrupt lines is implicit in creating an MPIC (and I'm fine with changing that), not in creating any device. I don't see how it's worse than being implicit in calling KVM_CREATE_IRQCHIP_ARGS (which doesn't allow for cascaded irqchips).

> The standardized interface doesn't make things any easier (as noted
> above, the caller is already mpic-specific code), and we'd have to
> come up with a scheme for flattening our interrupt numberspace
> (rather than introduce new attribute groups for things like IPI and
> timer interrupts).  It may still be necessary when it comes to
> irqfd, though...

With 24 bits of interrupt source number, you don't have to flatten it
very far.  IPIs are handled separately and don't appear in the
interrupt source space.

They do need to appear in the interrupt source space if we want to inject or irqfd them. Most users won't want to do that, but we have had a customer directly assign IPIs (to communicate with an OS running on the other CPU in an AMP setup -- host Linux was non-SMP so wasn't using them) and MPIC timers to a guest.

> >> How do MSIs get injected?
> >
> >Just like other interrupts - from the point of view of the interrupt
> >controller they're edge-triggered interrupt sources.
>
> Ah right, I guess this is all set up via hcalls for XICS.
>
> With MPIC exposing its registers via the device control api,
> everything just works -- the PCI device generates a write to the
> MPIC's memory region, the QEMU MPIC stub sends the write to the
> kernel as for any other MMIO access (this passthrough is also useful
> for debugging), the in-kernel MPIC sees the write to the "generate
> an MSI" register and does its thing.  Compare that to all special
> the MSI code for APIC... :-)

You're doing a round trip to userspace for every MPIC register access
by the guest?  Seriously?

No. Accesses by the guest get handled in the kernel. Accesses in QEMU, including MSIs generated by virtio, get forwarded to the kernel.

> >There are plenty of bits free in the 64 bits per source that I have
> >allowed.  We can accommodate those things.
>
> MPIC vector numbers take up 16 of the bits.  The architected
> interrupt level field is 8 bits, though only a handful of values are
> actually needed.  Add a couple binary flags, and it gets pretty
> tight if a third type of interrupt controller starts wanting
> something new.

There's enough space for MPIC to have 16 bits of vector and some
flags.  We don't need to overdesign this.

I view anything other than passing the actual MPIC register values around as overdesign here, given that it is communication between hw/kvm/mpic.c on the QEMU side and arch/powerpc/kvm/mpic.c on the kernel side.

> interrupts awaiting service (or even in service, if different
> priorities).  We have both "current task priority" (which is a
> user-set mask-by-priority register) and the priority of the
> highest-prio in-service interrupt -- which would "current processor
> priorty" be?  Etc.

It would be the current task priority.  I assume MPIC maintains a
16-bit map of the interrupt priorities in service, so that would need
to be added.

We don't maintain such a map in the emulation code. We have a per-CPU bitmap of the actual interrupt sources pending/active, which is another attribute that would need to be added in order to support migration on MPIC.

-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to