Module Name:    src
Committed By:   cherry
Date:           Sat Oct  6 16:37:11 UTC 2018

Modified Files:
        src/sys/arch/xen/x86: pintr.c

Log Message:
Teach intr_establish_xname() for XEN to tolerate shared legacy_irq
registrations.

The current XEN code has not been able to tolerate shared legacy_irq
requests in xen_pirq_alloc(). This was never a problem because:

i) The only potential callpath with shared legacy_irq was
   isa_intr_establish_xname().
ii) The other callpath, namely pci_intr_establish_xname() passed
    legacy_irq to intr_establish_xname(). However, this was ignored,
    and a value of zero was passed to xen_pirq_alloc() which in
    turn, allocated a new irq value, thus effectively demultiplexing
    any shared legacy_irq value intended across randomly allocated
    new irq values.
iii) Presumably on all platforms that XEN runs on, the isa callpath
     mentioned in i) never had shared irqs, and thus this was never
     a problem.

Except:
We now use a unified path for both isa_intr_establish_xname() and
pci_intr_establish_xname(). This means that now, intr_establish_xname()
which is a callee of both, needs to have a way to discern who the caller
was, and decide to pass on or discard the legacy_irq value, to preserve
the old semantics. However, this is impossible to do so, because the
callpath doesn't explicitly provide a mechanism for this discernment.

This is however not a problem, because from XEN's point of view, a
repeat registration of an irq is only a warning. legacy_irq is the only
case in which this repeat should occur, per the current implementation of
xen_pirq_alloc(). We thus tweak the KASSERT()s to tolerate a repeat value
in the legacy_irq, while maintaining the original intent of the KASSERT()
which was to ensure that existing unrelated irq registrations should never
be overwritten.

Once we re-organise XEN specific code and unify with the native
intr_establish_xname() path, we will not run into this problem, because
legacy_irq will be treated as the pin number of the i8259 pic
exactly as it is now treated in native.

In short, this commit should fix some of the panics being seen on
-current for certain configurations of hardware on which dom0 runs.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 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/xen/x86/pintr.c
diff -u src/sys/arch/xen/x86/pintr.c:1.3 src/sys/arch/xen/x86/pintr.c:1.4
--- src/sys/arch/xen/x86/pintr.c:1.3	Sat Feb 17 18:51:53 2018
+++ src/sys/arch/xen/x86/pintr.c	Sat Oct  6 16:37:11 2018
@@ -103,7 +103,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: pintr.c,v 1.3 2018/02/17 18:51:53 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: pintr.c,v 1.4 2018/10/06 16:37:11 cherry Exp $");
 
 #include "opt_multiprocessor.h"
 #include "opt_xen.h"
@@ -199,9 +199,13 @@ retry:
 					panic("PHYSDEVOP_ASSIGN_VECTOR irq %d", irq);
 				goto retry;
 			}
-			KASSERT(irq2vect[irq] == 0);
+			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);
+			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;
 			pic->pic_addroute(pic, &phycpu_info_primary, pin,
 			    op.u.irq_op.vector, type);

Reply via email to