| Message ID | 1552426813-9568-2-git-send-email-mikelley@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Drivers: hv: Move Hyper-V clock/timer code to separate clocksource driver | expand |
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. > I like the idea! Would it also make sense to consider moving Hyper-V clocksource from arch/x86/hyperv/hv_init.c? The thing is that we use it from arch-independent code (e.g. seee 'hyperv_cs' usage in drivers/hv/hv_util.c). > No behavior is changed and no new functionality is added. > > 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_syntimer.c | 206 ++++++++++++++++++++++++++++++++++ > drivers/hv/hv.c | 154 ------------------------- > drivers/hv/hyperv_vmbus.h | 3 - > drivers/hv/vmbus_drv.c | 39 +++---- > include/clocksource/hyperv_syntimer.h | 26 +++++ > 9 files changed, 263 insertions(+), 176 deletions(-) > create mode 100644 drivers/clocksource/hyperv_syntimer.c > create mode 100644 include/clocksource/hyperv_syntimer.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 21ab064..3352716 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7159,6 +7159,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_syntimer.c > F: drivers/hid/hid-hyperv.c > F: drivers/hv/ > F: drivers/input/serio/hyperv-keyboard.c > @@ -7168,6 +7169,7 @@ F: drivers/scsi/storvsc_drv.c > F: drivers/uio/uio_hv_generic.c > F: drivers/video/fbdev/hyperv_fb.c > F: net/vmw_vsock/hyperv_transport.c > +F: include/clocksource/hyperv_syntimer.h Nitpicking: This has nothing to do with your patch but we have a mix of 'stimer', 'timer', 'syntimer' names when we refer to Hyper-V Synthetic timers, it may make sense to try to converge on something (stimer would probably be my personal preference but I don't really care as long as we use the same abbreviation). Examples: hv_init_timer_config(...) HV_MSR_SYNTIMER_AVAILABLE HYPERV_STIMER0_VECTOR hv_syntimer_init(...) ... > 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 2bdbbbc..ee62f57 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 e81a2db..f53a35a 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 be6e0fb..a887955 100644 > --- a/drivers/clocksource/Makefile > +++ b/drivers/clocksource/Makefile > @@ -83,3 +83,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) += hyperv_syntimer.o (just a couple of spare thoughs) CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to support module loading/unloading then and honestly I see no reason for that. I would prefer everything but VMBus devices to be in kernel.) If we don't want it to be a module we can create a hidden CONFIG_HYPERV_STIMER or something like that - just like we already do for CONFIG_HYPERV_TSCPAGE. There is, however, one additional dependency here: when running in non-direct mode, Hyper-V clockevent devices require functional Hyper-V messaging - which currently lives in VMBus code so that may explain why you may want to keep stimer code in the same entity. Or, alternatively, we can move Hyper-V messaging out of VMBus code (is it actually architecture-agnostic?) > diff --git a/drivers/clocksource/hyperv_syntimer.c b/drivers/clocksource/hyperv_syntimer.c > new file mode 100644 > index 0000000..7276308 > --- /dev/null > +++ b/drivers/clocksource/hyperv_syntimer.c > @@ -0,0 +1,206 @@ > +// 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/cpuhotplug.h> > +#include <linux/mm.h> > +#include <clocksource/hyperv_syntimer.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; > +static int stimer0_cpuhp_online; > + > +/* > + * 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_syntimer_init - Per-cpu initialization of the clockevent > + */ > +static int hv_syntimer_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; > +} > + > +/* > + * hv_syntimer_cleanup - Per-cpu cleanup of the clockevent > + */ > +int hv_syntimer_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_syntimer_cleanup); > + > +/* hv_syntimer_alloc - Global initialization of the clockevent and stimer0 */ > +int hv_syntimer_alloc(int sint) > +{ > + int ret; > + > + 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)) > + goto err_irq; > + > + stimer0_message_sint = sint; > + > + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/stimer0:online", > + hv_syntimer_init, hv_syntimer_cleanup); > + if (ret < 0) > + goto err_cpuhp; > + stimer0_cpuhp_online = ret; > + return 0; > + > +err_cpuhp: > + if (direct_mode_enabled) > + hv_remove_stimer0_irq(stimer0_irq); > +err_irq: > + free_percpu(hv_clock_event); > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(hv_syntimer_alloc); > + > +/* hv_syntimer_free - Free global resources allocated by hv_syntimer_alloc() */ > +void hv_syntimer_free(void) > +{ > + cpuhp_remove_state(stimer0_cpuhp_online); > + if (direct_mode_enabled) > + hv_remove_stimer0_irq(stimer0_irq); > + free_percpu(hv_clock_event); > +} > +EXPORT_SYMBOL_GPL(hv_syntimer_free); > + > +/* > + * Do a global cleanup of clockevents for the cases of kexec and > + * vmbus exit > + */ > +void hv_syntimer_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_syntimer_free(); > +} > +EXPORT_SYMBOL_GPL(hv_syntimer_global_cleanup); > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > index 632d256..e3ee010 100644 > --- a/drivers/hv/hv.c > +++ b/drivers/hv/hv.c > @@ -36,21 +36,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 +45,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 +83,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 +111,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 +133,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 +151,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,39 +209,10 @@ 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; > - > - 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); > - } > -} > - > -/* > * hv_synic_cleanup - Cleanup routine for hv_synic_init(). > */ > int hv_synic_cleanup(unsigned int cpu) > @@ -401,16 +257,6 @@ 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); > - put_cpu_ptr(hv_cpu); > - } > - > hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64); > > shared_sint.masked = 1; > diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h > index cb86b133..ffd4ad8 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 */ > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 000b53e..8e442c5 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_syntimer.h> > #include "hyperv_vmbus.h" > > struct vmbus_dynid { > @@ -939,17 +940,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; > @@ -1143,9 +1133,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); > } > > @@ -1248,8 +1239,8 @@ static int vmbus_bus_init(void) > if (ret) > goto err_alloc; > /* > - * Initialize the per-cpu interrupt state and > - * connect to the host. > + * Initialize the per-cpu interrupt state and syntimer state. > + * Then connect to the host. > */ > ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online", > hv_synic_init, hv_synic_cleanup); > @@ -1257,6 +1248,10 @@ static int vmbus_bus_init(void) > goto err_alloc; > hyperv_cpuhp_online = ret; > > + ret = hv_syntimer_alloc(VMBUS_MESSAGE_SINT); > + if (ret < 0) > + goto err_syntimer_alloc; > + > ret = vmbus_connect(); > if (ret) > goto err_connect; > @@ -1301,6 +1296,8 @@ static int vmbus_bus_init(void) > return 0; > > err_connect: > + hv_syntimer_free(); > +err_syntimer_alloc: > cpuhp_remove_state(hyperv_cpuhp_online); > err_alloc: > hv_synic_free(); > @@ -1971,7 +1968,7 @@ static int vmbus_acpi_add(struct acpi_device *device) > > static void hv_kexec_handler(void) > { > - hv_synic_clockevents_cleanup(); > + hv_syntimer_global_cleanup(); > vmbus_initiate_unload(false); > vmbus_connection.conn_state = DISCONNECTED; > /* Make sure conn_state is set as hv_synic_cleanup checks for it */ > @@ -1982,6 +1979,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, > @@ -1989,7 +1988,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_syntimer_cleanup(cpu); > + hv_synic_cleanup(cpu); > hyperv_cleanup(); > }; > > @@ -2038,7 +2039,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_syntimer_global_cleanup(); > vmbus_disconnect(); > hv_remove_vmbus_irq(); > for_each_online_cpu(cpu) { > diff --git a/include/clocksource/hyperv_syntimer.h b/include/clocksource/hyperv_syntimer.h > new file mode 100644 > index 0000000..154138b > --- /dev/null > +++ b/include/clocksource/hyperv_syntimer.h > @@ -0,0 +1,26 @@ > +/* 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_SYNTIMER_H > +#define __CLKSOURCE_HYPERV_SYNTIMER_H > + > +#define HV_MAX_MAX_DELTA_TICKS 0xffffffff > +#define HV_MIN_DELTA_TICKS 1 > + > +/* Routines called by the VMbus driver */ > +extern int hv_syntimer_alloc(int sint); > +extern void hv_syntimer_free(void); > +extern int hv_syntimer_cleanup(unsigned int cpu); > +extern void hv_syntimer_global_cleanup(void); > +extern void hv_stimer0_isr(void); > + > +#endif
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, March 13, 2019 1:28 AM > > > 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. > > > > I like the idea! Would it also make sense to consider moving Hyper-V > clocksource from arch/x86/hyperv/hv_init.c? The thing is that we use it > from arch-independent code (e.g. seee 'hyperv_cs' usage in > drivers/hv/hv_util.c). That's what the second patch in the series does. :-) But let me know if there's something you think I've missed. > > Nitpicking: > > This has nothing to do with your patch but we have a mix of 'stimer', > 'timer', 'syntimer' names when we refer to Hyper-V Synthetic timers, it > may make sense to try to converge on something (stimer would probably be > my personal preference but I don't really care as long as we use the > same abbreviation). Examples: > > hv_init_timer_config(...) > HV_MSR_SYNTIMER_AVAILABLE > HYPERV_STIMER0_VECTOR > hv_syntimer_init(...) > ... Yes, you are right about the mish-mash of names. I also like converging on "stimer". I'll certainly change things like hv_syntimer_init() to hv_stimer_init() as part of the patch and avoid making the problem worse. What about the name of the new .c and .h files? They include code both the stimer-based clockevents, as well as the Hyper-V reference time source based clocksource. Stay with "syntimer" or shorten to "stimer", even though the code is slightly broader than just the stimers? (which highlights the generic Linux naming issue that "clocksource drivers" include code for both clockevents and clocksources) > > > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > > index be6e0fb..a887955 100644 > > --- a/drivers/clocksource/Makefile > > +++ b/drivers/clocksource/Makefile > > @@ -83,3 +83,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) += hyperv_syntimer.o > > (just a couple of spare thoughs) > > CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to > support module loading/unloading then and honestly I see no reason for > that. I would prefer everything but VMBus devices to be in > kernel.) If we don't want it to be a module we can create a hidden > CONFIG_HYPERV_STIMER or something like that - just like we already do > for CONFIG_HYPERV_TSCPAGE. > > There is, however, one additional dependency here: when running in > non-direct mode, Hyper-V clockevent devices require functional Hyper-V > messaging - which currently lives in VMBus code so that may explain why > you may want to keep stimer code in the same entity. Or, alternatively, > we can move Hyper-V messaging out of VMBus code (is it actually > architecture-agnostic?) > I thought about introducing CONFIG_HYPERV_STIMER, but in my judgment it was just unnecessary complexity. The Hyper-V clocksource driver can't exist independent of Hyper-V, and vice versa. When both the clocksource and clockevents code is considered, the VMbus driver and Hyper-V initialization code has to call directly into the driver since the Hyper-V synthetic timers and reference time counter aren't independently enumerated. Even if we could get the Hyper-V messaging out of VMbus code, we would still need the clocksource initialization call directly from hyperv_init(), which is not in a module (see the 2nd patch of the series). Michael
Michael Kelley <mikelley@microsoft.com> writes: > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, March 13, 2019 1:28 AM >> >> > 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. >> > >> >> I like the idea! Would it also make sense to consider moving Hyper-V >> clocksource from arch/x86/hyperv/hv_init.c? The thing is that we use it >> from arch-independent code (e.g. seee 'hyperv_cs' usage in >> drivers/hv/hv_util.c). > > That's what the second patch in the series does. :-) But let me > know if there's something you think I've missed. > Oh, sorry, it's just me - your subject lines messed with my brain :-) I like your idea even more then! >> >> Nitpicking: >> >> This has nothing to do with your patch but we have a mix of 'stimer', >> 'timer', 'syntimer' names when we refer to Hyper-V Synthetic timers, it >> may make sense to try to converge on something (stimer would probably be >> my personal preference but I don't really care as long as we use the >> same abbreviation). Examples: >> >> hv_init_timer_config(...) >> HV_MSR_SYNTIMER_AVAILABLE >> HYPERV_STIMER0_VECTOR >> hv_syntimer_init(...) >> ... > > Yes, you are right about the mish-mash of names. I also like converging > on "stimer". I'll certainly change things like hv_syntimer_init() to > hv_stimer_init() as part of the patch and avoid making the problem > worse. > > What about the name of the new .c and .h files? They include code > both the stimer-based clockevents, as well as the Hyper-V > reference time source based clocksource. Stay with "syntimer" or > shorten to "stimer", even though the code is slightly broader than > just the stimers? (which highlights the generic Linux naming issue that > "clocksource drivers" include code for both clockevents and > clocksources) "hyperv_timers.*" maybe? (those who read TLFS may get confused because 'Synthetic timers' doesn't refer to clocksources - just to clockevents). > >> >> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >> > index be6e0fb..a887955 100644 >> > --- a/drivers/clocksource/Makefile >> > +++ b/drivers/clocksource/Makefile >> > @@ -83,3 +83,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) += hyperv_syntimer.o >> >> (just a couple of spare thoughs) >> >> CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to >> support module loading/unloading then and honestly I see no reason for >> that. I would prefer everything but VMBus devices to be in >> kernel.) If we don't want it to be a module we can create a hidden >> CONFIG_HYPERV_STIMER or something like that - just like we already do >> for CONFIG_HYPERV_TSCPAGE. >> >> There is, however, one additional dependency here: when running in >> non-direct mode, Hyper-V clockevent devices require functional Hyper-V >> messaging - which currently lives in VMBus code so that may explain why >> you may want to keep stimer code in the same entity. Or, alternatively, >> we can move Hyper-V messaging out of VMBus code (is it actually >> architecture-agnostic?) >> > > I thought about introducing CONFIG_HYPERV_STIMER, but in my > judgment it was just unnecessary complexity. The Hyper-V clocksource > driver can't exist independent of Hyper-V, and vice versa. When both the > clocksource and clockevents code is considered, the VMbus driver and > Hyper-V initialization code has to call directly into the driver since the > Hyper-V synthetic timers and reference time counter aren't independently > enumerated. Even if we could get the Hyper-V messaging out of VMbus > code, we would still need the clocksource initialization call directly from > hyperv_init(), which is not in a module (see the 2nd patch of the series). > Right, so hv_init_clocksource() cannot live in hv_vmbus module and we need to somehow prevent hyperv_syntimer.o from going in there. And +obj-$(CONFIG_HYPERV) += hyperv_syntimer.o will do exactly the opposite - put it in hv_vmbus module. Or am I missing something? (I haven't tried to build your code yet, sorry). > Michael
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, March 13, 2019 7:23 AM > >> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile > >> > index be6e0fb..a887955 100644 > >> > --- a/drivers/clocksource/Makefile > >> > +++ b/drivers/clocksource/Makefile > >> > @@ -83,3 +83,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) += hyperv_syntimer.o > >> > >> (just a couple of spare thoughs) > >> > >> CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to > >> support module loading/unloading then and honestly I see no reason for > >> that. I would prefer everything but VMBus devices to be in > >> kernel.) If we don't want it to be a module we can create a hidden > >> CONFIG_HYPERV_STIMER or something like that - just like we already do > >> for CONFIG_HYPERV_TSCPAGE. > >> > >> There is, however, one additional dependency here: when running in > >> non-direct mode, Hyper-V clockevent devices require functional Hyper-V > >> messaging - which currently lives in VMBus code so that may explain why > >> you may want to keep stimer code in the same entity. Or, alternatively, > >> we can move Hyper-V messaging out of VMBus code (is it actually > >> architecture-agnostic?) > >> > > > > I thought about introducing CONFIG_HYPERV_STIMER, but in my > > judgment it was just unnecessary complexity. The Hyper-V clocksource > > driver can't exist independent of Hyper-V, and vice versa. When both the > > clocksource and clockevents code is considered, the VMbus driver and > > Hyper-V initialization code has to call directly into the driver since the > > Hyper-V synthetic timers and reference time counter aren't independently > > enumerated. Even if we could get the Hyper-V messaging out of VMbus > > code, we would still need the clocksource initialization call directly from > > hyperv_init(), which is not in a module (see the 2nd patch of the series). > > > > Right, so hv_init_clocksource() cannot live in hv_vmbus module and we > need to somehow prevent hyperv_syntimer.o from going in there. And > > +obj-$(CONFIG_HYPERV) += hyperv_syntimer.o > > will do exactly the opposite - put it in hv_vmbus module. Or am I > missing something? (I haven't tried to build your code yet, sorry). > That line just controls whether hyperv_syntimer.o is built. It doesn't put it in the hv_vmbus module. All of the clocksource .o files that are built go into the kernel, not in a module. But thinking about it more, the above works correctly when CONFIG_HYPERV=y, but not when CONFIG_HYPERV=m. I'll have to introduce CONFIG_HYPERV_TIMER after all. Will fix this in v2. Thanks for the discussion! Michael
Michael Kelley <mikelley@microsoft.com> writes: > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, March 13, 2019 7:23 AM > >> >> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile >> >> > index be6e0fb..a887955 100644 >> >> > --- a/drivers/clocksource/Makefile >> >> > +++ b/drivers/clocksource/Makefile >> >> > @@ -83,3 +83,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) += hyperv_syntimer.o >> >> >> >> (just a couple of spare thoughs) >> >> >> >> CONFIG_HYPERV can also be a module, are we OK with that? (we'll have to >> >> support module loading/unloading then and honestly I see no reason for >> >> that. I would prefer everything but VMBus devices to be in >> >> kernel.) If we don't want it to be a module we can create a hidden >> >> CONFIG_HYPERV_STIMER or something like that - just like we already do >> >> for CONFIG_HYPERV_TSCPAGE. >> >> >> >> There is, however, one additional dependency here: when running in >> >> non-direct mode, Hyper-V clockevent devices require functional Hyper-V >> >> messaging - which currently lives in VMBus code so that may explain why >> >> you may want to keep stimer code in the same entity. Or, alternatively, >> >> we can move Hyper-V messaging out of VMBus code (is it actually >> >> architecture-agnostic?) >> >> >> > >> > I thought about introducing CONFIG_HYPERV_STIMER, but in my >> > judgment it was just unnecessary complexity. The Hyper-V clocksource >> > driver can't exist independent of Hyper-V, and vice versa. When both the >> > clocksource and clockevents code is considered, the VMbus driver and >> > Hyper-V initialization code has to call directly into the driver since the >> > Hyper-V synthetic timers and reference time counter aren't independently >> > enumerated. Even if we could get the Hyper-V messaging out of VMbus >> > code, we would still need the clocksource initialization call directly from >> > hyperv_init(), which is not in a module (see the 2nd patch of the series). >> > >> >> Right, so hv_init_clocksource() cannot live in hv_vmbus module and we >> need to somehow prevent hyperv_syntimer.o from going in there. And >> >> +obj-$(CONFIG_HYPERV) += hyperv_syntimer.o >> >> will do exactly the opposite - put it in hv_vmbus module. Or am I >> missing something? (I haven't tried to build your code yet, sorry). >> > > That line just controls whether hyperv_syntimer.o is built. It doesn't put > it in the hv_vmbus module. All of the clocksource .o files that are built go > into the kernel, not in a module. But thinking about it more, the above works > correctly when CONFIG_HYPERV=y, but not when CONFIG_HYPERV=m. Yes, that's what I meant. > I'll have to introduce CONFIG_HYPERV_TIMER after all. Will fix this in v2. Thanks > for the discussion! Thanks!
diff --git a/MAINTAINERS b/MAINTAINERS index 21ab064..3352716 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -7159,6 +7159,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_syntimer.c F: drivers/hid/hid-hyperv.c F: drivers/hv/ F: drivers/input/serio/hyperv-keyboard.c @@ -7168,6 +7169,7 @@ F: drivers/scsi/storvsc_drv.c F: drivers/uio/uio_hv_generic.c F: drivers/video/fbdev/hyperv_fb.c F: net/vmw_vsock/hyperv_transport.c +F: include/clocksource/hyperv_syntimer.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 2bdbbbc..ee62f57 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 e81a2db..f53a35a 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 be6e0fb..a887955 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -83,3 +83,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) += hyperv_syntimer.o diff --git a/drivers/clocksource/hyperv_syntimer.c b/drivers/clocksource/hyperv_syntimer.c new file mode 100644 index 0000000..7276308 --- /dev/null +++ b/drivers/clocksource/hyperv_syntimer.c @@ -0,0 +1,206 @@ +// 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/cpuhotplug.h> +#include <linux/mm.h> +#include <clocksource/hyperv_syntimer.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; +static int stimer0_cpuhp_online; + +/* + * 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_syntimer_init - Per-cpu initialization of the clockevent + */ +static int hv_syntimer_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; +} + +/* + * hv_syntimer_cleanup - Per-cpu cleanup of the clockevent + */ +int hv_syntimer_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_syntimer_cleanup); + +/* hv_syntimer_alloc - Global initialization of the clockevent and stimer0 */ +int hv_syntimer_alloc(int sint) +{ + int ret; + + 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)) + goto err_irq; + + stimer0_message_sint = sint; + + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/stimer0:online", + hv_syntimer_init, hv_syntimer_cleanup); + if (ret < 0) + goto err_cpuhp; + stimer0_cpuhp_online = ret; + return 0; + +err_cpuhp: + if (direct_mode_enabled) + hv_remove_stimer0_irq(stimer0_irq); +err_irq: + free_percpu(hv_clock_event); + return -EINVAL; +} +EXPORT_SYMBOL_GPL(hv_syntimer_alloc); + +/* hv_syntimer_free - Free global resources allocated by hv_syntimer_alloc() */ +void hv_syntimer_free(void) +{ + cpuhp_remove_state(stimer0_cpuhp_online); + if (direct_mode_enabled) + hv_remove_stimer0_irq(stimer0_irq); + free_percpu(hv_clock_event); +} +EXPORT_SYMBOL_GPL(hv_syntimer_free); + +/* + * Do a global cleanup of clockevents for the cases of kexec and + * vmbus exit + */ +void hv_syntimer_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_syntimer_free(); +} +EXPORT_SYMBOL_GPL(hv_syntimer_global_cleanup); diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index 632d256..e3ee010 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -36,21 +36,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 +45,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 +83,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 +111,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 +133,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 +151,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,39 +209,10 @@ 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; - - 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); - } -} - -/* * hv_synic_cleanup - Cleanup routine for hv_synic_init(). */ int hv_synic_cleanup(unsigned int cpu) @@ -401,16 +257,6 @@ 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); - put_cpu_ptr(hv_cpu); - } - hv_get_synint_state(VMBUS_MESSAGE_SINT, shared_sint.as_uint64); shared_sint.masked = 1; diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index cb86b133..ffd4ad8 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 */ diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 000b53e..8e442c5 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_syntimer.h> #include "hyperv_vmbus.h" struct vmbus_dynid { @@ -939,17 +940,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; @@ -1143,9 +1133,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); } @@ -1248,8 +1239,8 @@ static int vmbus_bus_init(void) if (ret) goto err_alloc; /* - * Initialize the per-cpu interrupt state and - * connect to the host. + * Initialize the per-cpu interrupt state and syntimer state. + * Then connect to the host. */ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online", hv_synic_init, hv_synic_cleanup); @@ -1257,6 +1248,10 @@ static int vmbus_bus_init(void) goto err_alloc; hyperv_cpuhp_online = ret; + ret = hv_syntimer_alloc(VMBUS_MESSAGE_SINT); + if (ret < 0) + goto err_syntimer_alloc; + ret = vmbus_connect(); if (ret) goto err_connect; @@ -1301,6 +1296,8 @@ static int vmbus_bus_init(void) return 0; err_connect: + hv_syntimer_free(); +err_syntimer_alloc: cpuhp_remove_state(hyperv_cpuhp_online); err_alloc: hv_synic_free(); @@ -1971,7 +1968,7 @@ static int vmbus_acpi_add(struct acpi_device *device) static void hv_kexec_handler(void) { - hv_synic_clockevents_cleanup(); + hv_syntimer_global_cleanup(); vmbus_initiate_unload(false); vmbus_connection.conn_state = DISCONNECTED; /* Make sure conn_state is set as hv_synic_cleanup checks for it */ @@ -1982,6 +1979,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, @@ -1989,7 +1988,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_syntimer_cleanup(cpu); + hv_synic_cleanup(cpu); hyperv_cleanup(); }; @@ -2038,7 +2039,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_syntimer_global_cleanup(); vmbus_disconnect(); hv_remove_vmbus_irq(); for_each_online_cpu(cpu) { diff --git a/include/clocksource/hyperv_syntimer.h b/include/clocksource/hyperv_syntimer.h new file mode 100644 index 0000000..154138b --- /dev/null +++ b/include/clocksource/hyperv_syntimer.h @@ -0,0 +1,26 @@ +/* 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_SYNTIMER_H +#define __CLKSOURCE_HYPERV_SYNTIMER_H + +#define HV_MAX_MAX_DELTA_TICKS 0xffffffff +#define HV_MIN_DELTA_TICKS 1 + +/* Routines called by the VMbus driver */ +extern int hv_syntimer_alloc(int sint); +extern void hv_syntimer_free(void); +extern int hv_syntimer_cleanup(unsigned int cpu); +extern void hv_syntimer_global_cleanup(void); +extern void hv_stimer0_isr(void); + +#endif
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. 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_syntimer.c | 206 ++++++++++++++++++++++++++++++++++ drivers/hv/hv.c | 154 ------------------------- drivers/hv/hyperv_vmbus.h | 3 - drivers/hv/vmbus_drv.c | 39 +++---- include/clocksource/hyperv_syntimer.h | 26 +++++ 9 files changed, 263 insertions(+), 176 deletions(-) create mode 100644 drivers/clocksource/hyperv_syntimer.c create mode 100644 include/clocksource/hyperv_syntimer.h