Date: Fri, 30 Aug 2013 16:18:54 +0200 From: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= <roger.pau@citrix.com> To: "Justin T. Gibbs" <gibbs@FreeBSD.org> Cc: xen@FreeBSD.org Subject: Re: [CFR] Event channel Interrupts and unified Xen interrupt infrastructure. Message-ID: <5220A9CE.1090207@citrix.com> In-Reply-To: <FD7B1E0C-84A2-454E-8407-7DAC78275598@FreeBSD.org> References: <7D29BB74-2341-4164-B2B8-85A4CAB3A6CE@FreeBSD.org> <521F9DC5.6040606@freebsd.org> <FD7B1E0C-84A2-454E-8407-7DAC78275598@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--------------020007000202010003090706 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 8bit On 29/08/13 21:29, Justin T. Gibbs wrote: > On Aug 29, 2013, at 1:15 PM, Colin Percival <cperciva@freebsd.org> wrote: > >> On 08/29/13 09:32, Justin T. Gibbs wrote: >>> I've been working to get the next chunk of Spectra/Roger Pau Monné Xen work into head. The latest version of the patch I'm working on can be found here: >>> >>> http://people.freebsd.org/~gibbs/xen_intr.diff >>> >>> I will continue my testing today and commit it tonight unless I hear complaints. Comments, corrections, improvements, and bug reports welcome. >> >> Could this patch be split into functional and non-functional components? It's >> distracting having everything mixed up together in a single megapatch. Even >> committing the s/unlikely/__predict_false/ separately would help. > > It could, but it would delay an already long delayed process in getting this work into the tree. > >> How do you intend to have Xen HVM work in the GENERIC kernel configuration? >> Will you be adding 'options XENHVM' to GENERIC? I'm attaching a patch that introduces Xen support into GENERIC, it can be found as usual on my git tree: http://xenbits.xen.org/gitweb/?p=people/royger/freebsd.git;a=shortlog;h=refs/heads/xenhvm_merge_generic It builds on top of my previous PVHVM series, which is now halfway committed to HEAD. I've tested it under Xen using the GENERIC kernel (for both i386 and amd64), and everything seems to be fine, and I've also tested it on bare metal amd64, which also seems to be running fine. Roger. --------------020007000202010003090706 Content-Type: text/plain; charset="UTF-8"; x-mac-type=0; x-mac-creator=0; name="0001-xen-merge-XENHVM-into-GENERIC.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-xen-merge-XENHVM-into-GENERIC.patch" >From e4303d73826577e7a9bd3031d201aa02a31b720a Mon Sep 17 00:00:00 2001 From: Roger Pau Monne <roger.pau@citrix.com> Date: Thu, 29 Aug 2013 12:00:50 +0200 Subject: [PATCH] xen: merge XENHVM into GENERIC Merge Xen PVHVM support into generic for both amd64 and i386. sys/amd64/amd64/mp_machdep.c: sys/amd64/include/cpu.h: sys/i386/i386/mp_machdep.c: sys/i386/include/cpu.h: - Introduce two new CPU hooks, for initialization and resume purposes. This allows us to get rid of the XENHVM ifdefs in mp_machdep, and also sets some hooks into common code that can be used by others. sys/amd64/conf/XENHVM: sys/i386/conf/XENHVM: - Remove this configs now that GENERIC has builtin support for Xen. sys/kern/subr_smp.c: - Make sure there are no pending IPIs when suspending with the GENERIC kernel. sys/x86/xen/hvm.c: - Add cpu resume operation, that will be called from mp_machdep using the new hooks. - Remove the CPU_FOREACH call in xen_hvm_init, it is not needed, and at the point were xen_hvm_init gets called we still don't know how many CPUs there are on the system. - Gate xen_hvm_init_cpu only to systems running under Xen. sys/x86/xen/xen_intr.c: - Gate the setup of event channels only to systems running under Xen. --- sys/amd64/amd64/mp_machdep.c | 33 +++++++++++------------------- sys/amd64/conf/GENERIC | 4 +++ sys/amd64/conf/XENHVM | 22 -------------------- sys/amd64/include/cpu.h | 2 + sys/i386/conf/GENERIC | 4 +++ sys/i386/conf/XENHVM | 22 -------------------- sys/i386/i386/mp_machdep.c | 33 +++++++++++------------------- sys/i386/include/cpu.h | 2 + sys/kern/subr_smp.c | 2 - sys/x86/xen/hvm.c | 45 ++++++++++++++++++++++++++++++++--------- sys/x86/xen/xen_intr.c | 3 ++ 11 files changed, 74 insertions(+), 98 deletions(-) delete mode 100644 sys/amd64/conf/XENHVM delete mode 100644 sys/i386/conf/XENHVM diff --git a/sys/amd64/amd64/mp_machdep.c b/sys/amd64/amd64/mp_machdep.c index def7335..81c2fe6 100644 --- a/sys/amd64/amd64/mp_machdep.c +++ b/sys/amd64/amd64/mp_machdep.c @@ -71,10 +71,6 @@ __FBSDID("$FreeBSD$"); #include <machine/tss.h> #include <machine/cpu.h> -#ifdef XENHVM -#include <xen/hvm.h> -#endif - #define WARMBOOT_TARGET 0 #define WARMBOOT_OFF (KERNBASE + 0x0467) #define WARMBOOT_SEG (KERNBASE + 0x0469) @@ -126,7 +122,9 @@ static u_long *ipi_hardclock_counts[MAXCPU]; /* Machinery for allowing non-native IPI mechanisms */ DPCPU_DEFINE(struct cpu_ops, cpu_ops) = { - .ipi_vectored = lapic_ipi_vectored + .ipi_vectored = lapic_ipi_vectored, + .cpu_init = NULL, + .cpu_resume = NULL, }; extern inthand_t IDTVEC(fast_syscall), IDTVEC(fast_syscall32); @@ -157,7 +155,7 @@ int cpu_apic_ids[MAXCPU]; int apic_cpuids[MAX_APIC_ID + 1]; /* Holds pending bitmap based IPIs per CPU */ -static volatile u_int cpu_ipi_pending[MAXCPU]; +volatile u_int cpu_ipi_pending[MAXCPU]; static u_int boot_address; static int cpu_logical; /* logical cpus per core */ @@ -621,6 +619,7 @@ init_secondary(void) u_int cpuid; int cpu, gsel_tss, x; struct region_descriptor ap_gdt; + struct cpu_ops *cpu_ops; /* Set by the startup code for us to use */ cpu = bootAP; @@ -721,10 +720,9 @@ init_secondary(void) /* set up FPU state on the AP */ fpuinit(); -#ifdef XENHVM - /* register vcpu_info area */ - xen_hvm_init_cpu(); -#endif + cpu_ops = DPCPU_PTR(cpu_ops); + if (cpu_ops->cpu_init) + cpu_ops->cpu_init(); /* A quick check from sanity claus */ cpuid = PCPU_GET(cpuid); @@ -1436,12 +1434,11 @@ void cpususpend_handler(void) { u_int cpu; + struct cpu_ops *cpu_ops; cpu = PCPU_GET(cpuid); -#ifdef XENHVM KASSERT((!mtx_owned(&smp_ipi_mtx)), ("IPI in process while suspending")); -#endif if (savectx(susppcbs[cpu])) { ctx_fpusave(susppcbs[cpu]->pcb_fpususpend); @@ -1461,15 +1458,9 @@ cpususpend_handler(void) while (!CPU_ISSET(cpu, &started_cpus)) ia32_pause(); -#ifdef XENHVM - /* - * Reset pending bitmap IPIs, because Xen doesn't preserve pending - * event channels on migration. - */ - cpu_ipi_pending[cpu] = 0; - /* register vcpu_info area */ - xen_hvm_init_cpu(); -#endif + cpu_ops = DPCPU_PTR(cpu_ops); + if (cpu_ops->cpu_resume) + cpu_ops->cpu_resume(); /* Resume MCA and local APIC */ mca_resume(); diff --git a/sys/amd64/conf/GENERIC b/sys/amd64/conf/GENERIC index 08865a8..7c059f7 100644 --- a/sys/amd64/conf/GENERIC +++ b/sys/amd64/conf/GENERIC @@ -72,6 +72,7 @@ options KDTRACE_FRAME # Ensure frames are compiled in options KDTRACE_HOOKS # Kernel DTrace hooks options DDB_CTF # Kernel ELF linker loads CTF data options INCLUDE_CONFIG_FILE # Include this file in kernel +options XENHVM # Include Xen support # Debugging support. Always need this: options KDB # Enable kernel debugger support. @@ -340,3 +341,6 @@ device vtnet # VirtIO Ethernet device device virtio_blk # VirtIO Block device device virtio_scsi # VirtIO SCSI device device virtio_balloon # VirtIO Memory Balloon device + +# Xen support +device xenpci # Generic Xen bus diff --git a/sys/amd64/conf/XENHVM b/sys/amd64/conf/XENHVM deleted file mode 100644 index ee745ec..0000000 --- a/sys/amd64/conf/XENHVM +++ /dev/null @@ -1,22 +0,0 @@ -# -# XENHVM -- Xen HVM kernel configuration file for FreeBSD/amd64 -# -# $FreeBSD$ -# -include GENERIC -ident XENHVM - -# -# Adaptive locks rely on a lock-free pointer read to determine the run state -# of the thread holding a lock when under contention; under a virtualisation -# system, the thread run state may not accurately reflect whether the thread -# (or rather its host VCPU) is actually executing. As such, disable this -# optimisation. -# -options NO_ADAPTIVE_MUTEXES -options NO_ADAPTIVE_RWLOCKS -options NO_ADAPTIVE_SX - -# Xen HVM support -options XENHVM -device xenpci diff --git a/sys/amd64/include/cpu.h b/sys/amd64/include/cpu.h index 92b119d..ad627b6 100644 --- a/sys/amd64/include/cpu.h +++ b/sys/amd64/include/cpu.h @@ -59,6 +59,8 @@ * implementation, or hvm for Xen specific implementation. */ struct cpu_ops { + void (*cpu_init)(void); + void (*cpu_resume)(void); void (*ipi_vectored)(u_int, int); }; diff --git a/sys/i386/conf/GENERIC b/sys/i386/conf/GENERIC index 5e6bfd5..1cdd8ee 100644 --- a/sys/i386/conf/GENERIC +++ b/sys/i386/conf/GENERIC @@ -72,6 +72,7 @@ options MAC # TrustedBSD MAC Framework options KDTRACE_HOOKS # Kernel DTrace hooks options DDB_CTF # Kernel ELF linker loads CTF data options INCLUDE_CONFIG_FILE # Include this file in kernel +options XENHVM # Include Xen support # Debugging support. Always need this: options KDB # Enable kernel debugger support. @@ -354,3 +355,6 @@ device vtnet # VirtIO Ethernet device device virtio_blk # VirtIO Block device device virtio_scsi # VirtIO SCSI device device virtio_balloon # VirtIO Memory Balloon device + +# Xen support +device xenpci # Generic Xen bus diff --git a/sys/i386/conf/XENHVM b/sys/i386/conf/XENHVM deleted file mode 100644 index 9e1b52e..0000000 --- a/sys/i386/conf/XENHVM +++ /dev/null @@ -1,22 +0,0 @@ -# -# XENHVM -- Xen HVM kernel configuration file for FreeBSD/i386 -# -# $FreeBSD$ -# -include GENERIC -ident XENHVM - -# -# Adaptive locks rely on a lock-free pointer read to determine the run state -# of the thread holding a lock when under contention; under a virtualisation -# system, the thread run state may not accurately reflect whether the thread -# (or rather its host VCPU) is actually executing. As such, disable this -# optimisation. -# -options NO_ADAPTIVE_MUTEXES -options NO_ADAPTIVE_RWLOCKS -options NO_ADAPTIVE_SX - -# Xen HVM support -options XENHVM -device xenpci diff --git a/sys/i386/i386/mp_machdep.c b/sys/i386/i386/mp_machdep.c index 1d25ee5..8881b63 100644 --- a/sys/i386/i386/mp_machdep.c +++ b/sys/i386/i386/mp_machdep.c @@ -83,10 +83,6 @@ __FBSDID("$FreeBSD$"); #include <machine/specialreg.h> #include <machine/cpu.h> -#ifdef XENHVM -#include <xen/hvm.h> -#endif - #define WARMBOOT_TARGET 0 #define WARMBOOT_OFF (KERNBASE + 0x0467) #define WARMBOOT_SEG (KERNBASE + 0x0469) @@ -173,7 +169,9 @@ static u_long *ipi_hardclock_counts[MAXCPU]; /* Machinery for allowing non-native IPI mechanisms */ DPCPU_DEFINE(struct cpu_ops, cpu_ops) = { - .ipi_vectored = lapic_ipi_vectored + .ipi_vectored = lapic_ipi_vectored, + .cpu_init = NULL, + .cpu_resume = NULL, }; /* @@ -202,7 +200,7 @@ int cpu_apic_ids[MAXCPU]; int apic_cpuids[MAX_APIC_ID + 1]; /* Holds pending bitmap based IPIs per CPU */ -static volatile u_int cpu_ipi_pending[MAXCPU]; +volatile u_int cpu_ipi_pending[MAXCPU]; static u_int boot_address; static int cpu_logical; /* logical cpus per core */ @@ -674,6 +672,7 @@ init_secondary(void) int gsel_tss; int x, myid; u_int cpuid, cr0; + struct cpu_ops *cpu_ops; /* bootAP is set in start_ap() to our ID. */ myid = bootAP; @@ -757,10 +756,9 @@ init_secondary(void) /* set up SSE registers */ enable_sse(); -#ifdef XENHVM - /* register vcpu_info area */ - xen_hvm_init_cpu(); -#endif + cpu_ops = DPCPU_PTR(cpu_ops); + if (cpu_ops->cpu_init) + cpu_ops->cpu_init(); #ifdef PAE /* Enable the PTE no-execute bit. */ @@ -1528,12 +1526,11 @@ void cpususpend_handler(void) { u_int cpu; + struct cpu_ops *cpu_ops; cpu = PCPU_GET(cpuid); -#ifdef XENHVM KASSERT((!mtx_owned(&smp_ipi_mtx)), ("IPI in process while suspending")); -#endif if (savectx(susppcbs[cpu])) { wbinvd(); @@ -1551,15 +1548,9 @@ cpususpend_handler(void) while (!CPU_ISSET(cpu, &started_cpus)) ia32_pause(); -#ifdef XENHVM - /* - * Reset pending bitmap IPIs, because Xen doesn't preserve pending - * event channels on migration. - */ - cpu_ipi_pending[cpu] = 0; - /* register vcpu_info area */ - xen_hvm_init_cpu(); -#endif + cpu_ops = DPCPU_PTR(cpu_ops); + if (cpu_ops->cpu_resume) + cpu_ops->cpu_resume(); /* Resume MCA and local APIC */ mca_resume(); diff --git a/sys/i386/include/cpu.h b/sys/i386/include/cpu.h index f00b14d..5880224 100644 --- a/sys/i386/include/cpu.h +++ b/sys/i386/include/cpu.h @@ -59,6 +59,8 @@ * implementation, or hvm for Xen specific implementation. */ struct cpu_ops { + void (*cpu_init)(void); + void (*cpu_resume)(void); void (*ipi_vectored)(u_int, int); }; diff --git a/sys/kern/subr_smp.c b/sys/kern/subr_smp.c index b5694aa..77bb501 100644 --- a/sys/kern/subr_smp.c +++ b/sys/kern/subr_smp.c @@ -225,7 +225,6 @@ generic_stop_cpus(cpuset_t map, u_int type) CTR2(KTR_SMP, "stop_cpus(%s) with %u type", cpusetobj_strprint(cpusetbuf, &map), type); -#ifdef XENHVM /* * When migrating a PVHVM domain we need to make sure there are * no IPIs in progress, because on resume we must rebind all @@ -234,7 +233,6 @@ generic_stop_cpus(cpuset_t map, u_int type) */ if (type == IPI_SUSPEND) mtx_lock_spin(&smp_ipi_mtx); -#endif if (stopping_cpu != PCPU_GET(cpuid)) while (atomic_cmpset_int(&stopping_cpu, NOCPU, diff --git a/sys/x86/xen/hvm.c b/sys/x86/xen/hvm.c index 7d086ab..7a1bc85 100644 --- a/sys/x86/xen/hvm.c +++ b/sys/x86/xen/hvm.c @@ -96,6 +96,9 @@ extern volatile int smp_tlb_wait; extern vm_offset_t smp_tlb_addr1; extern vm_offset_t smp_tlb_addr2; +/* IPI related variables */ +extern volatile u_int cpu_ipi_pending[MAXCPU]; + #ifdef __i386__ extern void pmap_lazyfix_action(void); #endif @@ -226,6 +229,21 @@ xen_cpustophard_handler(void *arg) return (FILTER_HANDLED); } +/* XEN diverged cpu operations */ + +static void xen_hvm_resume_cpu() +{ + u_int cpuid = PCPU_GET(cpuid); + + /* + * Reset pending bitmap IPIs, because Xen doesn't preserve pending + * event channels on migration. + */ + cpu_ipi_pending[cpuid] = 0; + /* register vcpu_info area */ + xen_hvm_init_cpu(); +} + static int xen_translate_ipi(u_int vector) { @@ -382,13 +400,22 @@ xen_cpu_ipi_init(int cpu) } static void -xen_init_ipis(void) +xen_setup_cpus(void) { int i; - if (xen_hvm_domain() && xen_vector_callback_enabled) { - CPU_FOREACH(i) - xen_cpu_ipi_init(i); + if (xen_hvm_domain()) { + CPU_FOREACH(i) { + struct cpu_ops *cpu_ops = DPCPU_ID_PTR(i, cpu_ops); + + KASSERT((cpu_ops->cpu_init == NULL), ("cpu_init hook not NULL")); + cpu_ops->cpu_init = xen_hvm_init_cpu; + KASSERT((cpu_ops->cpu_resume == NULL), ("cpu_resume hook not NULL")); + cpu_ops->cpu_resume = xen_hvm_resume_cpu; + + if (xen_vector_callback_enabled) + xen_cpu_ipi_init(i); + } } } @@ -563,8 +590,6 @@ xen_hvm_resume(void) static void xen_hvm_init(void *dummy __unused) { - int i; - if (xen_hvm_init_hypercall_stubs() != 0) return; @@ -573,9 +598,6 @@ xen_hvm_init(void *dummy __unused) xen_hvm_init_shared_info_page(); xen_hvm_set_callback(NULL); xen_hvm_disable_emulated_devices(); - /* Clean vcpu_info */ - CPU_FOREACH(i) - DPCPU_ID_SET(i, vcpu_info, NULL); } void xen_hvm_init_cpu(void) @@ -585,6 +607,9 @@ void xen_hvm_init_cpu(void) struct vcpu_register_vcpu_info info; int rc; + if (!xen_domain()) + return; + if (DPCPU_GET(vcpu_info) != NULL) /* * vcpu_info has already been set, @@ -606,5 +631,5 @@ void xen_hvm_init_cpu(void) } SYSINIT(xen_hvm_init, SI_SUB_HYPERVISOR, SI_ORDER_FIRST, xen_hvm_init, NULL); -SYSINIT(xen_init_ipis, SI_SUB_SMP, SI_ORDER_FIRST, xen_init_ipis, NULL); +SYSINIT(xen_init_ipis, SI_SUB_SMP, SI_ORDER_FIRST, xen_setup_cpus, NULL); SYSINIT(xen_hvm_init_cpu, SI_SUB_INTR, SI_ORDER_FIRST, xen_hvm_init_cpu, NULL); diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c index 466279a..cf4aa8d 100644 --- a/sys/x86/xen/xen_intr.c +++ b/sys/x86/xen/xen_intr.c @@ -609,6 +609,9 @@ xen_intr_init(void *dummy __unused) struct xen_intr_pcpu_data *pcpu; int i; + if (!xen_domain()) + return 0; + mtx_init(&xen_intr_isrc_lock, "xen-irq-lock", NULL, MTX_DEF); /* -- 1.7.7.5 (Apple Git-26) --------------020007000202010003090706--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5220A9CE.1090207>