Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/arch Switch over to a "GSI" concept for guest irqs.



details:   https://anonhg.NetBSD.org/src/rev/160b8b4e9cb4
branches:  trunk
changeset: 836267:160b8b4e9cb4
user:      cherry <cherry%NetBSD.org@localhost>
date:      Sun Oct 07 05:23:01 2018 +0000

description:
Switch over to a "GSI" concept for guest irqs.

On XEN there is a namespace called GSI which includes:

i) legacy_irq (0 - 16)
ii) "gsi" (16-nr_irqs_gsi)
iii) msi

We try to mirror this in guest space, but are mindful that legacy_irq
is 1:1 bound to actual hardware legacy_irq. Apart from this, XEN doesn't
really care what number scheme we use, as long as it doesn't encroach
on the MSI space, which is TBD for us.

Thus we trust the mpbios.c/mpacpi.c code to correctly map the pic,pin
tuples into the correct global gsi space, which we then register with
xen. As we now do, we allow for duplicate gsi registrations, in case
any hardware shares the same (pic,pin);

This enables us to now use the (pic,pin) tuple as the canonical reference
for device interrupt addresses, and leave any global mappings to specific
code. Thus xen_pic_to_gsi().

Note that this requires separate support for MSI, which I will get around to
once things stabilise - however the API change facilitates this nicely.

I note that the msi addroute() function does not use the "pin" parameter.
This can be made use of, to encode the gsi number, for XEN. This is however
TBD.

We further tweak the xen_vec_alloc() code to be uniform for the NIOAPICS
and other cases, and ensure that i8259.c DTRT wrt to route().

This will allow us to use pic->pic_addroute() without needing to worry about
pic specific issues.

The next step is to consolidate the pic_addroute() XEN related #ifdefs into
a -DXEN specific file, so that we don't clutter x86/ code with #ifdef XENs.

This change has functional implications, and there is likely breakage coming
especially on bespoke platforms that I haven't been able to test yet.

I am especially interested in bug reports from platforms with legacy (esp. i386)
and with multiple ioapics.

diffstat:

 sys/arch/x86/x86/i8259.c    |   48 +++++++++++++++++-
 sys/arch/x86/x86/intr.c     |   33 ++----------
 sys/arch/xen/include/intr.h |    5 +-
 sys/arch/xen/x86/pintr.c    |  112 ++++++++++++++++++++-----------------------
 4 files changed, 108 insertions(+), 90 deletions(-)

diffs (truncated from 320 to 300 lines):

diff -r 0a291f0f86a1 -r 160b8b4e9cb4 sys/arch/x86/x86/i8259.c
--- a/sys/arch/x86/x86/i8259.c  Sat Oct 06 23:48:00 2018 +0000
+++ b/sys/arch/x86/x86/i8259.c  Sun Oct 07 05:23:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: i8259.c,v 1.17 2018/02/17 18:51:53 maxv Exp $  */
+/*     $NetBSD: i8259.c,v 1.18 2018/10/07 05:23:01 cherry Exp $        */
 
 /*
  * Copyright 2002 (c) Wasabi Systems, Inc.
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: i8259.c,v 1.17 2018/02/17 18:51:53 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: i8259.c,v 1.18 2018/10/07 05:23:01 cherry Exp $");
 
 #include <sys/param.h> 
 #include <sys/systm.h>
@@ -88,6 +88,9 @@
 #include <machine/pic.h>
 #include <machine/i8259.h>
 
+#ifdef XEN
+#include <xen/evtchn.h>
+#endif
 
 #ifndef __x86_64__
 #include "mca.h"
@@ -99,6 +102,7 @@
 static void i8259_hwmask(struct pic *, int);
 static void i8259_hwunmask(struct pic *, int);
 static void i8259_setup(struct pic *, struct cpu_info *, int, int, int);
+static void i8259_unsetup(struct pic *, struct cpu_info *, int, int, int);
 static void i8259_reinit_irqs(void);
 
 unsigned i8259_imen;
@@ -115,7 +119,7 @@
        .pic_hwmask = i8259_hwmask,
        .pic_hwunmask = i8259_hwunmask,
        .pic_addroute = i8259_setup,
-       .pic_delroute = i8259_setup,
+       .pic_delroute = i8259_unsetup,
        .pic_level_stubs = legacy_stubs,
        .pic_edge_stubs = legacy_stubs,
 };
@@ -252,10 +256,48 @@
 i8259_setup(struct pic *pic, struct cpu_info *ci,
     int pin, int idtvec, int type)
 {
+#if defined(XEN)
+       /*
+        * This is kludgy, and not the right place, but we can't bind
+        * before the routing has been set to the appropriate 'vector'.
+        * in x86/intr.c, this is done after idt_vec_set(), where this
+        * would have been more appropriate to put this.
+        */
+
+       int port, irq;
+       irq = vect2irq[idtvec];
+       KASSERT(irq != 0);
+       port = bind_pirq_to_evtch(irq);
+       KASSERT(port < NR_EVENT_CHANNELS);
+       KASSERT(port >= 0);
+
+       KASSERT(irq2port[irq] == 0);
+       irq2port[irq] = port + 1;
+
+       xen_atomic_set_bit(&ci->ci_evtmask[0], port);
+#else
        if (CPU_IS_PRIMARY(ci))
                i8259_reinit_irqs();
+#endif
 }
 
+static void
+i8259_unsetup(struct pic *pic, struct cpu_info *ci,
+    int pin, int idtvec, int type)
+{
+#if defined(XEN)
+       int port, irq;
+       irq = vect2irq[idtvec];
+       port = unbind_pirq_from_evtch(irq);
+
+       KASSERT(port < NR_EVENT_CHANNELS);
+#else
+       if (CPU_IS_PRIMARY(ci))
+               i8259_reinit_irqs();
+#endif
+}
+
+
 void
 i8259_reinit(void)
 {
diff -r 0a291f0f86a1 -r 160b8b4e9cb4 sys/arch/x86/x86/intr.c
--- a/sys/arch/x86/x86/intr.c   Sat Oct 06 23:48:00 2018 +0000
+++ b/sys/arch/x86/x86/intr.c   Sun Oct 07 05:23:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: intr.c,v 1.132 2018/10/06 16:49:54 cherry Exp $        */
+/*     $NetBSD: intr.c,v 1.133 2018/10/07 05:23:01 cherry Exp $        */
 
 /*
  * Copyright (c) 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -133,7 +133,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.132 2018/10/06 16:49:54 cherry Exp $");
+__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.133 2018/10/07 05:23:01 cherry Exp $");
 
 #include "opt_intrdebug.h"
 #include "opt_multiprocessor.h"
@@ -1263,7 +1263,7 @@
 
 #if NPCI > 0 || NISA > 0
        struct pintrhand *pih;
-       intr_handle_t irq;
+       int gsi;
        int vector, evtchn;
 
        KASSERTMSG(legacy_irq == -1 || (0 <= legacy_irq && legacy_irq < NUM_XEN_IRQS),
@@ -1271,40 +1271,21 @@
        KASSERTMSG(!(legacy_irq == -1 && pic == &i8259_pic),
            "non-legacy IRQon i8259 ");
 
-       if (pic->pic_type != PIC_I8259) {
-#if NIOAPIC > 0
-               /* Are we passing mp tranmogrified/cascaded irqs ? */
-               irq = (legacy_irq == -1) ? 0 : legacy_irq;
+       gsi = xen_pic_to_gsi(pic, pin);
 
-               /* will do interrupts via I/O APIC */
-               irq |= APIC_INT_VIA_APIC;
-               irq |= pic->pic_apicid << APIC_INT_APIC_SHIFT;
-               irq |= pin << APIC_INT_PIN_SHIFT;
-#else /* NIOAPIC */
-               return NULL;
-#endif /* NIOAPIC */
-       } else {
-               irq = legacy_irq;
-       }
-
-       intrstr = intr_create_intrid(irq, pic, pin, intrstr_buf,
+       intrstr = intr_create_intrid(gsi, pic, pin, intrstr_buf,
            sizeof(intrstr_buf));
 
-       vector = xen_vec_alloc(irq);
-       irq = vect2irq[vector];
-       irq = (legacy_irq == -1) ? irq : legacy_irq; /* ISA compat */
+       vector = xen_vec_alloc(gsi);
 
-#if NIOAPIC > 0
        extern struct cpu_info phycpu_info_primary; /* XXX */
        struct cpu_info *ci = &phycpu_info_primary;
        pic->pic_addroute(pic, ci, pin, vector, type);
-#else
 
-#endif /* NIOAPIC */
        evtchn = irq2port[vect2irq[vector]];
        KASSERT(evtchn > 0);
 
-       pih = pirq_establish(irq & 0xff, evtchn, handler, arg, level,
+       pih = pirq_establish(gsi, evtchn, handler, arg, level,
                             intrstr, xname);
        pih->pic_type = pic->pic_type;
        return pih;
diff -r 0a291f0f86a1 -r 160b8b4e9cb4 sys/arch/xen/include/intr.h
--- a/sys/arch/xen/include/intr.h       Sat Oct 06 23:48:00 2018 +0000
+++ b/sys/arch/xen/include/intr.h       Sun Oct 07 05:23:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: intr.h,v 1.47 2018/10/06 16:49:54 cherry Exp $ */
+/*     $NetBSD: intr.h,v 1.48 2018/10/07 05:23:01 cherry Exp $ */
 /*     NetBSD intr.h,v 1.15 2004/10/31 10:39:34 yamt Exp       */
 
 /*-
@@ -71,7 +71,8 @@
 #endif
 
 #if defined(DOM0OPS) || NPCI > 0
-int xen_vec_alloc(intr_handle_t);
+int xen_vec_alloc(int);
+int xen_pic_to_gsi(struct pic *, int);
 #endif /* defined(DOM0OPS) || NPCI > 0 */
 
 #ifdef MULTIPROCESSOR
diff -r 0a291f0f86a1 -r 160b8b4e9cb4 sys/arch/xen/x86/pintr.c
--- a/sys/arch/xen/x86/pintr.c  Sat Oct 06 23:48:00 2018 +0000
+++ b/sys/arch/xen/x86/pintr.c  Sun Oct 07 05:23:01 2018 +0000
@@ -103,7 +103,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pintr.c,v 1.6 2018/10/06 16:49:54 cherry Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pintr.c,v 1.7 2018/10/07 05:23:01 cherry Exp $");
 
 #include "opt_multiprocessor.h"
 #include "opt_xen.h"
@@ -160,70 +160,64 @@
 
 #if defined(DOM0OPS) || NPCI > 0
 int
-xen_vec_alloc(intr_handle_t pirq)
+xen_vec_alloc(int gsi)
 {
        physdev_op_t op;
-       int irq = pirq;
-#if NIOAPIC > 0
+
+       KASSERT(gsi < 255);
+
+       if (irq2port[gsi] == 0) {
+               op.cmd = PHYSDEVOP_ASSIGN_VECTOR;
+               op.u.irq_op.irq = gsi;
+               if (HYPERVISOR_physdev_op(&op) < 0) {
+                       panic("PHYSDEVOP_ASSIGN_VECTOR gsi %d", gsi);
+               }
+               KASSERT(irq2vect[gsi] == 0 ||
+                       irq2vect[gsi] == op.u.irq_op.vector);
+               irq2vect[gsi] = op.u.irq_op.vector;
+               KASSERT(vect2irq[op.u.irq_op.vector] == 0 ||
+                       (gsi > 0 && gsi < 16 &&
+                        vect2irq[op.u.irq_op.vector] == gsi));
+               vect2irq[op.u.irq_op.vector] = gsi;
+       }
+
+       return (irq2vect[gsi]);
+}
+
+/*
+ * This function doesn't "allocate" anything. It merely translates our
+ * understanding of PIC to the XEN 'gsi' namespace. In the case of
+ * MSIs, pirq == irq. In the case of everything else, the hypervisor
+ * doesn't really care, so we just use the native conventions that
+ * have been setup during boot by mpbios.c/mpacpi.c
+ */
+int
+xen_pic_to_gsi(struct pic *pic, int pin)
+{
+       int gsi;
+
+       KASSERT(pic != NULL);
 
        /*
-        * The hypervisor has already allocated vectors and IRQs for the
-        * devices. Reusing the same IRQ doesn't work because as we bind
-        * them for each devices, we can't then change the route entry
-        * of the next device if this one used this IRQ. The easiest is
-        * to allocate IRQs top-down, starting with a high number.
-        * 250 and 230 have been tried, but got rejected by Xen.
-        *
-        * Xen 3.5 also rejects 200. Try out all values until Xen accepts
-        * or none is available.
+        * We assume that mpbios/mpacpi have done the right thing.
+        * If so, legacy_irq should identity map correctly to gsi.
         */
-       static int xen_next_irq = 200;
-       struct ioapic_softc *ioapic = ioapic_find(APIC_IRQ_APIC(pirq));
-       int pin = APIC_IRQ_PIN(pirq);
+       gsi = pic->pic_vecbase + pin;
 
-       if (pirq & APIC_INT_VIA_APIC) {
-               irq = vect2irq[ioapic->sc_pins[pin].ip_vector];
-               if (ioapic->sc_pins[pin].ip_vector == 0 || irq == 0) {
-                       /* allocate IRQ */
-                       irq = APIC_IRQ_LEGACY_IRQ(pirq);
-                       if (irq <= 0 || irq > 15)
-                               irq = xen_next_irq--;
-retry:
-                       /* allocate vector and route interrupt */
-                       op.cmd = PHYSDEVOP_ASSIGN_VECTOR;
-                       op.u.irq_op.irq = irq;
-                       if (HYPERVISOR_physdev_op(&op) < 0) {
-                               irq = xen_next_irq--;
-                               if (xen_next_irq == 15)
-                                       panic("PHYSDEVOP_ASSIGN_VECTOR irq %d", irq);
-                               goto retry;
-                       }
-                       KASSERT(irq2vect[irq] == 0 ||
-                               (irq > 0 && irq < 16 &&
-                                irq2vect[irq] == op.u.irq_op.vector));
-                       irq2vect[irq] = op.u.irq_op.vector;
-                       KASSERT(vect2irq[op.u.irq_op.vector] == 0 ||
-                               (irq > 0 && irq < 16 &&
-                                vect2irq[op.u.irq_op.vector] == irq));
-                       vect2irq[op.u.irq_op.vector] = irq;
-               }
-       } else
-#endif /* NIOAPIC */
-       {
-               if (irq2port[irq] == 0) {
-                       op.cmd = PHYSDEVOP_ASSIGN_VECTOR;
-                       op.u.irq_op.irq = irq;
-                       if (HYPERVISOR_physdev_op(&op) < 0) {
-                               panic("PHYSDEVOP_ASSIGN_VECTOR irq %d", irq);
-                       }
-                       KASSERT(irq2vect[irq] == 0);
-                       irq2vect[irq] = op.u.irq_op.vector;
-                       KASSERT(vect2irq[op.u.irq_op.vector] == 0);
-                       vect2irq[op.u.irq_op.vector] = irq;
-                       KASSERT(irq2port[irq] == 0);



Home | Main Index | Thread Index | Old Index