From patchwork Tue Jun 2 02:07:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Henrique Cerri X-Patchwork-Id: 1302009 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49bb782Cq8z9sSJ; Tue, 2 Jun 2020 12:08:34 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jfwLs-0005CJ-Ax; Tue, 02 Jun 2020 02:08:28 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jfwLq-0005C7-CD for kernel-team@lists.ubuntu.com; Tue, 02 Jun 2020 02:08:26 +0000 Received: from mail-qv1-f72.google.com ([209.85.219.72]) by youngberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jfwLp-0001xz-US for kernel-team@lists.ubuntu.com; Tue, 02 Jun 2020 02:08:26 +0000 Received: by mail-qv1-f72.google.com with SMTP id t20so2035311qvy.16 for ; Mon, 01 Jun 2020 19:08:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=McMZdUNxEtwbf7ECmzRmX3haNrkLnJ9bCJ/JlYfZECU=; b=YRw2l0ZobLn1Fg//2ORBnT7xhEp/vfW7M7eO8amsBfY63o9ttrnulA4kM87O5546lF xZH6JGnxNpZ+PpLsZ25s3fUQ0Zrz3gFCLtmMdiDOjv+od+QB6n5tjFzL6RUjbH1nRl7e q4WxCEXU9UNbq9C5vDaebRjEhMOy/LpALeqVkHUXbx+HMqyzBeVS2VSjJU06bwEd484h K7IRbGbVwqRoc9/JSoda+nnlycpk/sPBHL5HWzyLSbRyFDaAj/YtV9GEmgaPtQbkziVX 7r91XbY46uuOsldI54cV7XrVhvGNWgONc3LsezXG1ru9r/DIN8siXgi1hxtD/8HvvDYc Y7+g== X-Gm-Message-State: AOAM531lxC9n2vQROa2iJMJZ91IpbCxBGSVhH3M9PURn7f5fPLEpj+/q dZrZTkD2/5YapOtVTikalHA8JFJbhI55wKKwL05lB/6hHv9W05zS9i/YbI2nJ2Iz+/EL3q3QIhH wuC0aYBLy5O8vX3Nxtv32K3UaSuaveZIJ2lwxIO+7 X-Received: by 2002:ac8:374e:: with SMTP id p14mr25368654qtb.361.1591063704184; Mon, 01 Jun 2020 19:08:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwGem6jVVUARlJumFQtgMzKS5bqtJfxBLuDWvQFxgHHfnB2P96EfEcD/L6PnPqDHbtyu3kwgA== X-Received: by 2002:ac8:374e:: with SMTP id p14mr25368628qtb.361.1591063703547; Mon, 01 Jun 2020 19:08:23 -0700 (PDT) Received: from gallifrey.lan ([201.82.186.200]) by smtp.gmail.com with ESMTPSA id t13sm1138342qtc.77.2020.06.01.19.08.21 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 Jun 2020 19:08:22 -0700 (PDT) From: Marcelo Henrique Cerri To: kernel-team@lists.ubuntu.com Subject: [focal:linux-azure][PATCH 01/21] x86/hyperv: Initialize clockevents earlier in CPU onlining Date: Mon, 1 Jun 2020 23:07:57 -0300 Message-Id: <20200602020817.236422-2-marcelo.cerri@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200602020817.236422-1-marcelo.cerri@canonical.com> References: <20200602020817.236422-1-marcelo.cerri@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Michael Kelley BugLink: http://bugs.launchpad.net/bugs/1880032 Hyper-V has historically initialized stimer-based clockevents late in the process of onlining a CPU because clockevents depend on stimer interrupts. In the original Hyper-V design, stimer interrupts generate a VMbus message, so the VMbus machinery must be running first, and VMbus can't be initialized until relatively late. On x86/64, LAPIC timer based clockevents are used during early initialization before VMbus and stimer-based clockevents are ready, and again during CPU offlining after the stimer clockevents have been shut down. Unfortunately, this design creates problems when offlining CPUs for hibernation or other purposes. stimer-based clockevents are shut down relatively early in the offlining process, so clockevents_unbind_device() must be used to fallback to the LAPIC-based clockevents for the remainder of the offlining process. Furthermore, the late initialization and early shutdown of stimer-based clockevents doesn't work well on ARM64 since there is no other timer like the LAPIC to fallback to. So CPU onlining and offlining doesn't work properly. Fix this by recognizing that stimer Direct Mode is the normal path for newer versions of Hyper-V on x86/64, and the only path on other architectures. With stimer Direct Mode, stimer interrupts don't require any VMbus machinery. stimer clockevents can be initialized and shut down consistent with how it is done for other clockevent devices. While the old VMbus-based stimer interrupts must still be supported for backward compatibility on x86, that mode of operation can be treated as legacy. So add a new Hyper-V stimer entry in the CPU hotplug state list, and use that new state when in Direct Mode. Update the Hyper-V clocksource driver to allocate and initialize stimer clockevents earlier during boot. Update Hyper-V initialization and the VMbus driver to use this new design. As a result, the LAPIC timer is no longer used during boot or CPU onlining/offlining and clockevents_unbind_device() is not called. But retain the old design as a legacy implementation for older versions of Hyper-V that don't support Direct Mode. Signed-off-by: Michael Kelley Signed-off-by: Thomas Gleixner Tested-by: Dexuan Cui Reviewed-by: Dexuan Cui Link: https://lkml.kernel.org/r/1573607467-9456-1-git-send-email-mikelley@microsoft.com (cherry picked from commit 4df4cb9e99f83b70d54bc0e25081ac23cceafcbc) Signed-off-by: Marcelo Henrique Cerri --- arch/x86/hyperv/hv_init.c | 6 ++ drivers/clocksource/hyperv_timer.c | 154 +++++++++++++++++++++++------ drivers/hv/hv.c | 4 +- drivers/hv/vmbus_drv.c | 30 +++--- include/clocksource/hyperv_timer.h | 7 +- include/linux/cpuhotplug.h | 1 + 6 files changed, 151 insertions(+), 51 deletions(-) diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 5739fa0886b1..60458769931b 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -320,6 +320,12 @@ void __init hyperv_init(void) hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg); wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); + /* + * Ignore any errors in setting up stimer clockevents + * as we can run with the LAPIC timer as a fallback. + */ + (void)hv_stimer_alloc(); + hv_apic_init(); x86_init.pci.arch_init = hv_pci_init; diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index 36933e2b3b0d..0d29b05d8889 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,15 @@ static u64 hv_sched_clock_offset __ro_after_init; * 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. + * + * Because Direct Mode does not require processing a VMbus + * message, stimer interrupts can be enabled earlier in the + * process of booting a CPU, and consistent with when timer + * interrupts are enabled for other clocksource drivers. + * However, for legacy versions of Hyper-V when Direct Mode + * is not enabled, setting up stimer interrupts must be + * delayed until VMbus is initialized and can process the + * interrupt message. */ static bool direct_mode_enabled; @@ -102,17 +112,12 @@ static int hv_ce_set_oneshot(struct clock_event_device *evt) /* * hv_stimer_init - Per-cpu initialization of the clockevent */ -void hv_stimer_init(unsigned int cpu) +static int hv_stimer_init(unsigned int cpu) { struct clock_event_device *ce; - /* - * Synthetic timers are always available except on old versions of - * Hyper-V on x86. In that case, just return as Linux will use a - * clocksource based on emulated PIT or LAPIC timer hardware. - */ - if (!(ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE)) - return; + if (!hv_clock_event) + return 0; ce = per_cpu_ptr(hv_clock_event, cpu); ce->name = "Hyper-V clockevent"; @@ -127,28 +132,55 @@ void hv_stimer_init(unsigned int cpu) 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 */ -void hv_stimer_cleanup(unsigned int cpu) +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); + if (!hv_clock_event) + return 0; + + /* + * In the legacy case where Direct Mode is not enabled + * (which can only be on x86/64), stimer cleanup happens + * relatively early in the CPU offlining process. We + * must unbind the stimer-based clockevent device so + * that the LAPIC timer can take over until clockevents + * are no longer needed in the offlining process. Note + * that clockevents_unbind_device() eventually calls + * hv_ce_shutdown(). + * + * The unbind should not be done when Direct Mode is + * enabled because we may be on an architecture where + * there are no other clockevent devices to fallback to. + */ + ce = per_cpu_ptr(hv_clock_event, cpu); + if (direct_mode_enabled) hv_ce_shutdown(ce); - } + else + clockevents_unbind_device(ce, cpu); + + return 0; } EXPORT_SYMBOL_GPL(hv_stimer_cleanup); /* hv_stimer_alloc - Global initialization of the clockevent and stimer0 */ -int hv_stimer_alloc(int sint) +int hv_stimer_alloc(void) { - int ret; + int ret = 0; + + /* + * Synthetic timers are always available except on old versions of + * Hyper-V on x86. In that case, return as error as Linux will use a + * clockevent based on emulated LAPIC timer hardware. + */ + if (!(ms_hyperv.features & HV_MSR_SYNTIMER_AVAILABLE)) + return -EINVAL; hv_clock_event = alloc_percpu(struct clock_event_device); if (!hv_clock_event) @@ -159,22 +191,78 @@ int hv_stimer_alloc(int sint) if (direct_mode_enabled) { ret = hv_setup_stimer0_irq(&stimer0_irq, &stimer0_vector, hv_stimer0_isr); - if (ret) { - free_percpu(hv_clock_event); - hv_clock_event = NULL; - return ret; - } + if (ret) + goto free_percpu; + + /* + * Since we are in Direct Mode, stimer initialization + * can be done now with a CPUHP value in the same range + * as other clockevent devices. + */ + ret = cpuhp_setup_state(CPUHP_AP_HYPERV_TIMER_STARTING, + "clockevents/hyperv/stimer:starting", + hv_stimer_init, hv_stimer_cleanup); + if (ret < 0) + goto free_stimer0_irq; } + return ret; - stimer0_message_sint = sint; - return 0; +free_stimer0_irq: + hv_remove_stimer0_irq(stimer0_irq); + stimer0_irq = 0; +free_percpu: + free_percpu(hv_clock_event); + hv_clock_event = NULL; + return ret; } EXPORT_SYMBOL_GPL(hv_stimer_alloc); +/* + * hv_stimer_legacy_init -- Called from the VMbus driver to handle + * the case when Direct Mode is not enabled, and the stimer + * must be initialized late in the CPU onlining process. + * + */ +void hv_stimer_legacy_init(unsigned int cpu, int sint) +{ + if (direct_mode_enabled) + return; + + /* + * This function gets called by each vCPU, so setting the + * global stimer_message_sint value each time is conceptually + * not ideal, but the value passed in is always the same and + * it avoids introducing yet another interface into this + * clocksource driver just to set the sint in the legacy case. + */ + stimer0_message_sint = sint; + (void)hv_stimer_init(cpu); +} +EXPORT_SYMBOL_GPL(hv_stimer_legacy_init); + +/* + * hv_stimer_legacy_cleanup -- Called from the VMbus driver to + * handle the case when Direct Mode is not enabled, and the + * stimer must be cleaned up early in the CPU offlining + * process. + */ +void hv_stimer_legacy_cleanup(unsigned int cpu) +{ + if (direct_mode_enabled) + return; + (void)hv_stimer_cleanup(cpu); +} +EXPORT_SYMBOL_GPL(hv_stimer_legacy_cleanup); + + /* hv_stimer_free - Free global resources allocated by hv_stimer_alloc() */ void hv_stimer_free(void) { - if (direct_mode_enabled && (stimer0_irq != 0)) { + if (!hv_clock_event) + return; + + if (direct_mode_enabled) { + cpuhp_remove_state(CPUHP_AP_HYPERV_TIMER_STARTING); hv_remove_stimer0_irq(stimer0_irq); stimer0_irq = 0; } @@ -190,14 +278,20 @@ EXPORT_SYMBOL_GPL(hv_stimer_free); 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_stime_legacy_cleanup() will stop the stimer if Direct + * Mode is not enabled, and fallback to the LAPIC timer. + */ + for_each_present_cpu(cpu) { + hv_stimer_legacy_cleanup(cpu); } + + /* + * If Direct Mode is enabled, the cpuhp teardown callback + * (hv_stimer_cleanup) will be run on all CPUs to stop the + * stimers. + */ hv_stimer_free(); } EXPORT_SYMBOL_GPL(hv_stimer_global_cleanup); diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index fcc52797c169..6098e0cbdb4b 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -202,7 +202,7 @@ int hv_synic_init(unsigned int cpu) { hv_synic_enable_regs(cpu); - hv_stimer_init(cpu); + hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT); return 0; } @@ -277,7 +277,7 @@ int hv_synic_cleanup(unsigned int cpu) if (channel_found && vmbus_connection.conn_state == CONNECTED) return -EBUSY; - hv_stimer_cleanup(cpu); + hv_stimer_legacy_cleanup(cpu); hv_synic_disable_regs(cpu); diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 9cdd434bb340..a8648fe63357 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1375,10 +1375,6 @@ static int vmbus_bus_init(void) 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 stimer state. * Then connect to the host. @@ -1445,9 +1441,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(); +err_alloc: hv_remove_vmbus_irq(); bus_unregister(&hv_bus); @@ -2366,20 +2361,23 @@ static void hv_crash_handler(struct pt_regs *regs) static int hv_synic_suspend(void) { /* - * When we reach here, all the non-boot CPUs have been offlined, and - * the stimers on them have been unbound in hv_synic_cleanup() -> + * When we reach here, all the non-boot CPUs have been offlined. + * If we're in a legacy configuration where stimer Direct Mode is + * not enabled, the stimers on the non-boot CPUs have been unbound + * in hv_synic_cleanup() -> hv_stimer_legacy_cleanup() -> * hv_stimer_cleanup() -> clockevents_unbind_device(). * - * hv_synic_suspend() only runs on CPU0 with interrupts disabled. Here - * we do not unbind the stimer on CPU0 because: 1) it's unnecessary - * because the interrupts remain disabled between syscore_suspend() - * and syscore_resume(): see create_image() and resume_target_kernel(); + * hv_synic_suspend() only runs on CPU0 with interrupts disabled. + * Here we do not call hv_stimer_legacy_cleanup() on CPU0 because: + * 1) it's unnecessary as interrupts remain disabled between + * syscore_suspend() and syscore_resume(): see create_image() and + * resume_target_kernel() * 2) the stimer on CPU0 is automatically disabled later by * syscore_suspend() -> timekeeping_suspend() -> tick_suspend() -> ... - * -> clockevents_shutdown() -> ... -> hv_ce_shutdown(); 3) a warning - * would be triggered if we call clockevents_unbind_device(), which - * may sleep, in an interrupts-disabled context. So, we intentionally - * don't call hv_stimer_cleanup(0) here. + * -> clockevents_shutdown() -> ... -> hv_ce_shutdown() + * 3) a warning would be triggered if we call + * clockevents_unbind_device(), which may sleep, in an + * interrupts-disabled context. */ hv_synic_disable_regs(0); diff --git a/include/clocksource/hyperv_timer.h b/include/clocksource/hyperv_timer.h index 422f5e5237be..553e539469f0 100644 --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -21,10 +21,11 @@ #define HV_MIN_DELTA_TICKS 1 /* Routines called by the VMbus driver */ -extern int hv_stimer_alloc(int sint); +extern int hv_stimer_alloc(void); extern void hv_stimer_free(void); -extern void hv_stimer_init(unsigned int cpu); -extern void hv_stimer_cleanup(unsigned int cpu); +extern int hv_stimer_cleanup(unsigned int cpu); +extern void hv_stimer_legacy_init(unsigned int cpu, int sint); +extern void hv_stimer_legacy_cleanup(unsigned int cpu); extern void hv_stimer_global_cleanup(void); extern void hv_stimer0_isr(void); diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h index 2d55cee638fc..363e81766823 100644 --- a/include/linux/cpuhotplug.h +++ b/include/linux/cpuhotplug.h @@ -130,6 +130,7 @@ enum cpuhp_state { CPUHP_AP_ARC_TIMER_STARTING, CPUHP_AP_RISCV_TIMER_STARTING, CPUHP_AP_CSKY_TIMER_STARTING, + CPUHP_AP_HYPERV_TIMER_STARTING, CPUHP_AP_KVM_STARTING, CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING, CPUHP_AP_KVM_ARM_VGIC_STARTING,