Date: Fri, 14 Apr 2023 14:00:58 GMT From: =?utf-8?Q?Roger=20Pau=20Monn=C3=A9?= <royger@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: af610cabf1f4 - main - xen/intr: adjust xen_intr_handle_upcall() to match driver filter Message-ID: <202304141400.33EE0wxP083825@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by royger: URL: https://cgit.FreeBSD.org/src/commit/?id=af610cabf1f4ae3129596d8f37a2ed70e142d057 commit af610cabf1f4ae3129596d8f37a2ed70e142d057 Author: Elliott Mitchell <ehem+freebsd@m5p.com> AuthorDate: 2021-04-21 14:05:11 +0000 Commit: Roger Pau Monné <royger@FreeBSD.org> CommitDate: 2023-04-14 13:58:52 +0000 xen/intr: adjust xen_intr_handle_upcall() to match driver filter xen_intr_handle_upcall() has two interfaces. It needs to be called by the x86 assembly code invoked by the APIC. Second, it needs to be called as a driver_filter_t for the XenPCI code and for architectures besides x86. Unfortunately the driver_filter_t interface was implemented as a wrapper around the x86-APIC interface. Now create a simple wrapper for the x86-APIC code, which calls an architecture-independent xen_intr_handle_upcall(). When called via intr_event_handle(), driver_filter_t functions expect preemption to be disabled. This removes the need for critical_enter()/critical_exit() when called this way. The lapic_eoi() call is only needed on x86 in some cases when invoked directly as an APIC vector handler. Additionally driver_filter_t functions have no need to handle interrupt counters. The intrcnt_add() calling function was reworked to match the current situation. intrcnt_add() is now only called via one path. The increment/decrement of curthread->td_intr_nesting_level had previously been left out. Appears this was mostly harmless, but this was noticed during implementation and has been added. CONFIG_X86 is a leftover from use with Linux. While the barrier isn't needed for FreeBSD on x86, it will be needed for FreeBSD on other architectures. Copyright note. xen_intr_intrcnt_add() was introduced at 76acc41fb7c7 by Justin T. Gibbs. xen_intrcnt_init() was introduced at fd036deac1695 by John Baldwin. sys/x86/xen/xen_arch_intr.c was originally created by Julien Grall in 2015 for the purpose of holding the x86 interrupt interface. Later it was found xen_intr_handle_upcall() was better earlier, and the x86 interrupt interface better later. As such the filename and header list belong to Julien Grall, but what those were created for is later. Reviewed by: royger Differential Revision: https://reviews.freebsd.org/D30006 --- sys/amd64/amd64/apic_vector.S | 2 +- sys/conf/files.x86 | 1 + sys/dev/xen/xenpci/xenpci.c | 9 +--- sys/i386/i386/apic_vector.S | 2 +- sys/x86/xen/xen_arch_intr.c | 103 ++++++++++++++++++++++++++++++++++++++++++ sys/x86/xen/xen_intr.c | 66 ++++----------------------- sys/xen/xen_intr.h | 5 +- 7 files changed, 120 insertions(+), 68 deletions(-) diff --git a/sys/amd64/amd64/apic_vector.S b/sys/amd64/amd64/apic_vector.S index cea9b79a0ec8..5515213cd154 100644 --- a/sys/amd64/amd64/apic_vector.S +++ b/sys/amd64/amd64/apic_vector.S @@ -159,7 +159,7 @@ IDTVEC(spuriousint) INTR_HANDLER xen_intr_upcall KMSAN_ENTER movq %rsp, %rdi - call xen_intr_handle_upcall + call xen_arch_intr_handle_upcall KMSAN_LEAVE jmp doreti #endif diff --git a/sys/conf/files.x86 b/sys/conf/files.x86 index 8774139eee3a..54f6e8c8260a 100644 --- a/sys/conf/files.x86 +++ b/sys/conf/files.x86 @@ -347,3 +347,4 @@ x86/x86/delay.c standard x86/xen/hvm.c optional xenhvm x86/xen/xen_intr.c optional xenhvm x86/xen/xen_apic.c optional xenhvm smp +x86/xen/xen_arch_intr.c optional xenhvm diff --git a/sys/dev/xen/xenpci/xenpci.c b/sys/dev/xen/xenpci/xenpci.c index ca54b691b8ad..7dd69688a00e 100644 --- a/sys/dev/xen/xenpci/xenpci.c +++ b/sys/dev/xen/xenpci/xenpci.c @@ -52,13 +52,6 @@ __FBSDID("$FreeBSD$"); #include <dev/xen/xenpci/xenpcivar.h> -static int -xenpci_intr_filter(void *trap_frame) -{ - xen_intr_handle_upcall(trap_frame); - return (FILTER_HANDLED); -} - static int xenpci_irq_init(device_t device, struct xenpci_softc *scp) { @@ -66,7 +59,7 @@ xenpci_irq_init(device_t device, struct xenpci_softc *scp) error = BUS_SETUP_INTR(device_get_parent(device), device, scp->res_irq, INTR_MPSAFE|INTR_TYPE_MISC, - xenpci_intr_filter, NULL, /*trap_frame*/NULL, + xen_intr_handle_upcall, NULL, NULL, &scp->intr_cookie); if (error) return error; diff --git a/sys/i386/i386/apic_vector.S b/sys/i386/i386/apic_vector.S index 6c4f8b78fe55..270e0d0393ab 100644 --- a/sys/i386/i386/apic_vector.S +++ b/sys/i386/i386/apic_vector.S @@ -183,7 +183,7 @@ IDTVEC(xen_intr_upcall) cld KENTER pushl %esp - movl $xen_intr_handle_upcall, %eax + movl $xen_arch_intr_handle_upcall, %eax call *%eax add $4, %esp jmp doreti diff --git a/sys/x86/xen/xen_arch_intr.c b/sys/x86/xen/xen_arch_intr.c new file mode 100644 index 000000000000..d946ca781355 --- /dev/null +++ b/sys/x86/xen/xen_arch_intr.c @@ -0,0 +1,103 @@ +/*- + * SPDX-License-Identifier: MIT OR GPL-2.0-only + * + * Copyright © 2013 Spectra Logic Corporation + * Copyright © 2018 John Baldwin/The FreeBSD Foundation + * Copyright © 2019 Roger Pau Monné/Citrix Systems R&D + * Copyright © 2021 Elliott Mitchell + * + * This file may be distributed separately from the Linux kernel, or + * incorporated into other software packages, subject to the following license: + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this source file (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, copy, modify, + * merge, publish, distribute, sublicense, and/or sell copies of the Software, + * and to permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include <sys/cdefs.h> +__FBSDID("$FreeBSD$"); + +#include <sys/param.h> +#include <sys/bus.h> +#include <sys/kernel.h> +#include <sys/pcpu.h> +#include <sys/proc.h> +#include <sys/smp.h> +#include <sys/stddef.h> + +#include <xen/xen-os.h> +#include <xen/xen_intr.h> + +#include <x86/intr_machdep.h> +#include <x86/apicvar.h> + +/************************ Xen x86 interrupt interface ************************/ + +/* + * Pointers to the interrupt counters + */ +DPCPU_DEFINE_STATIC(u_long *, pintrcnt); + +static void +xen_intrcnt_init(void *dummy __unused) +{ + unsigned int i; + + if (!xen_domain()) + return; + + CPU_FOREACH(i) { + char buf[MAXCOMLEN + 1]; + + snprintf(buf, sizeof(buf), "cpu%d:xen", i); + intrcnt_add(buf, DPCPU_ID_PTR(i, pintrcnt)); + } +} +SYSINIT(xen_intrcnt_init, SI_SUB_INTR, SI_ORDER_MIDDLE, xen_intrcnt_init, NULL); + +/* + * Transition from assembly language, called from + * sys/{amd64/amd64|i386/i386}/apic_vector.S + */ +extern void xen_arch_intr_handle_upcall(struct trapframe *); +void +xen_arch_intr_handle_upcall(struct trapframe *trap_frame) +{ + struct trapframe *old; + + /* + * Disable preemption in order to always check and fire events + * on the right vCPU + */ + critical_enter(); + + ++*DPCPU_GET(pintrcnt); + + ++curthread->td_intr_nesting_level; + old = curthread->td_intr_frame; + curthread->td_intr_frame = trap_frame; + + xen_intr_handle_upcall(NULL); + + curthread->td_intr_frame = old; + --curthread->td_intr_nesting_level; + + if (xen_evtchn_needs_ack) + lapic_eoi(); + + critical_exit(); +} diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c index 1bfe941bc67b..0c4d2cd9222b 100644 --- a/sys/x86/xen/xen_intr.c +++ b/sys/x86/xen/xen_intr.c @@ -45,6 +45,7 @@ __FBSDID("$FreeBSD$"); #include <sys/mutex.h> #include <sys/interrupt.h> #include <sys/pcpu.h> +#include <sys/proc.h> #include <sys/smp.h> #include <sys/refcount.h> @@ -56,7 +57,6 @@ __FBSDID("$FreeBSD$"); #include <machine/stdarg.h> #include <xen/xen-os.h> -#include <xen/hvm.h> #include <xen/hypervisor.h> #include <xen/xen_intr.h> #include <xen/evtchn/evtchnvar.h> @@ -102,9 +102,6 @@ struct xen_intr_pcpu_data { */ u_int last_processed_l2i; - /** Pointer to this CPU's interrupt statistic counter. */ - u_long *evtchn_intrcnt; - /** * A bitmap of ports that can be serviced from this CPU. * A set bit means interrupt handling is enabled. @@ -261,25 +258,6 @@ evtchn_cpu_unmask_port(u_int cpu, evtchn_port_t port) xen_set_bit(port, pcpu->evtchn_enabled); } -/** - * Allocate and register a per-cpu Xen upcall interrupt counter. - * - * \param cpu The cpu for which to register this interrupt count. - */ -static void -xen_intr_intrcnt_add(u_int cpu) -{ - char buf[MAXCOMLEN + 1]; - struct xen_intr_pcpu_data *pcpu; - - pcpu = DPCPU_ID_PTR(cpu, xen_intr_pcpu); - if (pcpu->evtchn_intrcnt != NULL) - return; - - snprintf(buf, sizeof(buf), "cpu%d:xen", cpu); - intrcnt_add(buf, &pcpu->evtchn_intrcnt); -} - /** * Search for an already allocated but currently unused Xen interrupt * source object. @@ -526,9 +504,10 @@ xen_intr_active_ports(const struct xen_intr_pcpu_data *const pcpu, * * \param trap_frame The trap frame context for the current interrupt. */ -void -xen_intr_handle_upcall(struct trapframe *trap_frame) +int +xen_intr_handle_upcall(void *unused __unused) { + struct trapframe *trap_frame = curthread->td_intr_frame; u_int l1i, l2i, port, cpu __diagused; u_long masked_l1, masked_l2; struct xenisrc *isrc; @@ -536,11 +515,8 @@ xen_intr_handle_upcall(struct trapframe *trap_frame) struct xen_intr_pcpu_data *pc; u_long l1, l2; - /* - * Disable preemption in order to always check and fire events - * on the right vCPU - */ - critical_enter(); + /* We must remain on the same vCPU during this function */ + CRITICAL_ASSERT(curthread); cpu = PCPU_GET(cpuid); pc = DPCPU_PTR(xen_intr_pcpu); @@ -551,19 +527,15 @@ xen_intr_handle_upcall(struct trapframe *trap_frame) } v->evtchn_upcall_pending = 0; - -#if 0 -#ifndef CONFIG_X86 /* No need for a barrier -- XCHG is a barrier on x86. */ +/* No need for a barrier on x86 -- XCHG is a barrier on x86. */ +#if !defined(__amd64__) && !defined(__i386__) /* Clear master flag /before/ clearing selector flag. */ wmb(); #endif -#endif - l1 = atomic_readandclear_long(&v->evtchn_pending_sel); l1i = pc->last_processed_l1i; l2i = pc->last_processed_l2i; - (*pc->evtchn_intrcnt)++; while (l1 != 0) { l1i = (l1i + 1) % LONG_BIT; @@ -627,10 +599,7 @@ xen_intr_handle_upcall(struct trapframe *trap_frame) } } - if (xen_evtchn_needs_ack) - lapic_eoi(); - - critical_exit(); + return (FILTER_HANDLED); } static int @@ -682,23 +651,6 @@ xen_intr_init(void *dummy __unused) } SYSINIT(xen_intr_init, SI_SUB_INTR, SI_ORDER_SECOND, xen_intr_init, NULL); -static void -xen_intrcnt_init(void *dummy __unused) -{ - unsigned int i; - - if (!xen_domain()) - return; - - /* - * Register interrupt count manually as we aren't guaranteed to see a - * call to xen_intr_assign_cpu() before our first interrupt. - */ - CPU_FOREACH(i) - xen_intr_intrcnt_add(i); -} -SYSINIT(xen_intrcnt_init, SI_SUB_INTR, SI_ORDER_MIDDLE, xen_intrcnt_init, NULL); - void xen_intr_alloc_irqs(void) { diff --git a/sys/xen/xen_intr.h b/sys/xen/xen_intr.h index 5ebce0678222..d8e85b73d95e 100644 --- a/sys/xen/xen_intr.h +++ b/sys/xen/xen_intr.h @@ -38,7 +38,10 @@ /** Registered Xen interrupt callback handle. */ typedef void * xen_intr_handle_t; -void xen_intr_handle_upcall(struct trapframe *trap_frame); +/* + * Main handler for Xen event channel interrupts + */ +extern driver_filter_t xen_intr_handle_upcall; /** * Associate an already allocated local event channel port an interrupt
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202304141400.33EE0wxP083825>