[src/trunk]: src/sys/arch/x86/x86 When we removed the XEN special case from i...

[cherry <cherry%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/5df3c79accdc
branches:  trunk
changeset: 433545:5df3c79accdc
user:      cherry <cherry%NetBSD.org@localhost>
date:      Thu Sep 20 05:08:45 2018 +0000

description:
When we removed the XEN special case from isa/isa_machdep.c
there was a corner case that was missed in
x86/intr.c:intr_establish_xname()

In isa_machdep.c:isa_intr_establish_xname() the legacy_irq parameter
is never set to -1. It is also incorrect to call
isa_intr_establish_xname() with a legacy_irq parameter of -1.

Thus we infer that whenever we see (legacy_irq == -1) in
intr_establish_xname() which is downstream, we were *NOT* called from
isa_machdep.c:isa_intr_establish_xname()

Given that there are no other users of intr_establish_xnam() which
pass a valid legacy_irq != -1, we assume therefore that we *WERE*
called from isa_machdep.c:isa_intr_establish_xname() in this case.

This is an important distinction in the case where a valid
legacy_irq > NUM_LEGACY_IRQS was passed down from
isa_intr_establish_xname() but was ignored by xen_pirq_alloc() and
overwritten with the "pseudo" irq which is then passed back. We thus
pass the incorrect "legacy" irq value to pirq_establish().

Even though non ISA (ie; PCI and MSI(X) cases), this is the correct
behaviour, we need to maintain (bug?) compatibility with the isa
case.

Thus the one liner diff.

CVS: ----------------------------------------------------------------------
CVS: CVSROOT  cvs.NetBSD.org:/cvsroot
CVS: please use "PR category/123" to have the commitmsg appended to PR 123
CVS:
CVS: Please evaluate your changes and consider the following.
CVS: Abort checkin if you answer no.
CVS: => For all changes:
CVS: Do the changed files compile?
CVS: Has the change been tested?
CVS: => If you are not completely familiar with the changed components:
CVS: Has the change been posted for review?
CVS: Have you allowed enough time for feedback?
CVS: => If the change is major:
CVS: => If the change adds files to, or removes files from $DESTDIR:
CVS: => If you are changing a library or kernel interface:
CVS: Have you successfully run "./build.sh release"?

diffstat:

 sys/arch/x86/x86/intr.c |  5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diffs (26 lines):

diff -r 0a091a27927b -r 5df3c79accdc sys/arch/x86/x86/intr.c
--- a/sys/arch/x86/x86/intr.c   Thu Sep 20 02:51:21 2018 +0000
+++ b/sys/arch/x86/x86/intr.c   Thu Sep 20 05:08:45 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: intr.c,v 1.129 2018/09/14 01:50:51 mrg Exp $   */
+/*     $NetBSD: intr.c,v 1.130 2018/09/20 05:08:45 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.129 2018/09/14 01:50:51 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.130 2018/09/20 05:08:45 cherry Exp $");
 
 #include "opt_intrdebug.h"
 #include "opt_multiprocessor.h"
@@ -1291,6 +1291,7 @@
            sizeof(intrstr_buf));
 
        evtchn = xen_pirq_alloc(&irq, type);
+       irq = (legacy_irq == -1) ? irq : legacy_irq; /* ISA compat */   
        pih = pirq_establish(irq & 0xff, evtchn, handler, arg, level,
            intrstr, xname);
        pih->pic_type = pic->pic_type;