Module Name:    src
Committed By:   cherry
Date:           Sun Oct  7 05:23:01 UTC 2018

Modified Files:
        src/sys/arch/x86/x86: i8259.c intr.c
        src/sys/arch/xen/include: intr.h
        src/sys/arch/xen/x86: pintr.c

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/arch/x86/x86/i8259.c
cvs rdiff -u -r1.132 -r1.133 src/sys/arch/x86/x86/intr.c
cvs rdiff -u -r1.47 -r1.48 src/sys/arch/xen/include/intr.h
cvs rdiff -u -r1.6 -r1.7 src/sys/arch/xen/x86/pintr.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/arch/x86/x86/i8259.c
diff -u src/sys/arch/x86/x86/i8259.c:1.17 src/sys/arch/x86/x86/i8259.c:1.18
--- src/sys/arch/x86/x86/i8259.c:1.17	Sat Feb 17 18:51:53 2018
+++ src/sys/arch/x86/x86/i8259.c	Sun Oct  7 05:23:01 2018
@@ -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 @@ __KERNEL_RCSID(0, "$NetBSD: i8259.c,v 1.
 #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 @@ __KERNEL_RCSID(0, "$NetBSD: i8259.c,v 1.
 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 @@ struct pic i8259_pic = {
 	.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 @@ static void
 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)
 {

Index: src/sys/arch/x86/x86/intr.c
diff -u src/sys/arch/x86/x86/intr.c:1.132 src/sys/arch/x86/x86/intr.c:1.133
--- src/sys/arch/x86/x86/intr.c:1.132	Sat Oct  6 16:49:54 2018
+++ src/sys/arch/x86/x86/intr.c	Sun Oct  7 05:23:01 2018
@@ -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 @@ intr_establish_xname(int legacy_irq, str
 
 #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 @@ intr_establish_xname(int legacy_irq, str
 	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;

Index: src/sys/arch/xen/include/intr.h
diff -u src/sys/arch/xen/include/intr.h:1.47 src/sys/arch/xen/include/intr.h:1.48
--- src/sys/arch/xen/include/intr.h:1.47	Sat Oct  6 16:49:54 2018
+++ src/sys/arch/xen/include/intr.h	Sun Oct  7 05:23:01 2018
@@ -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 @@ int xen_intr_biglock_wrapper(void *);
 #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

Index: src/sys/arch/xen/x86/pintr.c
diff -u src/sys/arch/xen/x86/pintr.c:1.6 src/sys/arch/xen/x86/pintr.c:1.7
--- src/sys/arch/xen/x86/pintr.c:1.6	Sat Oct  6 16:49:54 2018
+++ src/sys/arch/xen/x86/pintr.c	Sun Oct  7 05:23:01 2018
@@ -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 @@ int vect2irq[256] = {0};
 
 #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);
-
-	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);
-			irq2port[irq] = bind_pirq_to_evtch(irq) + 1;
-		}
+	gsi = pic->pic_vecbase + pin;
+
+	switch (pic->pic_type) {
+	case PIC_I8259:
+		KASSERT(gsi < 16);
+		break;
+	case PIC_IOAPIC:
+		break;
+	default: /* XXX: MSI Support */
+		panic("XXX: MSI(X) Support");
+		break;
 	}
-	return (irq2vect[irq]);
+
+	KASSERT(gsi < 255);
+	return gsi;
 }
+
+
 #endif /* defined(DOM0OPS) || NPCI > 0 */

Reply via email to