[src/trunk]: src/sys/arch/x86 x86/intr: Work around sleazy clockintr with a s...

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

details:   https://anonhg.NetBSD.org/src/rev/d4b92eab29b2
branches:  trunk
changeset: 373229:d4b92eab29b2
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed Jan 25 15:54:52 2023 +0000

description:
x86/intr: Work around sleazy clockintr with a secret frame argument.

PR kern/57197

diffstat:

 sys/arch/x86/include/intr_private.h |  39 +++++++++++++++++++++++++++++++++++++
 sys/arch/x86/isa/clock.c            |  18 ++++++++++------
 sys/arch/x86/x86/intr.c             |  20 +++++++++++++++---
 3 files changed, 66 insertions(+), 11 deletions(-)

diffs (156 lines):

diff -r e465486de8e3 -r d4b92eab29b2 sys/arch/x86/include/intr_private.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/sys/arch/x86/include/intr_private.h       Wed Jan 25 15:54:52 2023 +0000
@@ -0,0 +1,39 @@
+/*     $NetBSD: intr_private.h,v 1.1 2023/01/25 15:54:52 riastradh Exp $       */
+
+/*-
+ * Copyright (c) 2023 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef        _X86_INTR_PRIVATE_H_
+#define        _X86_INTR_PRIVATE_H_
+
+/*
+ * XXX This is a horrible kludge to let intr_establish_xname detect
+ * when it needs to handle a sleazy extra argument to the interrupt
+ * handler that's not part of the normal interrupt handler signature.
+ */
+int i8254_clockintr(void *, struct intrframe *);
+
+#endif /* _X86_INTR_PRIVATE_H_ */
diff -r e465486de8e3 -r d4b92eab29b2 sys/arch/x86/isa/clock.c
--- a/sys/arch/x86/isa/clock.c  Wed Jan 25 13:37:31 2023 +0000
+++ b/sys/arch/x86/isa/clock.c  Wed Jan 25 15:54:52 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: clock.c,v 1.40 2023/01/24 09:57:16 riastradh Exp $     */
+/*     $NetBSD: clock.c,v 1.41 2023/01/25 15:54:52 riastradh Exp $     */
 
 /*-
  * Copyright (c) 1990 The Regents of the University of California.
@@ -121,7 +121,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.40 2023/01/24 09:57:16 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: clock.c,v 1.41 2023/01/25 15:54:52 riastradh Exp $");
 
 /* #define CLOCKDEBUG */
 /* #define CLOCK_PARANOIA */
@@ -152,6 +152,7 @@
 #include <x86/lock.h>
 #include <machine/specialreg.h>
 #include <x86/rtc.h>
+#include <x86/intr_private.h>
 
 #ifndef __x86_64__
 #include "mca.h"
@@ -188,8 +189,6 @@
 void           sysbeep(int, int);
 static void     tickle_tc(void);
 
-static int     clockintr(void *, struct intrframe *);
-
 int            sysbeepdetach(device_t, int);
 
 static unsigned int    gettick_broken_latch(void);
@@ -371,8 +370,8 @@
 
 }
 
-static int
-clockintr(void *arg, struct intrframe *frame)
+int
+i8254_clockintr(void *arg, struct intrframe *frame)
 {
        tickle_tc();
 
@@ -555,9 +554,14 @@
        /*
         * XXX If you're doing strange things with multiple clocks, you might
         * want to keep track of clock handlers.
+        *
+        * XXX This is an abuse of the interrupt handler signature with
+        * __FPTRCAST which requires a special case for IPL_CLOCK in
+        * intr_establish_xname.  Please fix this nonsense!  See also
+        * the comments about i8254_clockintr in x86/x86/intr.c.
         */
        (void)isa_intr_establish(NULL, 0, IST_PULSE, IPL_CLOCK,
-           __FPTRCAST(int (*)(void *), clockintr), 0);
+           __FPTRCAST(int (*)(void *), i8254_clockintr), 0);
 }
 
 void
diff -r e465486de8e3 -r d4b92eab29b2 sys/arch/x86/x86/intr.c
--- a/sys/arch/x86/x86/intr.c   Wed Jan 25 13:37:31 2023 +0000
+++ b/sys/arch/x86/x86/intr.c   Wed Jan 25 15:54:52 2023 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: intr.c,v 1.163 2022/10/29 13:59:04 riastradh Exp $     */
+/*     $NetBSD: intr.c,v 1.164 2023/01/25 15:54:53 riastradh Exp $     */
 
 /*
  * Copyright (c) 2007, 2008, 2009, 2019 The NetBSD Foundation, Inc.
@@ -133,7 +133,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.163 2022/10/29 13:59:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: intr.c,v 1.164 2023/01/25 15:54:53 riastradh Exp $");
 
 #include "opt_intrdebug.h"
 #include "opt_multiprocessor.h"
@@ -162,6 +162,8 @@
 #include <machine/i8259.h>
 #include <machine/pio.h>
 
+#include <x86/intr_private.h>
+
 #include "ioapic.h"
 #include "lapic.h"
 #include "pci.h"
@@ -944,11 +946,21 @@
        ih->ih_slot = slot;
        strlcpy(ih->ih_xname, xname, sizeof(ih->ih_xname));
 #ifdef KDTRACE_HOOKS
-       ih->ih_fun = intr_kdtrace_wrapper;
-       ih->ih_arg = ih;
+       /*
+        * XXX i8254_clockintr is special -- takes a magic extra
+        * argument.  This should be fixed properly in some way that
+        * doesn't involve sketchy function pointer casts.  See also
+        * the comments in x86/isa/clock.c.
+        */
+       if (handler != __FPTRCAST(int (*)(void *), i8254_clockintr)) {
+               ih->ih_fun = intr_kdtrace_wrapper;
+               ih->ih_arg = ih;
+       }
 #endif
 #ifdef MULTIPROCESSOR
        if (!mpsafe) {
+               KASSERT(handler !=                      /* XXX */
+                   __FPTRCAST(int (*)(void *), i8254_clockintr));
                ih->ih_fun = intr_biglock_wrapper;
                ih->ih_arg = ih;
        }