* [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver @ 2019-05-27 14:59 Michael Kelley 2019-05-27 14:59 ` [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Michael Kelley ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Michael Kelley @ 2019-05-27 14:59 UTC (permalink / raw) To: will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, vkuznets, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Cc: Michael Kelley, catalin.marinas, mark.rutland This patch series moves Hyper-V clock/timer code to a separate Hyper-V clocksource driver. Previously, Hyper-V clock/timer code and data structures were mixed in with other Hyper-V code in the ISA independent drivers/hv code as well as in arch dependent code. The new Hyper-V clocksource driver is ISA independent, with a just few dependencies on arch specific functions. The patch series does not change any behavior or functionality -- it only reorganizes the existing code and fixes up the linkages. A few places outside of Hyper-V code are fixed up to use the new #include file structure. This restructuring is in response to Marc Zyngier's review comments on supporting Hyper-V running on ARM64, and is a good idea in general. It increases the amount of code shared between the x86 and ARM64 architectures, and reduces the size of the new code for supporting Hyper-V on ARM64. A new version of the Hyper-V on ARM64 patches will follow once this clocksource restructuring is accepted. The code is diff'ed against Linux 5.2.0-rc1-next-20190524. Changes in v3: * Removed boolean argument to hv_init_clocksource(). Always call sched_clock_register, which is needed on ARM64 but a no-op on x86. * Removed separate cpuhp setup in hv_stimer_alloc() and instead directly call hv_stimer_init() and hv_stimer_cleanup() from corresponding VMbus functions. This more closely matches original code and avoids clocksource stop/restart problems on ARM64 when VMbus code denies CPU offlining request. Changes in v2: * Revised commit short descriptions so the distinction between the first and second patches is clearer [GregKH] * Renamed new clocksource driver files and functions to use existing "timer" and "stimer" names instead of introducing "syntimer". [Vitaly Kuznetsov] * Introduced CONFIG_HYPER_TIMER to fix build problem when CONFIG_HYPERV=m [Vitaly Kuznetsov] * Added "Suggested-by: Marc Zyngier" Michael Kelley (2): Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Drivers: hv: Move Hyper-V clocksource code to new clocksource driver MAINTAINERS | 2 + arch/x86/entry/vdso/vclock_gettime.c | 1 + arch/x86/entry/vdso/vma.c | 2 +- arch/x86/hyperv/hv_init.c | 91 +--------- arch/x86/include/asm/hyperv-tlfs.h | 6 + arch/x86/include/asm/mshyperv.h | 81 ++------- arch/x86/kernel/cpu/mshyperv.c | 2 + arch/x86/kvm/x86.c | 1 + drivers/clocksource/Makefile | 1 + drivers/clocksource/hyperv_timer.c | 321 +++++++++++++++++++++++++++++++++++ drivers/hv/Kconfig | 3 + drivers/hv/hv.c | 156 +---------------- drivers/hv/hv_util.c | 1 + drivers/hv/hyperv_vmbus.h | 3 - drivers/hv/vmbus_drv.c | 42 ++--- include/clocksource/hyperv_timer.h | 105 ++++++++++++ 16 files changed, 484 insertions(+), 334 deletions(-) create mode 100644 drivers/clocksource/hyperv_timer.c create mode 100644 include/clocksource/hyperv_timer.h -- 1.8.3.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code 2019-05-27 14:59 [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver Michael Kelley @ 2019-05-27 14:59 ` Michael Kelley 2019-05-30 9:37 ` Vitaly Kuznetsov 2019-05-27 14:59 ` [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver Michael Kelley 2019-06-06 16:37 ` [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate " Sasha Levin 2 siblings, 1 reply; 8+ messages in thread From: Michael Kelley @ 2019-05-27 14:59 UTC (permalink / raw) To: will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, vkuznets, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Cc: Michael Kelley, catalin.marinas, mark.rutland Clockevents code for Hyper-V synthetic timers is currently mixed in with other Hyper-V code. Move the code to a Hyper-V specific driver in the "clocksource" directory. Update the VMbus driver to call initialization and cleanup routines since the Hyper-V synthetic timers are not independently enumerated in ACPI. No behavior is changed and no new functionality is added. Suggested-by: Marc Zyngier <marc.zyngier@arm.com> Signed-off-by: Michael Kelley <mikelley@microsoft.com> --- MAINTAINERS | 2 + arch/x86/include/asm/hyperv-tlfs.h | 6 ++ arch/x86/kernel/cpu/mshyperv.c | 2 + drivers/clocksource/Makefile | 1 + drivers/clocksource/hyperv_timer.c | 191 +++++++++++++++++++++++++++++++++++++ drivers/hv/Kconfig | 3 + drivers/hv/hv.c | 156 +----------------------------- drivers/hv/hyperv_vmbus.h | 3 - drivers/hv/vmbus_drv.c | 42 ++++---- include/clocksource/hyperv_timer.h | 27 ++++++ 10 files changed, 258 insertions(+), 175 deletions(-) create mode 100644 drivers/clocksource/hyperv_timer.c create mode 100644 include/clocksource/hyperv_timer.h diff --git a/MAINTAINERS b/MAINTAINERS index 992f1dd..cf2a5b7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7294,6 +7294,7 @@ F: arch/x86/include/asm/trace/hyperv.h F: arch/x86/include/asm/hyperv-tlfs.h F: arch/x86/kernel/cpu/mshyperv.c F: arch/x86/hyperv +F: drivers/clocksource/hyperv_timer.c F: drivers/hid/hid-hyperv.c F: drivers/hv/ F: drivers/input/serio/hyperv-keyboard.c @@ -7304,6 +7305,7 @@ F: drivers/uio/uio_hv_generic.c F: drivers/video/fbdev/hyperv_fb.c F: drivers/iommu/hyperv_iommu.c F: net/vmw_vsock/hyperv_transport.c +F: include/clocksource/hyperv_timer.h F: include/linux/hyperv.h F: include/uapi/linux/hyperv.h F: tools/hv/ diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h index cdf44aa..af78cd7 100644 --- a/arch/x86/include/asm/hyperv-tlfs.h +++ b/arch/x86/include/asm/hyperv-tlfs.h @@ -401,6 +401,12 @@ enum HV_GENERIC_SET_FORMAT { #define HV_STATUS_INVALID_CONNECTION_ID 18 #define HV_STATUS_INSUFFICIENT_BUFFERS 19 +/* + * The Hyper-V TimeRefCount register and the TSC + * page provide a guest VM clock with 100ns tick rate + */ +#define HV_CLOCK_HZ (NSEC_PER_SEC/100) + typedef struct _HV_REFERENCE_TSC_PAGE { __u32 tsc_sequence; __u32 res1; diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index faae611..b18ede4 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -21,6 +21,7 @@ #include <linux/irq.h> #include <linux/kexec.h> #include <linux/i8253.h> +#include <linux/random.h> #include <asm/processor.h> #include <asm/hypervisor.h> #include <asm/hyperv-tlfs.h> @@ -84,6 +85,7 @@ __visible void __irq_entry hv_stimer0_vector_handler(struct pt_regs *regs) inc_irq_stat(hyperv_stimer0_count); if (hv_stimer0_handler) hv_stimer0_handler(); + add_interrupt_randomness(HYPERV_STIMER0_VECTOR, 0); ack_APIC_irq(); exiting_irq(); diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 236858f..2b65c5f 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o +obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c new file mode 100644 index 0000000..30615f3 --- /dev/null +++ b/drivers/clocksource/hyperv_timer.c @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Clocksource driver for the synthetic counter and timers + * provided by the Hyper-V hypervisor to guest VMs, as described + * in the Hyper-V Top Level Functional Spec (TLFS). This driver + * is instruction set architecture independent. + * + * Copyright (C) 2019, Microsoft, Inc. + * + * Author: Michael Kelley <mikelley@microsoft.com> + */ + +#include <linux/percpu.h> +#include <linux/cpumask.h> +#include <linux/clockchips.h> +#include <linux/mm.h> +#include <clocksource/hyperv_timer.h> +#include <asm/hyperv-tlfs.h> +#include <asm/mshyperv.h> + +static struct clock_event_device __percpu *hv_clock_event; + +/* + * If false, we're using the old mechanism for stimer0 interrupts + * where it sends a VMbus message when it expires. The old + * mechanism is used when running on older versions of Hyper-V + * that don't support Direct Mode. While Hyper-V provides + * four stimer's per CPU, Linux uses only stimer0. + */ +static bool direct_mode_enabled; + +static int stimer0_irq; +static int stimer0_vector; +static int stimer0_message_sint; + +/* + * ISR for when stimer0 is operating in Direct Mode. Direct Mode + * does not use VMbus or any VMbus messages, so process here and not + * in the VMbus driver code. + */ +void hv_stimer0_isr(void) +{ + struct clock_event_device *ce; + + ce = this_cpu_ptr(hv_clock_event); + ce->event_handler(ce); +} +EXPORT_SYMBOL_GPL(hv_stimer0_isr); + +static int hv_ce_set_next_event(unsigned long delta, + struct clock_event_device *evt) +{ + u64 current_tick; + + WARN_ON(!clockevent_state_oneshot(evt)); + + current_tick = hyperv_cs->read(NULL); + current_tick += delta; + hv_init_timer(0, current_tick); + return 0; +} + +static int hv_ce_shutdown(struct clock_event_device *evt) +{ + hv_init_timer(0, 0); + hv_init_timer_config(0, 0); + if (direct_mode_enabled) + hv_disable_stimer0_percpu_irq(stimer0_irq); + + return 0; +} + +static int hv_ce_set_oneshot(struct clock_event_device *evt) +{ + union hv_stimer_config timer_cfg; + + timer_cfg.as_uint64 = 0; + timer_cfg.enable = 1; + timer_cfg.auto_enable = 1; + if (direct_mode_enabled) { + /* + * When it expires, the timer will directly interrupt + * on the specified hardware vector/IRQ. + */ + timer_cfg.direct_mode = 1; + timer_cfg.apic_vector = stimer0_vector; + hv_enable_stimer0_percpu_irq(stimer0_irq); + } else { + /* + * When it expires, the timer will generate a VMbus message, + * to be handled by the normal VMbus interrupt handler. + */ + timer_cfg.direct_mode = 0; + timer_cfg.sintx = stimer0_message_sint; + } + hv_init_timer_config(0, timer_cfg.as_uint64); + return 0; +} + +/* + * hv_stimer_init - Per-cpu initialization of the clockevent + */ +int hv_stimer_init(unsigned int cpu) +{ + struct clock_event_device *ce; + + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { + ce = per_cpu_ptr(hv_clock_event, cpu); + ce->name = "Hyper-V clockevent"; + ce->features = CLOCK_EVT_FEAT_ONESHOT; + ce->cpumask = cpumask_of(cpu); + ce->rating = 1000; + ce->set_state_shutdown = hv_ce_shutdown; + ce->set_state_oneshot = hv_ce_set_oneshot; + ce->set_next_event = hv_ce_set_next_event; + + clockevents_config_and_register(ce, + HV_CLOCK_HZ, + HV_MIN_DELTA_TICKS, + HV_MAX_MAX_DELTA_TICKS); + } + return 0; +} +EXPORT_SYMBOL_GPL(hv_stimer_init); + +/* + * hv_stimer_cleanup - Per-cpu cleanup of the clockevent + */ +int hv_stimer_cleanup(unsigned int cpu) +{ + struct clock_event_device *ce; + + /* Turn off clockevent device */ + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { + ce = per_cpu_ptr(hv_clock_event, cpu); + clockevents_unbind_device(ce, cpu); + hv_ce_shutdown(ce); + } + + return 0; +} +EXPORT_SYMBOL_GPL(hv_stimer_cleanup); + +/* hv_stimer_alloc - Global initialization of the clockevent and stimer0 */ +int hv_stimer_alloc(int sint) +{ + hv_clock_event = alloc_percpu(struct clock_event_device); + if (!hv_clock_event) + return -ENOMEM; + + direct_mode_enabled = ms_hyperv.misc_features & + HV_STIMER_DIRECT_MODE_AVAILABLE; + if (direct_mode_enabled && + hv_setup_stimer0_irq(&stimer0_irq, &stimer0_vector, + hv_stimer0_isr)) { + free_percpu(hv_clock_event); + return -EINVAL; + } + + stimer0_message_sint = sint; + return 0; +} +EXPORT_SYMBOL_GPL(hv_stimer_alloc); + +/* hv_stimer_free - Free global resources allocated by hv_stimer_alloc() */ +void hv_stimer_free(void) +{ + if (direct_mode_enabled) + hv_remove_stimer0_irq(stimer0_irq); + free_percpu(hv_clock_event); +} +EXPORT_SYMBOL_GPL(hv_stimer_free); + +/* + * Do a global cleanup of clockevents for the cases of kexec and + * vmbus exit + */ +void hv_stimer_global_cleanup(void) +{ + int cpu; + struct clock_event_device *ce; + + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) + for_each_present_cpu(cpu) { + ce = per_cpu_ptr(hv_clock_event, cpu); + clockevents_unbind_device(ce, cpu); + } + hv_stimer_free(); +} +EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup); diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig index 1c1a251..c423e57 100644 --- a/drivers/hv/Kconfig +++ b/drivers/hv/Kconfig @@ -10,6 +10,9 @@ config HYPERV Select this option to run Linux as a Hyper-V client operating system. +config HYPERV_TIMER + def_bool HYPERV + config HYPERV_TSCPAGE def_bool HYPERV && X86_64 diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index 4565302..bd6452f 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -29,6 +29,7 @@ #include <linux/version.h> #include <linux/random.h> #include <linux/clockchips.h> +#include <clocksource/hyperv_timer.h> #include <asm/mshyperv.h> #include "hyperv_vmbus.h" @@ -36,21 +37,6 @@ struct hv_context hv_context; /* - * If false, we're using the old mechanism for stimer0 interrupts - * where it sends a VMbus message when it expires. The old - * mechanism is used when running on older versions of Hyper-V - * that don't support Direct Mode. While Hyper-V provides - * four stimer's per CPU, Linux uses only stimer0. - */ -static bool direct_mode_enabled; -static int stimer0_irq; -static int stimer0_vector; - -#define HV_TIMER_FREQUENCY (10 * 1000 * 1000) /* 100ns period */ -#define HV_MAX_MAX_DELTA_TICKS 0xffffffff -#define HV_MIN_DELTA_TICKS 1 - -/* * hv_init - Main initialization routine. * * This routine must be called before any other routines in here are called @@ -60,9 +46,6 @@ int hv_init(void) hv_context.cpu_context = alloc_percpu(struct hv_per_cpu_context); if (!hv_context.cpu_context) return -ENOMEM; - - direct_mode_enabled = ms_hyperv.misc_features & - HV_STIMER_DIRECT_MODE_AVAILABLE; return 0; } @@ -101,89 +84,6 @@ int hv_post_message(union hv_connection_id connection_id, return status & 0xFFFF; } -/* - * ISR for when stimer0 is operating in Direct Mode. Direct Mode - * does not use VMbus or any VMbus messages, so process here and not - * in the VMbus driver code. - */ - -static void hv_stimer0_isr(void) -{ - struct hv_per_cpu_context *hv_cpu; - - hv_cpu = this_cpu_ptr(hv_context.cpu_context); - hv_cpu->clk_evt->event_handler(hv_cpu->clk_evt); - add_interrupt_randomness(stimer0_vector, 0); -} - -static int hv_ce_set_next_event(unsigned long delta, - struct clock_event_device *evt) -{ - u64 current_tick; - - WARN_ON(!clockevent_state_oneshot(evt)); - - current_tick = hyperv_cs->read(NULL); - current_tick += delta; - hv_init_timer(0, current_tick); - return 0; -} - -static int hv_ce_shutdown(struct clock_event_device *evt) -{ - hv_init_timer(0, 0); - hv_init_timer_config(0, 0); - if (direct_mode_enabled) - hv_disable_stimer0_percpu_irq(stimer0_irq); - - return 0; -} - -static int hv_ce_set_oneshot(struct clock_event_device *evt) -{ - union hv_stimer_config timer_cfg; - - timer_cfg.as_uint64 = 0; - timer_cfg.enable = 1; - timer_cfg.auto_enable = 1; - if (direct_mode_enabled) { - /* - * When it expires, the timer will directly interrupt - * on the specified hardware vector/IRQ. - */ - timer_cfg.direct_mode = 1; - timer_cfg.apic_vector = stimer0_vector; - hv_enable_stimer0_percpu_irq(stimer0_irq); - } else { - /* - * When it expires, the timer will generate a VMbus message, - * to be handled by the normal VMbus interrupt handler. - */ - timer_cfg.direct_mode = 0; - timer_cfg.sintx = VMBUS_MESSAGE_SINT; - } - hv_init_timer_config(0, timer_cfg.as_uint64); - return 0; -} - -static void hv_init_clockevent_device(struct clock_event_device *dev, int cpu) -{ - dev->name = "Hyper-V clockevent"; - dev->features = CLOCK_EVT_FEAT_ONESHOT; - dev->cpumask = cpumask_of(cpu); - dev->rating = 1000; - /* - * Avoid settint dev->owner = THIS_MODULE deliberately as doing so will - * result in clockevents_config_and_register() taking additional - * references to the hv_vmbus module making it impossible to unload. - */ - - dev->set_state_shutdown = hv_ce_shutdown; - dev->set_state_oneshot = hv_ce_set_oneshot; - dev->set_next_event = hv_ce_set_next_event; -} - - int hv_synic_alloc(void) { int cpu; @@ -212,14 +112,6 @@ int hv_synic_alloc(void) tasklet_init(&hv_cpu->msg_dpc, vmbus_on_msg_dpc, (unsigned long) hv_cpu); - hv_cpu->clk_evt = kzalloc(sizeof(struct clock_event_device), - GFP_KERNEL); - if (hv_cpu->clk_evt == NULL) { - pr_err("Unable to allocate clock event device\n"); - goto err; - } - hv_init_clockevent_device(hv_cpu->clk_evt, cpu); - hv_cpu->synic_message_page = (void *)get_zeroed_page(GFP_ATOMIC); if (hv_cpu->synic_message_page == NULL) { @@ -242,11 +134,6 @@ int hv_synic_alloc(void) INIT_LIST_HEAD(&hv_cpu->chan_list); } - if (direct_mode_enabled && - hv_setup_stimer0_irq(&stimer0_irq, &stimer0_vector, - hv_stimer0_isr)) - goto err; - return 0; err: /* @@ -265,7 +152,6 @@ void hv_synic_free(void) struct hv_per_cpu_context *hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu); - kfree(hv_cpu->clk_evt); free_page((unsigned long)hv_cpu->synic_event_page); free_page((unsigned long)hv_cpu->synic_message_page); free_page((unsigned long)hv_cpu->post_msg_page); @@ -324,36 +210,9 @@ int hv_synic_init(unsigned int cpu) hv_set_synic_state(sctrl.as_uint64); - /* - * Register the per-cpu clockevent source. - */ - if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) - clockevents_config_and_register(hv_cpu->clk_evt, - HV_TIMER_FREQUENCY, - HV_MIN_DELTA_TICKS, - HV_MAX_MAX_DELTA_TICKS); - return 0; -} - -/* - * hv_synic_clockevents_cleanup - Cleanup clockevent devices - */ -void hv_synic_clockevents_cleanup(void) -{ - int cpu; + hv_stimer_init(cpu); - if (!(ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE)) - return; - - if (direct_mode_enabled) - hv_remove_stimer0_irq(stimer0_irq); - - for_each_present_cpu(cpu) { - struct hv_per_cpu_context *hv_cpu - = per_cpu_ptr(hv_context.cpu_context, cpu); - - clockevents_unbind_device(hv_cpu->clk_evt, cpu); - } + return 0; } /* @@ -401,14 +260,7 @@ int hv_synic_cleanup(unsigned int cpu) if (channel_found && vmbus_connection.conn_state == CONNECTED) return -EBUSY; - /* Turn off clockevent device */ - if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { - struct hv_per_cpu_context *hv_cpu - = this_cpu_ptr(hv_context.cpu_context); - - clockevents_unbind_device(hv_cpu->clk_evt, cpu); - hv_ce_shutdown(hv_cpu->clk_evt); - } + hv_stimer_cleanup(cpu); hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64); diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index e5467b8..a8f8aee 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -151,7 +151,6 @@ struct hv_per_cpu_context { * per-cpu list of the channels based on their CPU affinity. */ struct list_head chan_list; - struct clock_event_device *clk_evt; }; struct hv_context { @@ -189,8 +188,6 @@ extern int hv_post_message(union hv_connection_id connection_id, extern int hv_synic_cleanup(unsigned int cpu); -extern void hv_synic_clockevents_cleanup(void); - /* Interface */ void hv_ringbuffer_pre_init(struct vmbus_channel *channel); diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 1cb9408..89aca5d 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -43,6 +43,7 @@ #include <linux/kdebug.h> #include <linux/efi.h> #include <linux/random.h> +#include <clocksource/hyperv_timer.h> #include "hyperv_vmbus.h" struct vmbus_dynid { @@ -968,17 +969,6 @@ static void vmbus_onmessage_work(struct work_struct *work) kfree(ctx); } -static void hv_process_timer_expiration(struct hv_message *msg, - struct hv_per_cpu_context *hv_cpu) -{ - struct clock_event_device *dev = hv_cpu->clk_evt; - - if (dev->event_handler) - dev->event_handler(dev); - - vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED); -} - void vmbus_on_msg_dpc(unsigned long data) { struct hv_per_cpu_context *hv_cpu = (void *)data; @@ -1172,9 +1162,10 @@ static void vmbus_isr(void) /* Check if there are actual msgs to be processed */ if (msg->header.message_type != HVMSG_NONE) { - if (msg->header.message_type == HVMSG_TIMER_EXPIRED) - hv_process_timer_expiration(msg, hv_cpu); - else + if (msg->header.message_type == HVMSG_TIMER_EXPIRED) { + hv_stimer0_isr(); + vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED); + } else tasklet_schedule(&hv_cpu->msg_dpc); } @@ -1276,14 +1267,19 @@ static int vmbus_bus_init(void) ret = hv_synic_alloc(); if (ret) goto err_alloc; + + ret = hv_stimer_alloc(VMBUS_MESSAGE_SINT); + if (ret < 0) + goto err_alloc; + /* - * Initialize the per-cpu interrupt state and - * connect to the host. + * Initialize the per-cpu interrupt state and stimer state. + * Then connect to the host. */ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online", hv_synic_init, hv_synic_cleanup); if (ret < 0) - goto err_alloc; + goto err_cpuhp; hyperv_cpuhp_online = ret; ret = vmbus_connect(); @@ -1331,6 +1327,8 @@ static int vmbus_bus_init(void) err_connect: cpuhp_remove_state(hyperv_cpuhp_online); +err_cpuhp: + hv_stimer_free(); err_alloc: hv_synic_free(); hv_remove_vmbus_irq(); @@ -2077,7 +2075,7 @@ static int vmbus_acpi_add(struct acpi_device *device) static void hv_kexec_handler(void) { - hv_synic_clockevents_cleanup(); + hv_stimer_global_cleanup(); vmbus_initiate_unload(false); vmbus_connection.conn_state = DISCONNECTED; /* Make sure conn_state is set as hv_synic_cleanup checks for it */ @@ -2088,6 +2086,8 @@ static void hv_kexec_handler(void) static void hv_crash_handler(struct pt_regs *regs) { + int cpu; + vmbus_initiate_unload(true); /* * In crash handler we can't schedule synic cleanup for all CPUs, @@ -2095,7 +2095,9 @@ static void hv_crash_handler(struct pt_regs *regs) * for kdump. */ vmbus_connection.conn_state = DISCONNECTED; - hv_synic_cleanup(smp_processor_id()); + cpu = smp_processor_id(); + hv_stimer_cleanup(cpu); + hv_synic_cleanup(cpu); hyperv_cleanup(); }; @@ -2144,7 +2146,7 @@ static void __exit vmbus_exit(void) hv_remove_kexec_handler(); hv_remove_crash_handler(); vmbus_connection.conn_state = DISCONNECTED; - hv_synic_clockevents_cleanup(); + hv_stimer_global_cleanup(); vmbus_disconnect(); hv_remove_vmbus_irq(); for_each_online_cpu(cpu) { diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h new file mode 100644 index 0000000..ba5dc17 --- /dev/null +++ b/include/clocksource/hyperv_timer.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Definitions for the clocksource provided by the Hyper-V + * hypervisor to guest VMs, as described in the Hyper-V Top + * Level Functional Spec (TLFS). + * + * Copyright (C) 2019, Microsoft, Inc. + * + * Author: Michael Kelley <mikelley@microsoft.com> + */ + +#ifndef __CLKSOURCE_HYPERV_TIMER_H +#define __CLKSOURCE_HYPERV_TIMER_H + +#define HV_MAX_MAX_DELTA_TICKS 0xffffffff +#define HV_MIN_DELTA_TICKS 1 + +/* Routines called by the VMbus driver */ +extern int hv_stimer_alloc(int sint); +extern void hv_stimer_free(void); +extern int hv_stimer_init(unsigned int cpu); +extern int hv_stimer_cleanup(unsigned int cpu); +extern void hv_stimer_global_cleanup(void); +extern void hv_stimer0_isr(void); + +#endif -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code 2019-05-27 14:59 ` [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Michael Kelley @ 2019-05-30 9:37 ` Vitaly Kuznetsov 0 siblings, 0 replies; 8+ messages in thread From: Vitaly Kuznetsov @ 2019-05-30 9:37 UTC (permalink / raw) To: Michael Kelley Cc: catalin.marinas, mark.rutland, will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Michael Kelley <mikelley@microsoft.com> writes: > Clockevents code for Hyper-V synthetic timers is currently mixed > in with other Hyper-V code. Move the code to a Hyper-V specific > driver in the "clocksource" directory. Update the VMbus driver > to call initialization and cleanup routines since the Hyper-V > synthetic timers are not independently enumerated in ACPI. > > No behavior is changed and no new functionality is added. > > Suggested-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > --- > MAINTAINERS | 2 + > arch/x86/include/asm/hyperv-tlfs.h | 6 ++ > arch/x86/kernel/cpu/mshyperv.c | 2 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/hyperv_timer.c | 191 +++++++++++++++++++++++++++++++++++++ > drivers/hv/Kconfig | 3 + > drivers/hv/hv.c | 156 +----------------------------- > drivers/hv/hyperv_vmbus.h | 3 - > drivers/hv/vmbus_drv.c | 42 ++++---- > include/clocksource/hyperv_timer.h | 27 ++++++ > 10 files changed, 258 insertions(+), 175 deletions(-) > create mode 100644 drivers/clocksource/hyperv_timer.c > create mode 100644 include/clocksource/hyperv_timer.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 992f1dd..cf2a5b7 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7294,6 +7294,7 @@ F: arch/x86/include/asm/trace/hyperv.h > F: arch/x86/include/asm/hyperv-tlfs.h > F: arch/x86/kernel/cpu/mshyperv.c > F: arch/x86/hyperv > +F: drivers/clocksource/hyperv_timer.c > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > @@ -7304,6 +7305,7 @@ F: drivers/uio/uio_hv_generic.c > F: drivers/video/fbdev/hyperv_fb.c > F: drivers/iommu/hyperv_iommu.c > F: net/vmw_vsock/hyperv_transport.c > +F: include/clocksource/hyperv_timer.h > F: include/linux/hyperv.h > F: include/uapi/linux/hyperv.h > F: tools/hv/ > diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h > index cdf44aa..af78cd7 100644 > --- a/arch/x86/include/asm/hyperv-tlfs.h > +++ b/arch/x86/include/asm/hyperv-tlfs.h > @@ -401,6 +401,12 @@ enum HV_GENERIC_SET_FORMAT { > #define HV_STATUS_INVALID_CONNECTION_ID 18 > #define HV_STATUS_INSUFFICIENT_BUFFERS 19 > > +/* > + * The Hyper-V TimeRefCount register and the TSC > + * page provide a guest VM clock with 100ns tick rate > + */ > +#define HV_CLOCK_HZ (NSEC_PER_SEC/100) > + > typedef struct _HV_REFERENCE_TSC_PAGE { > __u32 tsc_sequence; > __u32 res1; > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c > index faae611..b18ede4 100644 > --- a/arch/x86/kernel/cpu/mshyperv.c > +++ b/arch/x86/kernel/cpu/mshyperv.c > @@ -21,6 +21,7 @@ > #include <linux/irq.h> > #include <linux/kexec.h> > #include <linux/i8253.h> > +#include <linux/random.h> > #include <asm/processor.h> > #include <asm/hypervisor.h> > #include <asm/hyperv-tlfs.h> > @@ -84,6 +85,7 @@ __visible void __irq_entry hv_stimer0_vector_handler(struct pt_regs *regs) > inc_irq_stat(hyperv_stimer0_count); > if (hv_stimer0_handler) > hv_stimer0_handler(); > + add_interrupt_randomness(HYPERV_STIMER0_VECTOR, 0); > ack_APIC_irq(); > > exiting_irq(); > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > index 236858f..2b65c5f 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -84,3 +84,4 @@ obj-$(CONFIG_ATCPIT100_TIMER) += timer-atcpit100.o > obj-$(CONFIG_RISCV_TIMER) += timer-riscv.o > obj-$(CONFIG_CSKY_MP_TIMER) += timer-mp-csky.o > obj-$(CONFIG_GX6605S_TIMER) += timer-gx6605s.o > +obj-$(CONFIG_HYPERV_TIMER) += hyperv_timer.o > diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c > new file mode 100644 > index 0000000..30615f3 > --- /dev/null > +++ b/drivers/clocksource/hyperv_timer.c > @@ -0,0 +1,191 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * Clocksource driver for the synthetic counter and timers > + * provided by the Hyper-V hypervisor to guest VMs, as described > + * in the Hyper-V Top Level Functional Spec (TLFS). This driver > + * is instruction set architecture independent. > + * > + * Copyright (C) 2019, Microsoft, Inc. > + * > + * Author: Michael Kelley <mikelley@microsoft.com> > + */ > + > +#include <linux/percpu.h> > +#include <linux/cpumask.h> > +#include <linux/clockchips.h> > +#include <linux/mm.h> > +#include <clocksource/hyperv_timer.h> > +#include <asm/hyperv-tlfs.h> > +#include <asm/mshyperv.h> > + > +static struct clock_event_device __percpu *hv_clock_event; > + > +/* > + * If false, we're using the old mechanism for stimer0 interrupts > + * where it sends a VMbus message when it expires. The old > + * mechanism is used when running on older versions of Hyper-V > + * that don't support Direct Mode. While Hyper-V provides > + * four stimer's per CPU, Linux uses only stimer0. > + */ > +static bool direct_mode_enabled; > + > +static int stimer0_irq; > +static int stimer0_vector; > +static int stimer0_message_sint; > + > +/* > + * ISR for when stimer0 is operating in Direct Mode. Direct Mode > + * does not use VMbus or any VMbus messages, so process here and not > + * in the VMbus driver code. > + */ > +void hv_stimer0_isr(void) > +{ > + struct clock_event_device *ce; > + > + ce = this_cpu_ptr(hv_clock_event); > + ce->event_handler(ce); > +} > +EXPORT_SYMBOL_GPL(hv_stimer0_isr); > + > +static int hv_ce_set_next_event(unsigned long delta, > + struct clock_event_device *evt) > +{ > + u64 current_tick; > + > + WARN_ON(!clockevent_state_oneshot(evt)); > + > + current_tick = hyperv_cs->read(NULL); > + current_tick += delta; > + hv_init_timer(0, current_tick); > + return 0; > +} > + > +static int hv_ce_shutdown(struct clock_event_device *evt) > +{ > + hv_init_timer(0, 0); > + hv_init_timer_config(0, 0); > + if (direct_mode_enabled) > + hv_disable_stimer0_percpu_irq(stimer0_irq); > + > + return 0; > +} > + > +static int hv_ce_set_oneshot(struct clock_event_device *evt) > +{ > + union hv_stimer_config timer_cfg; > + > + timer_cfg.as_uint64 = 0; > + timer_cfg.enable = 1; > + timer_cfg.auto_enable = 1; > + if (direct_mode_enabled) { > + /* > + * When it expires, the timer will directly interrupt > + * on the specified hardware vector/IRQ. > + */ > + timer_cfg.direct_mode = 1; > + timer_cfg.apic_vector = stimer0_vector; > + hv_enable_stimer0_percpu_irq(stimer0_irq); > + } else { > + /* > + * When it expires, the timer will generate a VMbus message, > + * to be handled by the normal VMbus interrupt handler. > + */ > + timer_cfg.direct_mode = 0; > + timer_cfg.sintx = stimer0_message_sint; > + } > + hv_init_timer_config(0, timer_cfg.as_uint64); > + return 0; > +} > + > +/* > + * hv_stimer_init - Per-cpu initialization of the clockevent > + */ > +int hv_stimer_init(unsigned int cpu) > +{ > + struct clock_event_device *ce; > + > + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { > + ce = per_cpu_ptr(hv_clock_event, cpu); > + ce->name = "Hyper-V clockevent"; > + ce->features = CLOCK_EVT_FEAT_ONESHOT; > + ce->cpumask = cpumask_of(cpu); > + ce->rating = 1000; > + ce->set_state_shutdown = hv_ce_shutdown; > + ce->set_state_oneshot = hv_ce_set_oneshot; > + ce->set_next_event = hv_ce_set_next_event; > + > + clockevents_config_and_register(ce, > + HV_CLOCK_HZ, > + HV_MIN_DELTA_TICKS, > + HV_MAX_MAX_DELTA_TICKS); > + } > + return 0; > +} > +EXPORT_SYMBOL_GPL(hv_stimer_init); > + > +/* > + * hv_stimer_cleanup - Per-cpu cleanup of the clockevent > + */ > +int hv_stimer_cleanup(unsigned int cpu) > +{ > + struct clock_event_device *ce; > + > + /* Turn off clockevent device */ > + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { > + ce = per_cpu_ptr(hv_clock_event, cpu); > + clockevents_unbind_device(ce, cpu); > + hv_ce_shutdown(ce); > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(hv_stimer_cleanup); > + > +/* hv_stimer_alloc - Global initialization of the clockevent and stimer0 */ > +int hv_stimer_alloc(int sint) > +{ > + hv_clock_event = alloc_percpu(struct clock_event_device); > + if (!hv_clock_event) > + return -ENOMEM; > + > + direct_mode_enabled = ms_hyperv.misc_features & > + HV_STIMER_DIRECT_MODE_AVAILABLE; > + if (direct_mode_enabled && > + hv_setup_stimer0_irq(&stimer0_irq, &stimer0_vector, > + hv_stimer0_isr)) { > + free_percpu(hv_clock_event); > + return -EINVAL; > + } > + > + stimer0_message_sint = sint; > + return 0; > +} > +EXPORT_SYMBOL_GPL(hv_stimer_alloc); > + > +/* hv_stimer_free - Free global resources allocated by hv_stimer_alloc() */ > +void hv_stimer_free(void) > +{ > + if (direct_mode_enabled) > + hv_remove_stimer0_irq(stimer0_irq); > + free_percpu(hv_clock_event); > +} > +EXPORT_SYMBOL_GPL(hv_stimer_free); > + > +/* > + * Do a global cleanup of clockevents for the cases of kexec and > + * vmbus exit > + */ > +void hv_stimer_global_cleanup(void) > +{ > + int cpu; > + struct clock_event_device *ce; > + > + if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) > + for_each_present_cpu(cpu) { > + ce = per_cpu_ptr(hv_clock_event, cpu); > + clockevents_unbind_device(ce, cpu); > + } > + hv_stimer_free(); > +} > +EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup); > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig > index 1c1a251..c423e57 100644 > --- a/drivers/hv/Kconfig > +++ b/drivers/hv/Kconfig > @@ -10,6 +10,9 @@ config HYPERV > Select this option to run Linux as a Hyper-V client operating > system. > > +config HYPERV_TIMER > + def_bool HYPERV > + > config HYPERV_TSCPAGE > def_bool HYPERV && X86_64 > > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > index 4565302..bd6452f 100644 > --- a/drivers/hv/hv.c > +++ b/drivers/hv/hv.c > @@ -29,6 +29,7 @@ > #include <linux/version.h> > #include <linux/random.h> > #include <linux/clockchips.h> (very minor nit, just an idea for future cleanup): I think we can throw away #include <linux/random.h> #include <linux/clockchips.h> now. > +#include <clocksource/hyperv_timer.h> > #include <asm/mshyperv.h> > #include "hyperv_vmbus.h" > > @@ -36,21 +37,6 @@ > struct hv_context hv_context; > > /* > - * If false, we're using the old mechanism for stimer0 interrupts > - * where it sends a VMbus message when it expires. The old > - * mechanism is used when running on older versions of Hyper-V > - * that don't support Direct Mode. While Hyper-V provides > - * four stimer's per CPU, Linux uses only stimer0. > - */ > -static bool direct_mode_enabled; > -static int stimer0_irq; > -static int stimer0_vector; > - > -#define HV_TIMER_FREQUENCY (10 * 1000 * 1000) /* 100ns period */ > -#define HV_MAX_MAX_DELTA_TICKS 0xffffffff > -#define HV_MIN_DELTA_TICKS 1 > - > -/* > * hv_init - Main initialization routine. > * > * This routine must be called before any other routines in here are called > @@ -60,9 +46,6 @@ int hv_init(void) > hv_context.cpu_context = alloc_percpu(struct hv_per_cpu_context); > if (!hv_context.cpu_context) > return -ENOMEM; > - > - direct_mode_enabled = ms_hyperv.misc_features & > - HV_STIMER_DIRECT_MODE_AVAILABLE; > return 0; > } > > @@ -101,89 +84,6 @@ int hv_post_message(union hv_connection_id connection_id, > return status & 0xFFFF; > } > > -/* > - * ISR for when stimer0 is operating in Direct Mode. Direct Mode > - * does not use VMbus or any VMbus messages, so process here and not > - * in the VMbus driver code. > - */ > - > -static void hv_stimer0_isr(void) > -{ > - struct hv_per_cpu_context *hv_cpu; > - > - hv_cpu = this_cpu_ptr(hv_context.cpu_context); > - hv_cpu->clk_evt->event_handler(hv_cpu->clk_evt); > - add_interrupt_randomness(stimer0_vector, 0); > -} > - > -static int hv_ce_set_next_event(unsigned long delta, > - struct clock_event_device *evt) > -{ > - u64 current_tick; > - > - WARN_ON(!clockevent_state_oneshot(evt)); > - > - current_tick = hyperv_cs->read(NULL); > - current_tick += delta; > - hv_init_timer(0, current_tick); > - return 0; > -} > - > -static int hv_ce_shutdown(struct clock_event_device *evt) > -{ > - hv_init_timer(0, 0); > - hv_init_timer_config(0, 0); > - if (direct_mode_enabled) > - hv_disable_stimer0_percpu_irq(stimer0_irq); > - > - return 0; > -} > - > -static int hv_ce_set_oneshot(struct clock_event_device *evt) > -{ > - union hv_stimer_config timer_cfg; > - > - timer_cfg.as_uint64 = 0; > - timer_cfg.enable = 1; > - timer_cfg.auto_enable = 1; > - if (direct_mode_enabled) { > - /* > - * When it expires, the timer will directly interrupt > - * on the specified hardware vector/IRQ. > - */ > - timer_cfg.direct_mode = 1; > - timer_cfg.apic_vector = stimer0_vector; > - hv_enable_stimer0_percpu_irq(stimer0_irq); > - } else { > - /* > - * When it expires, the timer will generate a VMbus message, > - * to be handled by the normal VMbus interrupt handler. > - */ > - timer_cfg.direct_mode = 0; > - timer_cfg.sintx = VMBUS_MESSAGE_SINT; > - } > - hv_init_timer_config(0, timer_cfg.as_uint64); > - return 0; > -} > - > -static void hv_init_clockevent_device(struct clock_event_device *dev, int cpu) > -{ > - dev->name = "Hyper-V clockevent"; > - dev->features = CLOCK_EVT_FEAT_ONESHOT; > - dev->cpumask = cpumask_of(cpu); > - dev->rating = 1000; > - /* > - * Avoid settint dev->owner = THIS_MODULE deliberately as doing so will > - * result in clockevents_config_and_register() taking additional > - * references to the hv_vmbus module making it impossible to unload. > - */ > - > - dev->set_state_shutdown = hv_ce_shutdown; > - dev->set_state_oneshot = hv_ce_set_oneshot; > - dev->set_next_event = hv_ce_set_next_event; > -} > - > - > int hv_synic_alloc(void) > { > int cpu; > @@ -212,14 +112,6 @@ int hv_synic_alloc(void) > tasklet_init(&hv_cpu->msg_dpc, > vmbus_on_msg_dpc, (unsigned long) hv_cpu); > > - hv_cpu->clk_evt = kzalloc(sizeof(struct clock_event_device), > - GFP_KERNEL); > - if (hv_cpu->clk_evt == NULL) { > - pr_err("Unable to allocate clock event device\n"); > - goto err; > - } > - hv_init_clockevent_device(hv_cpu->clk_evt, cpu); > - > hv_cpu->synic_message_page = > (void *)get_zeroed_page(GFP_ATOMIC); > if (hv_cpu->synic_message_page == NULL) { > @@ -242,11 +134,6 @@ int hv_synic_alloc(void) > INIT_LIST_HEAD(&hv_cpu->chan_list); > } > > - if (direct_mode_enabled && > - hv_setup_stimer0_irq(&stimer0_irq, &stimer0_vector, > - hv_stimer0_isr)) > - goto err; > - > return 0; > err: > /* > @@ -265,7 +152,6 @@ void hv_synic_free(void) > struct hv_per_cpu_context *hv_cpu > = per_cpu_ptr(hv_context.cpu_context, cpu); > > - kfree(hv_cpu->clk_evt); > free_page((unsigned long)hv_cpu->synic_event_page); > free_page((unsigned long)hv_cpu->synic_message_page); > free_page((unsigned long)hv_cpu->post_msg_page); > @@ -324,36 +210,9 @@ int hv_synic_init(unsigned int cpu) > > hv_set_synic_state(sctrl.as_uint64); > > - /* > - * Register the per-cpu clockevent source. > - */ > - if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) > - clockevents_config_and_register(hv_cpu->clk_evt, > - HV_TIMER_FREQUENCY, > - HV_MIN_DELTA_TICKS, > - HV_MAX_MAX_DELTA_TICKS); > - return 0; > -} > - > -/* > - * hv_synic_clockevents_cleanup - Cleanup clockevent devices > - */ > -void hv_synic_clockevents_cleanup(void) > -{ > - int cpu; > + hv_stimer_init(cpu); > > - if (!(ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE)) > - return; > - > - if (direct_mode_enabled) > - hv_remove_stimer0_irq(stimer0_irq); > - > - for_each_present_cpu(cpu) { > - struct hv_per_cpu_context *hv_cpu > - = per_cpu_ptr(hv_context.cpu_context, cpu); > - > - clockevents_unbind_device(hv_cpu->clk_evt, cpu); > - } > + return 0; > } > > /* > @@ -401,14 +260,7 @@ int hv_synic_cleanup(unsigned int cpu) > if (channel_found && vmbus_connection.conn_state == CONNECTED) > return -EBUSY; > > - /* Turn off clockevent device */ > - if (ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE) { > - struct hv_per_cpu_context *hv_cpu > - = this_cpu_ptr(hv_context.cpu_context); > - > - clockevents_unbind_device(hv_cpu->clk_evt, cpu); > - hv_ce_shutdown(hv_cpu->clk_evt); > - } > + hv_stimer_cleanup(cpu); > > hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64); > > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > index e5467b8..a8f8aee 100644 > --- a/drivers/hv/hyperv_vmbus.h > +++ b/drivers/hv/hyperv_vmbus.h > @@ -151,7 +151,6 @@ struct hv_per_cpu_context { > * per-cpu list of the channels based on their CPU affinity. > */ > struct list_head chan_list; > - struct clock_event_device *clk_evt; > }; > > struct hv_context { > @@ -189,8 +188,6 @@ extern int hv_post_message(union hv_connection_id connection_id, > > extern int hv_synic_cleanup(unsigned int cpu); > > -extern void hv_synic_clockevents_cleanup(void); > - > /* Interface */ > > void hv_ringbuffer_pre_init(struct vmbus_channel *channel); > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 1cb9408..89aca5d 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -43,6 +43,7 @@ > #include <linux/kdebug.h> > #include <linux/efi.h> > #include <linux/random.h> > +#include <clocksource/hyperv_timer.h> > #include "hyperv_vmbus.h" > > struct vmbus_dynid { > @@ -968,17 +969,6 @@ static void vmbus_onmessage_work(struct work_struct *work) > kfree(ctx); > } > > -static void hv_process_timer_expiration(struct hv_message *msg, > - struct hv_per_cpu_context *hv_cpu) > -{ > - struct clock_event_device *dev = hv_cpu->clk_evt; > - > - if (dev->event_handler) > - dev->event_handler(dev); > - > - vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED); > -} > - > void vmbus_on_msg_dpc(unsigned long data) > { > struct hv_per_cpu_context *hv_cpu = (void *)data; > @@ -1172,9 +1162,10 @@ static void vmbus_isr(void) > > /* Check if there are actual msgs to be processed */ > if (msg->header.message_type != HVMSG_NONE) { > - if (msg->header.message_type == HVMSG_TIMER_EXPIRED) > - hv_process_timer_expiration(msg, hv_cpu); > - else > + if (msg->header.message_type == HVMSG_TIMER_EXPIRED) { > + hv_stimer0_isr(); > + vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED); > + } else > tasklet_schedule(&hv_cpu->msg_dpc); > } > > @@ -1276,14 +1267,19 @@ static int vmbus_bus_init(void) > ret = hv_synic_alloc(); > if (ret) > goto err_alloc; > + > + ret = hv_stimer_alloc(VMBUS_MESSAGE_SINT); > + if (ret < 0) > + goto err_alloc; > + > /* > - * Initialize the per-cpu interrupt state and > - * connect to the host. > + * Initialize the per-cpu interrupt state and stimer state. > + * Then connect to the host. > */ > ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online", > hv_synic_init, hv_synic_cleanup); > if (ret < 0) > - goto err_alloc; > + goto err_cpuhp; > hyperv_cpuhp_online = ret; > > ret = vmbus_connect(); > @@ -1331,6 +1327,8 @@ static int vmbus_bus_init(void) > > err_connect: > cpuhp_remove_state(hyperv_cpuhp_online); > +err_cpuhp: > + hv_stimer_free(); > err_alloc: > hv_synic_free(); > hv_remove_vmbus_irq(); > @@ -2077,7 +2075,7 @@ static int vmbus_acpi_add(struct acpi_device *device) > > static void hv_kexec_handler(void) > { > - hv_synic_clockevents_cleanup(); > + hv_stimer_global_cleanup(); > vmbus_initiate_unload(false); > vmbus_connection.conn_state = DISCONNECTED; > /* Make sure conn_state is set as hv_synic_cleanup checks for it */ > @@ -2088,6 +2086,8 @@ static void hv_kexec_handler(void) > > static void hv_crash_handler(struct pt_regs *regs) > { > + int cpu; > + > vmbus_initiate_unload(true); > /* > * In crash handler we can't schedule synic cleanup for all CPUs, > @@ -2095,7 +2095,9 @@ static void hv_crash_handler(struct pt_regs *regs) > * for kdump. > */ > vmbus_connection.conn_state = DISCONNECTED; > - hv_synic_cleanup(smp_processor_id()); > + cpu = smp_processor_id(); > + hv_stimer_cleanup(cpu); > + hv_synic_cleanup(cpu); > hyperv_cleanup(); > }; > > @@ -2144,7 +2146,7 @@ static void __exit vmbus_exit(void) > hv_remove_kexec_handler(); > hv_remove_crash_handler(); > vmbus_connection.conn_state = DISCONNECTED; > - hv_synic_clockevents_cleanup(); > + hv_stimer_global_cleanup(); > vmbus_disconnect(); > hv_remove_vmbus_irq(); > for_each_online_cpu(cpu) { > diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h > new file mode 100644 > index 0000000..ba5dc17 > --- /dev/null > +++ b/include/clocksource/hyperv_timer.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * Definitions for the clocksource provided by the Hyper-V > + * hypervisor to guest VMs, as described in the Hyper-V Top > + * Level Functional Spec (TLFS). > + * > + * Copyright (C) 2019, Microsoft, Inc. > + * > + * Author: Michael Kelley <mikelley@microsoft.com> > + */ > + > +#ifndef __CLKSOURCE_HYPERV_TIMER_H > +#define __CLKSOURCE_HYPERV_TIMER_H > + > +#define HV_MAX_MAX_DELTA_TICKS 0xffffffff > +#define HV_MIN_DELTA_TICKS 1 > + > +/* Routines called by the VMbus driver */ > +extern int hv_stimer_alloc(int sint); > +extern void hv_stimer_free(void); > +extern int hv_stimer_init(unsigned int cpu); > +extern int hv_stimer_cleanup(unsigned int cpu); > +extern void hv_stimer_global_cleanup(void); > +extern void hv_stimer0_isr(void); > + > +#endif Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> -- Vitaly ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver 2019-05-27 14:59 [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver Michael Kelley 2019-05-27 14:59 ` [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Michael Kelley @ 2019-05-27 14:59 ` Michael Kelley 2019-05-30 9:48 ` Vitaly Kuznetsov 2019-06-06 16:37 ` [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate " Sasha Levin 2 siblings, 1 reply; 8+ messages in thread From: Michael Kelley @ 2019-05-27 14:59 UTC (permalink / raw) To: will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, vkuznets, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Cc: Michael Kelley, catalin.marinas, mark.rutland Code for the Hyper-V specific clocksources is currently mixed in with other Hyper-V code. Move the code to the Hyper-V specific driver in the "clocksource" directory, while separating out ISA dependencies so that the clocksource driver remains ISA independent. Update the Hyper-V initialization code to call initialization and cleanup routines since the Hyper-V synthetic timers are not independently enumerated in ACPI. Update Hyper-V clocksource users KVM and VDSO to get definitions from a new include file. No behavior is changed and no new functionality is added. Suggested-by: Marc Zyngier <marc.zyngier@arm.com> Signed-off-by: Michael Kelley <mikelley@microsoft.com> --- arch/x86/entry/vdso/vclock_gettime.c | 1 + arch/x86/entry/vdso/vma.c | 2 +- arch/x86/hyperv/hv_init.c | 91 +----------------------- arch/x86/include/asm/mshyperv.h | 81 +++------------------- arch/x86/kvm/x86.c | 1 + drivers/clocksource/hyperv_timer.c | 130 +++++++++++++++++++++++++++++++++++ drivers/hv/hv_util.c | 1 + include/clocksource/hyperv_timer.h | 78 +++++++++++++++++++++ 8 files changed, 226 insertions(+), 159 deletions(-) diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c index 98c7d12..7983ca2 100644 --- a/arch/x86/entry/vdso/vclock_gettime.c +++ b/arch/x86/entry/vdso/vclock_gettime.c @@ -21,6 +21,7 @@ #include <linux/math64.h> #include <linux/time.h> #include <linux/kernel.h> +#include <clocksource/hyperv_timer.h> #define gtod (&VVAR(vsyscall_gtod_data)) diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c index babc4e7..99b38e9 100644 --- a/arch/x86/entry/vdso/vma.c +++ b/arch/x86/entry/vdso/vma.c @@ -22,7 +22,7 @@ #include <asm/page.h> #include <asm/desc.h> #include <asm/cpufeature.h> -#include <asm/mshyperv.h> +#include <clocksource/hyperv_timer.h> #if defined(CONFIG_X86_64) unsigned int __read_mostly vdso64_enabled = 1; diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index e4ba467..79f626a 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -27,64 +27,13 @@ #include <linux/version.h> #include <linux/vmalloc.h> #include <linux/mm.h> -#include <linux/clockchips.h> #include <linux/hyperv.h> #include <linux/slab.h> #include <linux/cpuhotplug.h> - -#ifdef CONFIG_HYPERV_TSCPAGE - -static struct ms_hyperv_tsc_page *tsc_pg; - -struct ms_hyperv_tsc_page *hv_get_tsc_page(void) -{ - return tsc_pg; -} -EXPORT_SYMBOL_GPL(hv_get_tsc_page); - -static u64 read_hv_clock_tsc(struct clocksource *arg) -{ - u64 current_tick = hv_read_tsc_page(tsc_pg); - - if (current_tick == U64_MAX) - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); - - return current_tick; -} - -static struct clocksource hyperv_cs_tsc = { - .name = "hyperv_clocksource_tsc_page", - .rating = 400, - .read = read_hv_clock_tsc, - .mask = CLOCKSOURCE_MASK(64), - .flags = CLOCK_SOURCE_IS_CONTINUOUS, -}; -#endif - -static u64 read_hv_clock_msr(struct clocksource *arg) -{ - u64 current_tick; - /* - * Read the partition counter to get the current tick count. This count - * is set to 0 when the partition is created and is incremented in - * 100 nanosecond units. - */ - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); - return current_tick; -} - -static struct clocksource hyperv_cs_msr = { - .name = "hyperv_clocksource_msr", - .rating = 400, - .read = read_hv_clock_msr, - .mask = CLOCKSOURCE_MASK(64), - .flags = CLOCK_SOURCE_IS_CONTINUOUS, -}; +#include <clocksource/hyperv_timer.h> void *hv_hypercall_pg; EXPORT_SYMBOL_GPL(hv_hypercall_pg); -struct clocksource *hyperv_cs; -EXPORT_SYMBOL_GPL(hyperv_cs); u32 *hv_vp_index; EXPORT_SYMBOL_GPL(hv_vp_index); @@ -353,42 +302,8 @@ void __init hyperv_init(void) x86_init.pci.arch_init = hv_pci_init; - /* - * Register Hyper-V specific clocksource. - */ -#ifdef CONFIG_HYPERV_TSCPAGE - if (ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) { - union hv_x64_msr_hypercall_contents tsc_msr; - - tsc_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL); - if (!tsc_pg) - goto register_msr_cs; - - hyperv_cs = &hyperv_cs_tsc; - - rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64); - - tsc_msr.enable = 1; - tsc_msr.guest_physical_address = vmalloc_to_pfn(tsc_pg); - - wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64); - - hyperv_cs_tsc.archdata.vclock_mode = VCLOCK_HVCLOCK; - - clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100); - return; - } -register_msr_cs: -#endif - /* - * For 32 bit guests just use the MSR based mechanism for reading - * the partition counter. - */ - - hyperv_cs = &hyperv_cs_msr; - if (ms_hyperv.features & HV_MSR_TIME_REF_COUNT_AVAILABLE) - clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100); - + /* Register Hyper-V specific clocksource */ + hv_init_clocksource(); return; remove_cpuhp_state: diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h index cc60e61..f4fa8a9 100644 --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -105,6 +105,17 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type) #define hv_get_crash_ctl(val) \ rdmsrl(HV_X64_MSR_CRASH_CTL, val) +#define hv_get_time_ref_count(val) \ + rdmsrl(HV_X64_MSR_TIME_REF_COUNT, val) + +#define hv_get_reference_tsc(val) \ + rdmsrl(HV_X64_MSR_REFERENCE_TSC, val) +#define hv_set_reference_tsc(val) \ + wrmsrl(HV_X64_MSR_REFERENCE_TSC, val) +#define hv_set_clocksource_vdso(val) \ + ((val).archdata.vclock_mode = VCLOCK_HVCLOCK) +#define hv_get_raw_timer() rdtsc_ordered() + void hyperv_callback_vector(void); void hyperv_reenlightenment_vector(void); #ifdef CONFIG_TRACING @@ -133,7 +144,6 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {} #if IS_ENABLED(CONFIG_HYPERV) -extern struct clocksource *hyperv_cs; extern void *hv_hypercall_pg; extern void __percpu **hyperv_pcpu_input_arg; @@ -387,73 +397,4 @@ static inline int hyperv_flush_guest_mapping_range(u64 as, } #endif /* CONFIG_HYPERV */ -#ifdef CONFIG_HYPERV_TSCPAGE -struct ms_hyperv_tsc_page *hv_get_tsc_page(void); -static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, - u64 *cur_tsc) -{ - u64 scale, offset; - u32 sequence; - - /* - * The protocol for reading Hyper-V TSC page is specified in Hypervisor - * Top-Level Functional Specification ver. 3.0 and above. To get the - * reference time we must do the following: - * - READ ReferenceTscSequence - * A special '0' value indicates the time source is unreliable and we - * need to use something else. The currently published specification - * versions (up to 4.0b) contain a mistake and wrongly claim '-1' - * instead of '0' as the special value, see commit c35b82ef0294. - * - ReferenceTime = - * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset - * - READ ReferenceTscSequence again. In case its value has changed - * since our first reading we need to discard ReferenceTime and repeat - * the whole sequence as the hypervisor was updating the page in - * between. - */ - do { - sequence = READ_ONCE(tsc_pg->tsc_sequence); - if (!sequence) - return U64_MAX; - /* - * Make sure we read sequence before we read other values from - * TSC page. - */ - smp_rmb(); - - scale = READ_ONCE(tsc_pg->tsc_scale); - offset = READ_ONCE(tsc_pg->tsc_offset); - *cur_tsc = rdtsc_ordered(); - - /* - * Make sure we read sequence after we read all other values - * from TSC page. - */ - smp_rmb(); - - } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence); - - return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset; -} - -static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) -{ - u64 cur_tsc; - - return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc); -} - -#else -static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) -{ - return NULL; -} - -static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, - u64 *cur_tsc) -{ - BUG(); - return U64_MAX; -} -#endif #endif diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 536b78c..3fbaac0 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -70,6 +70,7 @@ #include <asm/mshyperv.h> #include <asm/hypervisor.h> #include <asm/intel_pt.h> +#include <clocksource/hyperv_timer.h> #define CREATE_TRACE_POINTS #include "trace.h" diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index 30615f3..274614d 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -14,6 +14,8 @@ #include <linux/percpu.h> #include <linux/cpumask.h> #include <linux/clockchips.h> +#include <linux/clocksource.h> +#include <linux/sched_clock.h> #include <linux/mm.h> #include <clocksource/hyperv_timer.h> #include <asm/hyperv-tlfs.h> @@ -189,3 +191,131 @@ void hv_stimer_global_cleanup(void) hv_stimer_free(); } EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup); + +/* + * Code and definitions for the Hyper-V clocksources. Two + * clocksources are defined: one that reads the Hyper-V defined MSR, and + * the other that uses the TSC reference page feature as defined in the + * TLFS. The MSR version is for compatibility with old versions of + * Hyper-V and 32-bit x86. The TSC reference page version is preferred. + */ + +struct clocksource *hyperv_cs; +EXPORT_SYMBOL_GPL(hyperv_cs); + +#ifdef CONFIG_HYPERV_TSCPAGE + +static struct ms_hyperv_tsc_page *tsc_pg; + +struct ms_hyperv_tsc_page *hv_get_tsc_page(void) +{ + return tsc_pg; +} +EXPORT_SYMBOL_GPL(hv_get_tsc_page); + +static u64 read_hv_sched_clock_tsc(void) +{ + u64 current_tick = hv_read_tsc_page(tsc_pg); + + if (current_tick == U64_MAX) + hv_get_time_ref_count(current_tick); + + return current_tick; +} + +static u64 read_hv_clock_tsc(struct clocksource *arg) +{ + return read_hv_sched_clock_tsc(); +} + +static struct clocksource hyperv_cs_tsc = { + .name = "hyperv_clocksource_tsc_page", + .rating = 400, + .read = read_hv_clock_tsc, + .mask = CLOCKSOURCE_MASK(64), + .flags = CLOCK_SOURCE_IS_CONTINUOUS, +}; +#endif + +static u64 read_hv_sched_clock_msr(void) +{ + u64 current_tick; + /* + * Read the partition counter to get the current tick count. This count + * is set to 0 when the partition is created and is incremented in + * 100 nanosecond units. + */ + hv_get_time_ref_count(current_tick); + return current_tick; +} + +static u64 read_hv_clock_msr(struct clocksource *arg) +{ + return read_hv_sched_clock_msr(); +} + +static struct clocksource hyperv_cs_msr = { + .name = "hyperv_clocksource_msr", + .rating = 400, + .read = read_hv_clock_msr, + .mask = CLOCKSOURCE_MASK(64), + .flags = CLOCK_SOURCE_IS_CONTINUOUS, +}; + +void __init hv_init_clocksource(void) +{ +#ifdef CONFIG_HYPERV_TSCPAGE + if (ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) { + + u64 tsc_msr; + phys_addr_t phys_addr; + + tsc_pg = vmalloc(PAGE_SIZE); + if (!tsc_pg) + goto register_msr_cs; + + hyperv_cs = &hyperv_cs_tsc; + phys_addr = page_to_phys(vmalloc_to_page(tsc_pg)); + + /* The Hyper-V TLFS specifies to preserve the value of + * reserved bits in registers. So read the existing + * value, preserve the low order 12 bits, and add in + * the guest physical address (which already has at + * least the low 12 bits set to zero since it is page + * aligned). Also set the "enable" bit, which is bit 0. + */ + hv_get_reference_tsc(tsc_msr); + tsc_msr &= GENMASK_ULL(11, 0); + tsc_msr = tsc_msr | 0x1 | (u64)phys_addr; + hv_set_reference_tsc(tsc_msr); + + hv_set_clocksource_vdso(hyperv_cs_tsc); + clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100); + + /* + * sched_clock_register is needed on ARM64 but + * is a no-op on x86 + */ + sched_clock_register(read_hv_sched_clock_tsc, + 64, HV_CLOCK_HZ); + return; + } +register_msr_cs: +#endif + /* + * For 32 bit guests just use the MSR based mechanism for reading + * the partition counter. + */ + hyperv_cs = &hyperv_cs_msr; + if (ms_hyperv.features & HV_MSR_TIME_REF_COUNT_AVAILABLE) { + clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100); + + /* + * sched_clock_register is needed on ARM64 but + * is a no-op on x86 + */ + sched_clock_register(read_hv_sched_clock_msr, + 64, HV_CLOCK_HZ); + } +} +EXPORT_SYMBOL_GPL(hv_init_clocksource); diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index f10eeb1..9eff85e 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -29,6 +29,7 @@ #include <linux/hyperv.h> #include <linux/clockchips.h> #include <linux/ptp_clock_kernel.h> +#include <clocksource/hyperv_timer.h> #include <asm/mshyperv.h> #include "hyperv_vmbus.h" diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h index ba5dc17..e17e8a2 100644 --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -13,6 +13,10 @@ #ifndef __CLKSOURCE_HYPERV_TIMER_H #define __CLKSOURCE_HYPERV_TIMER_H +#include <linux/clocksource.h> +#include <linux/math64.h> +#include <asm/mshyperv.h> + #define HV_MAX_MAX_DELTA_TICKS 0xffffffff #define HV_MIN_DELTA_TICKS 1 @@ -24,4 +28,78 @@ extern void hv_stimer_global_cleanup(void); extern void hv_stimer0_isr(void); +#if IS_ENABLED(CONFIG_HYPERV) +extern struct clocksource *hyperv_cs; +extern void hv_init_clocksource(void); +#endif /* CONFIG_HYPERV */ + +#ifdef CONFIG_HYPERV_TSCPAGE +extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void); +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, + u64 *cur_tsc) +{ + u64 scale, offset; + u32 sequence; + + /* + * The protocol for reading Hyper-V TSC page is specified in Hypervisor + * Top-Level Functional Specification ver. 3.0 and above. To get the + * reference time we must do the following: + * - READ ReferenceTscSequence + * A special '0' value indicates the time source is unreliable and we + * need to use something else. The currently published specification + * versions (up to 4.0b) contain a mistake and wrongly claim '-1' + * instead of '0' as the special value, see commit c35b82ef0294. + * - ReferenceTime = + * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset + * - READ ReferenceTscSequence again. In case its value has changed + * since our first reading we need to discard ReferenceTime and repeat + * the whole sequence as the hypervisor was updating the page in + * between. + */ + do { + sequence = READ_ONCE(tsc_pg->tsc_sequence); + if (!sequence) + return U64_MAX; + /* + * Make sure we read sequence before we read other values from + * TSC page. + */ + smp_rmb(); + + scale = READ_ONCE(tsc_pg->tsc_scale); + offset = READ_ONCE(tsc_pg->tsc_offset); + *cur_tsc = hv_get_raw_timer(); + + /* + * Make sure we read sequence after we read all other values + * from TSC page. + */ + smp_rmb(); + + } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence); + + return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset; +} + +static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) +{ + u64 cur_tsc; + + return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc); +} + +#else /* CONFIG_HYPERV_TSC_PAGE */ +static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) +{ + return NULL; +} + +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, + u64 *cur_tsc) +{ + return U64_MAX; +} +#endif /* CONFIG_HYPERV_TSCPAGE */ + #endif -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver 2019-05-27 14:59 ` [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver Michael Kelley @ 2019-05-30 9:48 ` Vitaly Kuznetsov 2019-05-30 12:39 ` Michael Kelley 0 siblings, 1 reply; 8+ messages in thread From: Vitaly Kuznetsov @ 2019-05-30 9:48 UTC (permalink / raw) To: Michael Kelley Cc: catalin.marinas, mark.rutland, will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Michael Kelley <mikelley@microsoft.com> writes: > Code for the Hyper-V specific clocksources is currently mixed > in with other Hyper-V code. Move the code to the Hyper-V specific > driver in the "clocksource" directory, while separating out > ISA dependencies so that the clocksource driver remains ISA > independent. Update the Hyper-V initialization code to call > initialization and cleanup routines since the Hyper-V synthetic > timers are not independently enumerated in ACPI. Update Hyper-V > clocksource users KVM and VDSO to get definitions from a new > include file. > > No behavior is changed and no new functionality is added. > > Suggested-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > --- > arch/x86/entry/vdso/vclock_gettime.c | 1 + > arch/x86/entry/vdso/vma.c | 2 +- > arch/x86/hyperv/hv_init.c | 91 +----------------------- > arch/x86/include/asm/mshyperv.h | 81 +++------------------- > arch/x86/kvm/x86.c | 1 + > drivers/clocksource/hyperv_timer.c | 130 +++++++++++++++++++++++++++++++++++ > drivers/hv/hv_util.c | 1 + > include/clocksource/hyperv_timer.h | 78 +++++++++++++++++++++ > 8 files changed, 226 insertions(+), 159 deletions(-) > > diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c > index 98c7d12..7983ca2 100644 > --- a/arch/x86/entry/vdso/vclock_gettime.c > +++ b/arch/x86/entry/vdso/vclock_gettime.c > @@ -21,6 +21,7 @@ > #include <linux/math64.h> > #include <linux/time.h> > #include <linux/kernel.h> > +#include <clocksource/hyperv_timer.h> > > #define gtod (&VVAR(vsyscall_gtod_data)) > > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > index babc4e7..99b38e9 100644 > --- a/arch/x86/entry/vdso/vma.c > +++ b/arch/x86/entry/vdso/vma.c > @@ -22,7 +22,7 @@ > #include <asm/page.h> > #include <asm/desc.h> > #include <asm/cpufeature.h> > -#include <asm/mshyperv.h> > +#include <clocksource/hyperv_timer.h> > > #if defined(CONFIG_X86_64) > unsigned int __read_mostly vdso64_enabled = 1; > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index e4ba467..79f626a 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -27,64 +27,13 @@ > #include <linux/version.h> > #include <linux/vmalloc.h> > #include <linux/mm.h> > -#include <linux/clockchips.h> > #include <linux/hyperv.h> > #include <linux/slab.h> > #include <linux/cpuhotplug.h> > - > -#ifdef CONFIG_HYPERV_TSCPAGE > - > -static struct ms_hyperv_tsc_page *tsc_pg; > - > -struct ms_hyperv_tsc_page *hv_get_tsc_page(void) > -{ > - return tsc_pg; > -} > -EXPORT_SYMBOL_GPL(hv_get_tsc_page); > - > -static u64 read_hv_clock_tsc(struct clocksource *arg) > -{ > - u64 current_tick = hv_read_tsc_page(tsc_pg); > - > - if (current_tick == U64_MAX) > - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); > - > - return current_tick; > -} > - > -static struct clocksource hyperv_cs_tsc = { > - .name = "hyperv_clocksource_tsc_page", > - .rating = 400, > - .read = read_hv_clock_tsc, > - .mask = CLOCKSOURCE_MASK(64), > - .flags = CLOCK_SOURCE_IS_CONTINUOUS, > -}; > -#endif > - > -static u64 read_hv_clock_msr(struct clocksource *arg) > -{ > - u64 current_tick; > - /* > - * Read the partition counter to get the current tick count. This count > - * is set to 0 when the partition is created and is incremented in > - * 100 nanosecond units. > - */ > - rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick); > - return current_tick; > -} > - > -static struct clocksource hyperv_cs_msr = { > - .name = "hyperv_clocksource_msr", > - .rating = 400, > - .read = read_hv_clock_msr, > - .mask = CLOCKSOURCE_MASK(64), > - .flags = CLOCK_SOURCE_IS_CONTINUOUS, > -}; > +#include <clocksource/hyperv_timer.h> > > void *hv_hypercall_pg; > EXPORT_SYMBOL_GPL(hv_hypercall_pg); > -struct clocksource *hyperv_cs; > -EXPORT_SYMBOL_GPL(hyperv_cs); > > u32 *hv_vp_index; > EXPORT_SYMBOL_GPL(hv_vp_index); > @@ -353,42 +302,8 @@ void __init hyperv_init(void) > > x86_init.pci.arch_init = hv_pci_init; > > - /* > - * Register Hyper-V specific clocksource. > - */ > -#ifdef CONFIG_HYPERV_TSCPAGE > - if (ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) { > - union hv_x64_msr_hypercall_contents tsc_msr; > - > - tsc_pg = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL); > - if (!tsc_pg) > - goto register_msr_cs; > - > - hyperv_cs = &hyperv_cs_tsc; > - > - rdmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64); > - > - tsc_msr.enable = 1; > - tsc_msr.guest_physical_address = vmalloc_to_pfn(tsc_pg); > - > - wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64); > - > - hyperv_cs_tsc.archdata.vclock_mode = VCLOCK_HVCLOCK; > - > - clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100); > - return; > - } > -register_msr_cs: > -#endif > - /* > - * For 32 bit guests just use the MSR based mechanism for reading > - * the partition counter. > - */ > - > - hyperv_cs = &hyperv_cs_msr; > - if (ms_hyperv.features & HV_MSR_TIME_REF_COUNT_AVAILABLE) > - clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100); > - > + /* Register Hyper-V specific clocksource */ > + hv_init_clocksource(); > return; > > remove_cpuhp_state: > diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h > index cc60e61..f4fa8a9 100644 > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -105,6 +105,17 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type) > #define hv_get_crash_ctl(val) \ > rdmsrl(HV_X64_MSR_CRASH_CTL, val) > > +#define hv_get_time_ref_count(val) \ > + rdmsrl(HV_X64_MSR_TIME_REF_COUNT, val) > + > +#define hv_get_reference_tsc(val) \ > + rdmsrl(HV_X64_MSR_REFERENCE_TSC, val) > +#define hv_set_reference_tsc(val) \ > + wrmsrl(HV_X64_MSR_REFERENCE_TSC, val) > +#define hv_set_clocksource_vdso(val) \ > + ((val).archdata.vclock_mode = VCLOCK_HVCLOCK) > +#define hv_get_raw_timer() rdtsc_ordered() > + > void hyperv_callback_vector(void); > void hyperv_reenlightenment_vector(void); > #ifdef CONFIG_TRACING > @@ -133,7 +144,6 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {} > > > #if IS_ENABLED(CONFIG_HYPERV) > -extern struct clocksource *hyperv_cs; > extern void *hv_hypercall_pg; > extern void __percpu **hyperv_pcpu_input_arg; > > @@ -387,73 +397,4 @@ static inline int hyperv_flush_guest_mapping_range(u64 as, > } > #endif /* CONFIG_HYPERV */ > > -#ifdef CONFIG_HYPERV_TSCPAGE > -struct ms_hyperv_tsc_page *hv_get_tsc_page(void); > -static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, > - u64 *cur_tsc) > -{ > - u64 scale, offset; > - u32 sequence; > - > - /* > - * The protocol for reading Hyper-V TSC page is specified in Hypervisor > - * Top-Level Functional Specification ver. 3.0 and above. To get the > - * reference time we must do the following: > - * - READ ReferenceTscSequence > - * A special '0' value indicates the time source is unreliable and we > - * need to use something else. The currently published specification > - * versions (up to 4.0b) contain a mistake and wrongly claim '-1' > - * instead of '0' as the special value, see commit c35b82ef0294. > - * - ReferenceTime = > - * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset > - * - READ ReferenceTscSequence again. In case its value has changed > - * since our first reading we need to discard ReferenceTime and repeat > - * the whole sequence as the hypervisor was updating the page in > - * between. > - */ > - do { > - sequence = READ_ONCE(tsc_pg->tsc_sequence); > - if (!sequence) > - return U64_MAX; > - /* > - * Make sure we read sequence before we read other values from > - * TSC page. > - */ > - smp_rmb(); > - > - scale = READ_ONCE(tsc_pg->tsc_scale); > - offset = READ_ONCE(tsc_pg->tsc_offset); > - *cur_tsc = rdtsc_ordered(); > - > - /* > - * Make sure we read sequence after we read all other values > - * from TSC page. > - */ > - smp_rmb(); > - > - } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence); > - > - return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset; > -} > - > -static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) > -{ > - u64 cur_tsc; > - > - return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc); > -} > - > -#else > -static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) > -{ > - return NULL; > -} > - > -static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, > - u64 *cur_tsc) > -{ > - BUG(); > - return U64_MAX; > -} > -#endif > #endif > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 536b78c..3fbaac0 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -70,6 +70,7 @@ > #include <asm/mshyperv.h> > #include <asm/hypervisor.h> > #include <asm/intel_pt.h> > +#include <clocksource/hyperv_timer.h> > > #define CREATE_TRACE_POINTS > #include "trace.h" > diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c > index 30615f3..274614d 100644 > --- a/drivers/clocksource/hyperv_timer.c > +++ b/drivers/clocksource/hyperv_timer.c > @@ -14,6 +14,8 @@ > #include <linux/percpu.h> > #include <linux/cpumask.h> > #include <linux/clockchips.h> > +#include <linux/clocksource.h> > +#include <linux/sched_clock.h> > #include <linux/mm.h> > #include <clocksource/hyperv_timer.h> > #include <asm/hyperv-tlfs.h> > @@ -189,3 +191,131 @@ void hv_stimer_global_cleanup(void) > hv_stimer_free(); > } > EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup); > + > +/* > + * Code and definitions for the Hyper-V clocksources. Two > + * clocksources are defined: one that reads the Hyper-V defined MSR, and > + * the other that uses the TSC reference page feature as defined in the > + * TLFS. The MSR version is for compatibility with old versions of > + * Hyper-V and 32-bit x86. The TSC reference page version is preferred. > + */ > + > +struct clocksource *hyperv_cs; > +EXPORT_SYMBOL_GPL(hyperv_cs); > + > +#ifdef CONFIG_HYPERV_TSCPAGE > + > +static struct ms_hyperv_tsc_page *tsc_pg; > + > +struct ms_hyperv_tsc_page *hv_get_tsc_page(void) > +{ > + return tsc_pg; > +} > +EXPORT_SYMBOL_GPL(hv_get_tsc_page); > + > +static u64 read_hv_sched_clock_tsc(void) > +{ > + u64 current_tick = hv_read_tsc_page(tsc_pg); > + > + if (current_tick == U64_MAX) > + hv_get_time_ref_count(current_tick); > + > + return current_tick; > +} > + > +static u64 read_hv_clock_tsc(struct clocksource *arg) > +{ > + return read_hv_sched_clock_tsc(); > +} > + > +static struct clocksource hyperv_cs_tsc = { > + .name = "hyperv_clocksource_tsc_page", > + .rating = 400, > + .read = read_hv_clock_tsc, > + .mask = CLOCKSOURCE_MASK(64), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS, > +}; > +#endif > + > +static u64 read_hv_sched_clock_msr(void) > +{ > + u64 current_tick; > + /* > + * Read the partition counter to get the current tick count. This count > + * is set to 0 when the partition is created and is incremented in > + * 100 nanosecond units. > + */ > + hv_get_time_ref_count(current_tick); > + return current_tick; > +} > + > +static u64 read_hv_clock_msr(struct clocksource *arg) > +{ > + return read_hv_sched_clock_msr(); > +} > + > +static struct clocksource hyperv_cs_msr = { > + .name = "hyperv_clocksource_msr", > + .rating = 400, > + .read = read_hv_clock_msr, > + .mask = CLOCKSOURCE_MASK(64), > + .flags = CLOCK_SOURCE_IS_CONTINUOUS, > +}; > + > +void __init hv_init_clocksource(void) > +{ > +#ifdef CONFIG_HYPERV_TSCPAGE > + if (ms_hyperv.features & HV_MSR_REFERENCE_TSC_AVAILABLE) { > + > + u64 tsc_msr; > + phys_addr_t phys_addr; > + > + tsc_pg = vmalloc(PAGE_SIZE); > + if (!tsc_pg) > + goto register_msr_cs; > + > + hyperv_cs = &hyperv_cs_tsc; > + phys_addr = page_to_phys(vmalloc_to_page(tsc_pg)); > + > + /* The Hyper-V TLFS specifies to preserve the value of > + * reserved bits in registers. So read the existing > + * value, preserve the low order 12 bits, and add in > + * the guest physical address (which already has at > + * least the low 12 bits set to zero since it is page > + * aligned). Also set the "enable" bit, which is bit 0. > + */ > + hv_get_reference_tsc(tsc_msr); > + tsc_msr &= GENMASK_ULL(11, 0); > + tsc_msr = tsc_msr | 0x1 | (u64)phys_addr; > + hv_set_reference_tsc(tsc_msr); > + > + hv_set_clocksource_vdso(hyperv_cs_tsc); > + clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100); > + > + /* > + * sched_clock_register is needed on ARM64 but > + * is a no-op on x86 > + */ > + sched_clock_register(read_hv_sched_clock_tsc, > + 64, HV_CLOCK_HZ); > + return; > + } > +register_msr_cs: > +#endif > + /* > + * For 32 bit guests just use the MSR based mechanism for reading > + * the partition counter. > + */ > + hyperv_cs = &hyperv_cs_msr; > + if (ms_hyperv.features & HV_MSR_TIME_REF_COUNT_AVAILABLE) { > + clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100); > + > + /* > + * sched_clock_register is needed on ARM64 but > + * is a no-op on x86 > + */ > + sched_clock_register(read_hv_sched_clock_msr, > + 64, HV_CLOCK_HZ); I'm not sure about ARM, but MSR-based clocksource would be a really bad choice for sched clock on x86, this will slow things down significantly. Luckily, as you're validly stating above, sched_clock_register() is a no-op on x86 as we don't define CONFIG_GENERIC_SCHED_CLOCK. Can we actually *not* do sched_clock_register() in case TSC page is unavailable (and revert to counting jiffies or whatever)? > + } > +} > +EXPORT_SYMBOL_GPL(hv_init_clocksource); > diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c > index f10eeb1..9eff85e 100644 > --- a/drivers/hv/hv_util.c > +++ b/drivers/hv/hv_util.c > @@ -29,6 +29,7 @@ > #include <linux/hyperv.h> > #include <linux/clockchips.h> > #include <linux/ptp_clock_kernel.h> > +#include <clocksource/hyperv_timer.h> > #include <asm/mshyperv.h> > > #include "hyperv_vmbus.h" > diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h > index ba5dc17..e17e8a2 100644 > --- a/include/clocksource/hyperv_timer.h > +++ b/include/clocksource/hyperv_timer.h > @@ -13,6 +13,10 @@ > #ifndef __CLKSOURCE_HYPERV_TIMER_H > #define __CLKSOURCE_HYPERV_TIMER_H > > +#include <linux/clocksource.h> > +#include <linux/math64.h> > +#include <asm/mshyperv.h> > + > #define HV_MAX_MAX_DELTA_TICKS 0xffffffff > #define HV_MIN_DELTA_TICKS 1 > > @@ -24,4 +28,78 @@ > extern void hv_stimer_global_cleanup(void); > extern void hv_stimer0_isr(void); > > +#if IS_ENABLED(CONFIG_HYPERV) > +extern struct clocksource *hyperv_cs; > +extern void hv_init_clocksource(void); > +#endif /* CONFIG_HYPERV */ > + > +#ifdef CONFIG_HYPERV_TSCPAGE > +extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void); > +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, > + u64 *cur_tsc) > +{ > + u64 scale, offset; > + u32 sequence; > + > + /* > + * The protocol for reading Hyper-V TSC page is specified in Hypervisor > + * Top-Level Functional Specification ver. 3.0 and above. To get the > + * reference time we must do the following: > + * - READ ReferenceTscSequence > + * A special '0' value indicates the time source is unreliable and we > + * need to use something else. The currently published specification > + * versions (up to 4.0b) contain a mistake and wrongly claim '-1' > + * instead of '0' as the special value, see commit c35b82ef0294. > + * - ReferenceTime = > + * ((RDTSC() * ReferenceTscScale) >> 64) + ReferenceTscOffset > + * - READ ReferenceTscSequence again. In case its value has changed > + * since our first reading we need to discard ReferenceTime and repeat > + * the whole sequence as the hypervisor was updating the page in > + * between. > + */ > + do { > + sequence = READ_ONCE(tsc_pg->tsc_sequence); > + if (!sequence) > + return U64_MAX; > + /* > + * Make sure we read sequence before we read other values from > + * TSC page. > + */ > + smp_rmb(); > + > + scale = READ_ONCE(tsc_pg->tsc_scale); > + offset = READ_ONCE(tsc_pg->tsc_offset); > + *cur_tsc = hv_get_raw_timer(); > + > + /* > + * Make sure we read sequence after we read all other values > + * from TSC page. > + */ > + smp_rmb(); > + > + } while (READ_ONCE(tsc_pg->tsc_sequence) != sequence); > + > + return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset; > +} > + > +static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg) > +{ > + u64 cur_tsc; > + > + return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc); > +} > + > +#else /* CONFIG_HYPERV_TSC_PAGE */ > +static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) > +{ > + return NULL; > +} > + > +static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, > + u64 *cur_tsc) > +{ > + return U64_MAX; > +} > +#endif /* CONFIG_HYPERV_TSCPAGE */ > + > #endif With the (theoretical) question above answered, Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> -- Vitaly ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver 2019-05-30 9:48 ` Vitaly Kuznetsov @ 2019-05-30 12:39 ` Michael Kelley 2019-05-30 13:51 ` Vitaly Kuznetsov 0 siblings, 1 reply; 8+ messages in thread From: Michael Kelley @ 2019-05-30 12:39 UTC (permalink / raw) To: vkuznets Cc: catalin.marinas, mark.rutland, will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, May 30, 2019 2:48 AM > > + /* > > + * sched_clock_register is needed on ARM64 but > > + * is a no-op on x86 > > + */ > > + sched_clock_register(read_hv_sched_clock_msr, > > + 64, HV_CLOCK_HZ); > > I'm not sure about ARM, but MSR-based clocksource would be a really bad > choice for sched clock on x86, this will slow things down > significantly. Luckily, as you're validly stating above, > sched_clock_register() is a no-op on x86 as we don't define > CONFIG_GENERIC_SCHED_CLOCK. > > Can we actually *not* do sched_clock_register() in case > TSC page is unavailable (and revert to counting jiffies or whatever)? > We can't skip the sched_clock_register() on ARM64 because it does define CONFIG_GENERIC_SCHED_CLOCK. However, Hyper-V should always provide REFERENCE_TSC_AVAILALBE on ARM64, so we should never end up in the MSR-based code on ARM64. Arguably that means the call to sched_clock_register() could be removed since it's a no-op on x86. But I'd like to keep it for symmetry and in case there's a testing/debugging situation on ARM64 where we want to clear REFERENCE_TSC_AVAILABLE and go down the MSR-based code path. Michael ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver 2019-05-30 12:39 ` Michael Kelley @ 2019-05-30 13:51 ` Vitaly Kuznetsov 0 siblings, 0 replies; 8+ messages in thread From: Vitaly Kuznetsov @ 2019-05-30 13:51 UTC (permalink / raw) To: Michael Kelley Cc: catalin.marinas, mark.rutland, will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan Michael Kelley <mikelley@microsoft.com> writes: > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, May 30, 2019 2:48 AM >> > + /* >> > + * sched_clock_register is needed on ARM64 but >> > + * is a no-op on x86 >> > + */ >> > + sched_clock_register(read_hv_sched_clock_msr, >> > + 64, HV_CLOCK_HZ); >> >> I'm not sure about ARM, but MSR-based clocksource would be a really bad >> choice for sched clock on x86, this will slow things down >> significantly. Luckily, as you're validly stating above, >> sched_clock_register() is a no-op on x86 as we don't define >> CONFIG_GENERIC_SCHED_CLOCK. >> >> Can we actually *not* do sched_clock_register() in case >> TSC page is unavailable (and revert to counting jiffies or whatever)? >> > > We can't skip the sched_clock_register() on ARM64 because it > does define CONFIG_GENERIC_SCHED_CLOCK. However, Hyper-V > should always provide REFERENCE_TSC_AVAILALBE on ARM64, > so we should never end up in the MSR-based code on ARM64. > Arguably that means the call to sched_clock_register() could be > removed since it's a no-op on x86. But I'd like to keep it for symmetry > and in case there's a testing/debugging situation on ARM64 where > we want to clear REFERENCE_TSC_AVAILABLE and go down the > MSR-based code path. Ok, so it is just a fall-back and not going to be actively used. Thanks! -- Vitaly ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver 2019-05-27 14:59 [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver Michael Kelley 2019-05-27 14:59 ` [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Michael Kelley 2019-05-27 14:59 ` [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver Michael Kelley @ 2019-06-06 16:37 ` Sasha Levin 2 siblings, 0 replies; 8+ messages in thread From: Sasha Levin @ 2019-06-06 16:37 UTC (permalink / raw) To: Michael Kelley Cc: will.deacon, marc.zyngier, linux-arm-kernel, gregkh, linux-kernel, linux-hyperv, olaf, apw, vkuznets, jasowang, marcelo.cerri, Sunil Muthuswamy, KY Srinivasan, catalin.marinas, mark.rutland On Mon, May 27, 2019 at 02:59:07PM +0000, Michael Kelley wrote: >This patch series moves Hyper-V clock/timer code to a separate Hyper-V >clocksource driver. Previously, Hyper-V clock/timer code and data >structures were mixed in with other Hyper-V code in the ISA independent >drivers/hv code as well as in arch dependent code. The new Hyper-V >clocksource driver is ISA independent, with a just few dependencies on >arch specific functions. The patch series does not change any behavior >or functionality -- it only reorganizes the existing code and fixes up >the linkages. A few places outside of Hyper-V code are fixed up to use >the new #include file structure. > >This restructuring is in response to Marc Zyngier's review comments >on supporting Hyper-V running on ARM64, and is a good idea in general. >It increases the amount of code shared between the x86 and ARM64 >architectures, and reduces the size of the new code for supporting >Hyper-V on ARM64. A new version of the Hyper-V on ARM64 patches will >follow once this clocksource restructuring is accepted. > >The code is diff'ed against Linux 5.2.0-rc1-next-20190524. > >Changes in v3: >* Removed boolean argument to hv_init_clocksource(). Always call >sched_clock_register, which is needed on ARM64 but a no-op on x86. >* Removed separate cpuhp setup in hv_stimer_alloc() and instead >directly call hv_stimer_init() and hv_stimer_cleanup() from >corresponding VMbus functions. This more closely matches original >code and avoids clocksource stop/restart problems on ARM64 when >VMbus code denies CPU offlining request. > >Changes in v2: >* Revised commit short descriptions so the distinction between >the first and second patches is clearer [GregKH] >* Renamed new clocksource driver files and functions to use >existing "timer" and "stimer" names instead of introducing >"syntimer". [Vitaly Kuznetsov] >* Introduced CONFIG_HYPER_TIMER to fix build problem when >CONFIG_HYPERV=m [Vitaly Kuznetsov] >* Added "Suggested-by: Marc Zyngier" > >Michael Kelley (2): > Drivers: hv: Create Hyper-V clocksource driver from existing > clockevents code > Drivers: hv: Move Hyper-V clocksource code to new clocksource driver > > MAINTAINERS | 2 + > arch/x86/entry/vdso/vclock_gettime.c | 1 + > arch/x86/entry/vdso/vma.c | 2 +- > arch/x86/hyperv/hv_init.c | 91 +--------- > arch/x86/include/asm/hyperv-tlfs.h | 6 + > arch/x86/include/asm/mshyperv.h | 81 ++------- > arch/x86/kernel/cpu/mshyperv.c | 2 + > arch/x86/kvm/x86.c | 1 + > drivers/clocksource/Makefile | 1 + > drivers/clocksource/hyperv_timer.c | 321 +++++++++++++++++++++++++++++++++++ > drivers/hv/Kconfig | 3 + > drivers/hv/hv.c | 156 +---------------- > drivers/hv/hv_util.c | 1 + > drivers/hv/hyperv_vmbus.h | 3 - > drivers/hv/vmbus_drv.c | 42 ++--- > include/clocksource/hyperv_timer.h | 105 ++++++++++++ > 16 files changed, 484 insertions(+), 334 deletions(-) > create mode 100644 drivers/clocksource/hyperv_timer.c > create mode 100644 include/clocksource/hyperv_timer.h Queued for hyperv-next, thanks! -- Thanks, Sasha ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-06 16:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-27 14:59 [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver Michael Kelley 2019-05-27 14:59 ` [PATCH v3 1/2] Drivers: hv: Create Hyper-V clocksource driver from existing clockevents code Michael Kelley 2019-05-30 9:37 ` Vitaly Kuznetsov 2019-05-27 14:59 ` [PATCH v3 2/2] Drivers: hv: Move Hyper-V clocksource code to new clocksource driver Michael Kelley 2019-05-30 9:48 ` Vitaly Kuznetsov 2019-05-30 12:39 ` Michael Kelley 2019-05-30 13:51 ` Vitaly Kuznetsov 2019-06-06 16:37 ` [PATCH v3 0/2] Drivers: hv: Move Hyper-V clock/timer code to separate " Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).