Message ID | 87zgcwt2qg.ffs@tglx |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1396645wru; Sat, 12 Nov 2022 11:14:19 -0800 (PST) X-Google-Smtp-Source: AA0mqf5yxcl1Z5n8FBq1syM9g8ztSb15odC+Fl3z2IYiaILgWd+26czaEyQaAe4ZpHucte1qAmxt X-Received: by 2002:a05:6402:43c7:b0:460:19c3:2992 with SMTP id p7-20020a05640243c700b0046019c32992mr6048424edc.1.1668280459459; Sat, 12 Nov 2022 11:14:19 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668280459; cv=none; d=google.com; s=arc-20160816; b=ln49IltLX5L/vuRBPRjOlqrB005lt7DyagRATfh7qXGjOHj1LM6a1fq0r6sJPt317r dAhBa+TEaKxklW127l0fpsoGZycUxCnQjhqG8HWZMeW2ZMZm8ulkPfIv669hmGJnJ5of czTXSRqs3oG45Vz+yY1c8WxSEHYwkGotl37uAz+iS5vwSyPQxIqn0/1K9ZF05qAmP/68 DaA6DBBATqpBkOQhB3u1flO2xC66Zvv32k2AwUGYLNRQL46Fb33hC5ZMOeHMIJQs1Weo QjBtBcX/O1TYnUPlwkB0zPGeWwBKH2c1GIC8QKIwAvPxul9ahtgY7Fn2LJz7UKStY3XG B2Xw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:subject:cc:to :dkim-signature:dkim-signature:from; bh=UlPHg7oN9UJeMU3FvPvwIBheN+dHkzkfX0GVcGqmqwI=; b=Ttdi4tzc2Dc8g5EYjLbFfI2ZY4P2TT1xx6SvzulVDrh3kn2dVPMYI40mFlth5QbU2o yKmYEVx1Cn4Jeduac+wAiMYVy5Uca3kMB6YuIvRj4aiCjYGoU+SAtr/gYPLAcRMhXmmJ KI1e88uJJlKNVyoQhSdUA8+RElwgLR+G8JamlGHhYPFUU162DnVe3eDqLvG7yat9wjCc QD5r0VmpNZy1Jmdo4mXpaX81lIXLH+cnZGKwmDUXzo6fDxD5E+F2q4Pvj4NfFPA8ctSU 1JMzvvEpMyOaMT/3kNjpdL8YclWciWoiRLvfltdZISQ2iBQy5UzQBZh1axy2XgX4JrhH KgZw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linutronix.de header.s=2020 header.b=UWKjKP7A; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id d6-20020aa7d5c6000000b004596db363c4si4704325eds.264.2022.11.12.11.13.56; Sat, 12 Nov 2022 11:14:19 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@linutronix.de header.s=2020 header.b=UWKjKP7A; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235078AbiKLTDJ (ORCPT <rfc822;winker.wchi@gmail.com> + 99 others); Sat, 12 Nov 2022 14:03:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51544 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230257AbiKLTDH (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 12 Nov 2022 14:03:07 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 690F32706; Sat, 12 Nov 2022 11:03:06 -0800 (PST) From: Thomas Gleixner <tglx@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1668279784; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=UlPHg7oN9UJeMU3FvPvwIBheN+dHkzkfX0GVcGqmqwI=; b=UWKjKP7Aw9LNViF8bq4SlSEZGztBoENYR3B+xt2fLeqdlkYYzKlyMSXQ9IA5SOddDh9fSP Zq0FBB9wCsaZxoaq/6nPJNEoFOOAJ+bIA4Gm4f8xeptwYYZ7ydl7Wywpb97LCs0FxwQqTk biatQUQrZmuwResgDPFP1aS+Zg/OTi0AAtCqjUACSf0MhDHQ0DGJeHzrXe49x4CKjRL2ur ZwBU3NtCDu6hZpg4kSs8L5SP4ziAUwxNDc7Xowr8BHqfLajLFXTbEdrS8HmEhLM1owakfh zon+UbZenOssKE3dbYK3Cd6MdHXiXi0h4sRF5DN8twxaoSvnp6nNHGG7ySJY3w== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1668279784; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=UlPHg7oN9UJeMU3FvPvwIBheN+dHkzkfX0GVcGqmqwI=; b=UhlvPMsdlaHdxfKyllnCqK/WOiyDUNsoQc0Mj8Xb0nn9NW2ve9BmOBQpHKgCMMFkR6btNT 1q13Fd+Djka/OdCg== To: LKML <linux-kernel@vger.kernel.org> Cc: linux-hyperv@vger.kernel.org, Haiyang Zhang <haiyangz@microsoft.com>, Wei Liu <wei.liu@kernel.org>, Dexuan Cui <decui@microsoft.com>, Michael Kelley <mikelley@microsoft.com>, Andy Lutomirski <luto@kernel.org>, Vincenzo Frascino <vincenzo.frascino@arm.com>, Daniel Lezcano <daniel.lezcano@linaro.org> Subject: [PATCH] clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h Date: Sat, 12 Nov 2022 20:03:03 +0100 Message-ID: <87zgcwt2qg.ffs@tglx> MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749318851125877705?= X-GMAIL-MSGID: =?utf-8?q?1749318851125877705?= |
Series |
clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h
|
|
Commit Message
Thomas Gleixner
Nov. 12, 2022, 7:03 p.m. UTC
clocksource/hyperv_timer.h is included into the VDSO build. It includes
asm/mshyperv.h which in turn includes the world and some more.
This worked so far by chance, but any subtle change in the include chain
results in a build breakage because VDSO builds are building user space
libraries.
Include asm/hyperv-tlfs.h instead which contains everything what the VDSO
build needs and move the hv_get_raw_timer() define into the header file.
Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of
asm/mshyperv.h.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/include/asm/mshyperv.h | 2 --
drivers/hv/vmbus_drv.c | 1 +
include/clocksource/hyperv_timer.h | 4 +++-
3 files changed, 4 insertions(+), 3 deletions(-)
Comments
From: Thomas Gleixner <tglx@linutronix.de> Sent: Saturday, November 12, 2022 11:03 AM > > clocksource/hyperv_timer.h is included into the VDSO build. It includes > asm/mshyperv.h which in turn includes the world and some more. > > This worked so far by chance, but any subtle change in the include chain > results in a build breakage because VDSO builds are building user space > libraries. > > Include asm/hyperv-tlfs.h instead which contains everything what the VDSO > build needs and move the hv_get_raw_timer() define into the header file. We've been keeping x86-isms out of hyperv_timer.c and hyperv_timer.h so that they are architecture independent. That's why the #define for hv_get_raw_timer() is in an x86-specific include file. But I can see the problem with too much getting dragged into the VDSO builds. If hv_get_raw_timer() is added to hyperv_timer.h, it should be under #ifdef CONFIG_X86. Adding an #ifdef isn't ideal, and a more more proper solution might be to have a separate hyperv_timer.h include file under arch/x86/include/asm. But the latter seems like overkill for just hv_get_raw_timer(), so I'm OK with the #ifdef. Or does someone have a better idea? Michael > > Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of > asm/mshyperv.h. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/include/asm/mshyperv.h | 2 -- > drivers/hv/vmbus_drv.c | 1 + > include/clocksource/hyperv_timer.h | 4 +++- > 3 files changed, 4 insertions(+), 3 deletions(-) > > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun > struct hv_guest_mapping_flush_list *flush, > void *data); > > -#define hv_get_raw_timer() rdtsc_ordered() > - > void hyperv_vector_handler(struct pt_regs *regs); > > #if IS_ENABLED(CONFIG_HYPERV) > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -37,6 +37,7 @@ > #include <linux/dma-map-ops.h> > #include <linux/pci.h> > #include <clocksource/hyperv_timer.h> > +#include <asm/mshyperv.h> > #include "hyperv_vmbus.h" > > struct vmbus_dynid { > --- a/include/clocksource/hyperv_timer.h > +++ b/include/clocksource/hyperv_timer.h > @@ -15,7 +15,7 @@ > > #include <linux/clocksource.h> > #include <linux/math64.h> > -#include <asm/mshyperv.h> > +#include <asm/hyperv-tlfs.h> > > #define HV_MAX_MAX_DELTA_TICKS 0xffffffff > #define HV_MIN_DELTA_TICKS 1 > @@ -34,6 +34,8 @@ extern void hv_init_clocksource(void); > > extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void); > > +#define hv_get_raw_timer() rdtsc_ordered() > + > static inline notrace u64 > hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) > {
On Sat, Nov 12 2022 at 21:55, Michael Kelley wrote: > But I can see the problem with too much getting dragged into the VDSO > builds. If hv_get_raw_timer() is added to hyperv_timer.h, it should > be under #ifdef CONFIG_X86. Adding an #ifdef isn't ideal, and a more > more proper solution might be to have a separate hyperv_timer.h include > file under arch/x86/include/asm. But the latter seems like overkill for just > hv_get_raw_timer(), so I'm OK with the #ifdef. We surely can have asm/hyperv_timer.h but TBH: >> static inline notrace u64 >> hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) >> { hv_read_tsc_page_tsc() does not look architecture agnostic either. TSC is pretty x86 specific :) Thanks, tglx
On Sun, Nov 13 2022 at 10:50, Thomas Gleixner wrote: > On Sat, Nov 12 2022 at 21:55, Michael Kelley wrote: >> But I can see the problem with too much getting dragged into the VDSO >> builds. If hv_get_raw_timer() is added to hyperv_timer.h, it should >> be under #ifdef CONFIG_X86. Adding an #ifdef isn't ideal, and a more >> more proper solution might be to have a separate hyperv_timer.h include >> file under arch/x86/include/asm. But the latter seems like overkill for just >> hv_get_raw_timer(), so I'm OK with the #ifdef. > > We surely can have asm/hyperv_timer.h but TBH: > >>> static inline notrace u64 >>> hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) >>> { > > hv_read_tsc_page_tsc() does not look architecture agnostic either. TSC > is pretty x86 specific :) Though the below makes sense on its own because it ensures that msr.h is included, which is required for making clocksource/hyperv_timer.h self contained. Thanks, tglx --- Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h From: Thomas Gleixner <tglx@linutronix.de> Date: Sat, 12 Nov 2022 19:08:15 +0100 clocksource/hyperv_timer.h is included into the VDSO build. It includes asm/mshyperv.h which in turn includes the world and some more. This worked so far by chance, but any subtle change in the include chain results in a build breakage because VDSO builds are building user space libraries. Include asm/hyperv-tlfs.h instead which contains everything what the VDSO build needs and move the hv_get_raw_timer() define into a separate header file which also includes asm/msr.h to resolve rdtsc_ordered(). Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of asm/mshyperv.h. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/include/asm/hyperv_timer.h | 9 +++++++++ arch/x86/include/asm/mshyperv.h | 2 -- drivers/hv/vmbus_drv.c | 1 + include/clocksource/hyperv_timer.h | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) --- /dev/null +++ b/arch/x86/include/asm/hyperv_timer.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_HYPERV_TIMER_H +#define _ASM_X86_HYPERV_TIMER_H + +#include <asm/msr.h> + +#define hv_get_raw_timer() rdtsc_ordered() + +#endif --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun struct hv_guest_mapping_flush_list *flush, void *data); -#define hv_get_raw_timer() rdtsc_ordered() - void hyperv_vector_handler(struct pt_regs *regs); #if IS_ENABLED(CONFIG_HYPERV) --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -37,6 +37,7 @@ #include <linux/dma-map-ops.h> #include <linux/pci.h> #include <clocksource/hyperv_timer.h> +#include <asm/mshyperv.h> #include "hyperv_vmbus.h" struct vmbus_dynid { --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -15,7 +15,7 @@ #include <linux/clocksource.h> #include <linux/math64.h> -#include <asm/mshyperv.h> +#include <asm/hyperv-tlfs.h> #define HV_MAX_MAX_DELTA_TICKS 0xffffffff #define HV_MIN_DELTA_TICKS 1
On Sun, Nov 13 2022 at 11:33, Thomas Gleixner wrote: > --- a/include/clocksource/hyperv_timer.h > +++ b/include/clocksource/hyperv_timer.h > @@ -15,7 +15,7 @@ > > #include <linux/clocksource.h> > #include <linux/math64.h> > -#include <asm/mshyperv.h> > +#include <asm/hyperv-tlfs.h> Bah, that obviously wants to include the new header... --- Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h From: Thomas Gleixner <tglx@linutronix.de> Date: Sat, 12 Nov 2022 19:08:15 +0100 clocksource/hyperv_timer.h is included into the VDSO build. It includes asm/mshyperv.h which in turn includes the world and some more. This worked so far by chance, but any subtle change in the include chain results in a build breakage because VDSO builds are building user space libraries. Include asm/hyperv-tlfs.h instead which contains everything what the VDSO build needs and move the hv_get_raw_timer() define into the header file. Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of asm/mshyperv.h. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/include/asm/hyperv_timer.h | 9 +++++++++ arch/x86/include/asm/mshyperv.h | 2 -- drivers/hv/vmbus_drv.c | 1 + include/clocksource/hyperv_timer.h | 3 ++- 4 files changed, 12 insertions(+), 3 deletions(-) --- /dev/null +++ b/arch/x86/include/asm/hyperv_timer.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_HYPERV_TIMER_H +#define _ASM_X86_HYPERV_TIMER_H + +#include <asm/msr.h> + +#define hv_get_raw_timer() rdtsc_ordered() + +#endif --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun struct hv_guest_mapping_flush_list *flush, void *data); -#define hv_get_raw_timer() rdtsc_ordered() - void hyperv_vector_handler(struct pt_regs *regs); #if IS_ENABLED(CONFIG_HYPERV) --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -37,6 +37,7 @@ #include <linux/dma-map-ops.h> #include <linux/pci.h> #include <clocksource/hyperv_timer.h> +#include <asm/mshyperv.h> #include "hyperv_vmbus.h" struct vmbus_dynid { --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -15,7 +15,8 @@ #include <linux/clocksource.h> #include <linux/math64.h> -#include <asm/mshyperv.h> +#include <asm/hyperv_timer.h> +#include <asm/hyperv-tlfs.h> #define HV_MAX_MAX_DELTA_TICKS 0xffffffff #define HV_MIN_DELTA_TICKS 1
From: Thomas Gleixner <tglx@linutronix.de> Sent: Sunday, November 13, 2022 1:50 AM > > On Sat, Nov 12 2022 at 21:55, Michael Kelley wrote: > > But I can see the problem with too much getting dragged into the VDSO > > builds. If hv_get_raw_timer() is added to hyperv_timer.h, it should > > be under #ifdef CONFIG_X86. Adding an #ifdef isn't ideal, and a more > > more proper solution might be to have a separate hyperv_timer.h include > > file under arch/x86/include/asm. But the latter seems like overkill for just > > hv_get_raw_timer(), so I'm OK with the #ifdef. > > We surely can have asm/hyperv_timer.h but TBH: > > >> static inline notrace u64 > >> hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) > >> { > > hv_read_tsc_page_tsc() does not look architecture agnostic either. TSC > is pretty x86 specific :) Yes, the naming still says "tsc". But there's nothing in the code that actually requires the TSC if hv_get_raw_timer() maps to some other hardware counter on a different architecture. That's why the hv_get_raw_timer() abstraction is there in the first place. If we didn't care about x86-isms, hv_read_tsc_page_tsc() would just directly invoke rdtsc_ordered(). Michael > > Thanks, > > tglx >
On Sun, Nov 13 2022 at 13:12, Michael Kelley wrote: > From: Thomas Gleixner <tglx@linutronix.de> Sent: Sunday, November 13, 2022 1:50 AM >> On Sat, Nov 12 2022 at 21:55, Michael Kelley wrote: >> > But I can see the problem with too much getting dragged into the VDSO >> > builds. If hv_get_raw_timer() is added to hyperv_timer.h, it should >> > be under #ifdef CONFIG_X86. Adding an #ifdef isn't ideal, and a more >> > more proper solution might be to have a separate hyperv_timer.h include >> > file under arch/x86/include/asm. But the latter seems like overkill for just >> > hv_get_raw_timer(), so I'm OK with the #ifdef. >> >> We surely can have asm/hyperv_timer.h but TBH: >> >> >> static inline notrace u64 >> >> hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) >> >> { >> >> hv_read_tsc_page_tsc() does not look architecture agnostic either. TSC >> is pretty x86 specific :) > > Yes, the naming still says "tsc". But there's nothing in the code that actually > requires the TSC if hv_get_raw_timer() maps to some other hardware counter on > a different architecture. That's why the hv_get_raw_timer() abstraction is there > in the first place. If we didn't care about x86-isms, hv_read_tsc_page_tsc() would > just directly invoke rdtsc_ordered(). Not really intuitive, but anyway...
On Sun, Nov 13 2022 at 11:40, Thomas Gleixner wrote: > Bah, that obviously wants to include the new header... > > --- a/include/clocksource/hyperv_timer.h > +++ b/include/clocksource/hyperv_timer.h > @@ -15,7 +15,8 @@ > > #include <linux/clocksource.h> > #include <linux/math64.h> > -#include <asm/mshyperv.h> > +#include <asm/hyperv_timer.h> and that breaks when CONFIG_HYPERV_TIMER=n and compiled for ARM64. Sigh... --- Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h From: Thomas Gleixner <tglx@linutronix.de> Date: Sat, 12 Nov 2022 19:08:15 +0100 clocksource/hyperv_timer.h is included into the VDSO build. It includes asm/mshyperv.h which in turn includes the world and some more. This worked so far by chance, but any subtle change in the include chain results in a build breakage because VDSO builds are building user space libraries. Include asm/hyperv-tlfs.h instead which contains everything what the VDSO build needs and move the hv_get_raw_timer() define into the header file. Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of asm/mshyperv.h. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/include/asm/hyperv_timer.h | 9 +++++++++ arch/x86/include/asm/mshyperv.h | 2 -- drivers/hv/vmbus_drv.c | 1 + include/clocksource/hyperv_timer.h | 4 +++- 4 files changed, 13 insertions(+), 3 deletions(-) --- /dev/null +++ b/arch/x86/include/asm/hyperv_timer.h @@ -0,0 +1,9 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_HYPERV_TIMER_H +#define _ASM_X86_HYPERV_TIMER_H + +#include <asm/msr.h> + +#define hv_get_raw_timer() rdtsc_ordered() + +#endif --- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun struct hv_guest_mapping_flush_list *flush, void *data); -#define hv_get_raw_timer() rdtsc_ordered() - void hyperv_vector_handler(struct pt_regs *regs); #if IS_ENABLED(CONFIG_HYPERV) --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -37,6 +37,7 @@ #include <linux/dma-map-ops.h> #include <linux/pci.h> #include <clocksource/hyperv_timer.h> +#include <asm/mshyperv.h> #include "hyperv_vmbus.h" struct vmbus_dynid { --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -15,13 +15,15 @@ #include <linux/clocksource.h> #include <linux/math64.h> -#include <asm/mshyperv.h> +#include <asm/hyperv-tlfs.h> #define HV_MAX_MAX_DELTA_TICKS 0xffffffff #define HV_MIN_DELTA_TICKS 1 #ifdef CONFIG_HYPERV_TIMER +#include <asm/hyperv_timer.h> + /* Routines called by the VMbus driver */ extern int hv_stimer_alloc(bool have_percpu_irqs); extern int hv_stimer_cleanup(unsigned int cpu);
Michael! On Sun, Nov 13 2022 at 22:21, Thomas Gleixner wrote: > Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h > From: Thomas Gleixner <tglx@linutronix.de> > Date: Sat, 12 Nov 2022 19:08:15 +0100 > > clocksource/hyperv_timer.h is included into the VDSO build. It includes > asm/mshyperv.h which in turn includes the world and some more. This worked > so far by chance, but any subtle change in the include chain results in a > build breakage because VDSO builds are building user space libraries. > > Include asm/hyperv-tlfs.h instead which contains everything what the VDSO > build needs and move the hv_get_raw_timer() define into the header file. > > Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of > asm/mshyperv.h. Any comments on this latest version? > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/include/asm/hyperv_timer.h | 9 +++++++++ > arch/x86/include/asm/mshyperv.h | 2 -- > drivers/hv/vmbus_drv.c | 1 + > include/clocksource/hyperv_timer.h | 4 +++- > 4 files changed, 13 insertions(+), 3 deletions(-) > > --- /dev/null > +++ b/arch/x86/include/asm/hyperv_timer.h > @@ -0,0 +1,9 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_X86_HYPERV_TIMER_H > +#define _ASM_X86_HYPERV_TIMER_H > + > +#include <asm/msr.h> > + > +#define hv_get_raw_timer() rdtsc_ordered() > + > +#endif > --- a/arch/x86/include/asm/mshyperv.h > +++ b/arch/x86/include/asm/mshyperv.h > @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun > struct hv_guest_mapping_flush_list *flush, > void *data); > > -#define hv_get_raw_timer() rdtsc_ordered() > - > void hyperv_vector_handler(struct pt_regs *regs); > > #if IS_ENABLED(CONFIG_HYPERV) > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -37,6 +37,7 @@ > #include <linux/dma-map-ops.h> > #include <linux/pci.h> > #include <clocksource/hyperv_timer.h> > +#include <asm/mshyperv.h> > #include "hyperv_vmbus.h" > > struct vmbus_dynid { > --- a/include/clocksource/hyperv_timer.h > +++ b/include/clocksource/hyperv_timer.h > @@ -15,13 +15,15 @@ > > #include <linux/clocksource.h> > #include <linux/math64.h> > -#include <asm/mshyperv.h> > +#include <asm/hyperv-tlfs.h> > > #define HV_MAX_MAX_DELTA_TICKS 0xffffffff > #define HV_MIN_DELTA_TICKS 1 > > #ifdef CONFIG_HYPERV_TIMER > > +#include <asm/hyperv_timer.h> > + > /* Routines called by the VMbus driver */ > extern int hv_stimer_alloc(bool have_percpu_irqs); > extern int hv_stimer_cleanup(unsigned int cpu);
From: Thomas Gleixner <tglx@linutronix.de> Sent: Wednesday, November 16, 2022 12:52 PM > > Michael! > > On Sun, Nov 13 2022 at 22:21, Thomas Gleixner wrote: > > Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not > asm/mshyperv.h > > From: Thomas Gleixner <tglx@linutronix.de> > > Date: Sat, 12 Nov 2022 19:08:15 +0100 > > > > clocksource/hyperv_timer.h is included into the VDSO build. It includes > > asm/mshyperv.h which in turn includes the world and some more. This worked > > so far by chance, but any subtle change in the include chain results in a > > build breakage because VDSO builds are building user space libraries. > > > > Include asm/hyperv-tlfs.h instead which contains everything what the VDSO > > build needs and move the hv_get_raw_timer() define into the header file. > > > > Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of > > asm/mshyperv.h. > > Any comments on this latest version? Sorry. This looks good to me. Maybe the commit message needs a bit of tweaking -- it's not clear what "move hv_get_raw_timer() define into the header file" exactly refers to. But otherwise, Reviewed-by: Michael Kelley <mikelley@microsoft.com> > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > --- > > arch/x86/include/asm/hyperv_timer.h | 9 +++++++++ > > arch/x86/include/asm/mshyperv.h | 2 -- > > drivers/hv/vmbus_drv.c | 1 + > > include/clocksource/hyperv_timer.h | 4 +++- > > 4 files changed, 13 insertions(+), 3 deletions(-) > > > > --- /dev/null > > +++ b/arch/x86/include/asm/hyperv_timer.h > > @@ -0,0 +1,9 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _ASM_X86_HYPERV_TIMER_H > > +#define _ASM_X86_HYPERV_TIMER_H > > + > > +#include <asm/msr.h> > > + > > +#define hv_get_raw_timer() rdtsc_ordered() > > + > > +#endif > > --- a/arch/x86/include/asm/mshyperv.h > > +++ b/arch/x86/include/asm/mshyperv.h > > @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun > > struct hv_guest_mapping_flush_list *flush, > > void *data); > > > > -#define hv_get_raw_timer() rdtsc_ordered() > > - > > void hyperv_vector_handler(struct pt_regs *regs); > > > > #if IS_ENABLED(CONFIG_HYPERV) > > --- a/drivers/hv/vmbus_drv.c > > +++ b/drivers/hv/vmbus_drv.c > > @@ -37,6 +37,7 @@ > > #include <linux/dma-map-ops.h> > > #include <linux/pci.h> > > #include <clocksource/hyperv_timer.h> > > +#include <asm/mshyperv.h> > > #include "hyperv_vmbus.h" > > > > struct vmbus_dynid { > > --- a/include/clocksource/hyperv_timer.h > > +++ b/include/clocksource/hyperv_timer.h > > @@ -15,13 +15,15 @@ > > > > #include <linux/clocksource.h> > > #include <linux/math64.h> > > -#include <asm/mshyperv.h> > > +#include <asm/hyperv-tlfs.h> > > > > #define HV_MAX_MAX_DELTA_TICKS 0xffffffff > > #define HV_MIN_DELTA_TICKS 1 > > > > #ifdef CONFIG_HYPERV_TIMER > > > > +#include <asm/hyperv_timer.h> > > + > > /* Routines called by the VMbus driver */ > > extern int hv_stimer_alloc(bool have_percpu_irqs); > > extern int hv_stimer_cleanup(unsigned int cpu);
On Wed, Nov 16 2022 at 22:34, Michael Kelley wrote: > From: Thomas Gleixner <tglx@linutronix.de> Sent: Wednesday, November 16, 2022 12:52 PM >> On Sun, Nov 13 2022 at 22:21, Thomas Gleixner wrote: >> > Subject: clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not >> asm/mshyperv.h >> > From: Thomas Gleixner <tglx@linutronix.de> >> > Date: Sat, 12 Nov 2022 19:08:15 +0100 >> > >> > clocksource/hyperv_timer.h is included into the VDSO build. It includes >> > asm/mshyperv.h which in turn includes the world and some more. This worked >> > so far by chance, but any subtle change in the include chain results in a >> > build breakage because VDSO builds are building user space libraries. >> > >> > Include asm/hyperv-tlfs.h instead which contains everything what the VDSO >> > build needs and move the hv_get_raw_timer() define into the header file. >> > >> > Fixup drivers/hv/vmbus_drv.c which relies on the indirect include of >> > asm/mshyperv.h. >> >> Any comments on this latest version? > > Sorry. This looks good to me. Maybe the commit message needs a > bit of tweaking -- it's not clear what "move hv_get_raw_timer() > define into the header file" exactly refers to. But otherwise, That should obviously be 'into a separate header file, which is included from clocksource/hyperv_timer.h' or something like that. Thanks, tglx
--- a/arch/x86/include/asm/mshyperv.h +++ b/arch/x86/include/asm/mshyperv.h @@ -19,8 +19,6 @@ typedef int (*hyperv_fill_flush_list_fun struct hv_guest_mapping_flush_list *flush, void *data); -#define hv_get_raw_timer() rdtsc_ordered() - void hyperv_vector_handler(struct pt_regs *regs); #if IS_ENABLED(CONFIG_HYPERV) --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -37,6 +37,7 @@ #include <linux/dma-map-ops.h> #include <linux/pci.h> #include <clocksource/hyperv_timer.h> +#include <asm/mshyperv.h> #include "hyperv_vmbus.h" struct vmbus_dynid { --- a/include/clocksource/hyperv_timer.h +++ b/include/clocksource/hyperv_timer.h @@ -15,7 +15,7 @@ #include <linux/clocksource.h> #include <linux/math64.h> -#include <asm/mshyperv.h> +#include <asm/hyperv-tlfs.h> #define HV_MAX_MAX_DELTA_TICKS 0xffffffff #define HV_MIN_DELTA_TICKS 1 @@ -34,6 +34,8 @@ extern void hv_init_clocksource(void); extern struct ms_hyperv_tsc_page *hv_get_tsc_page(void); +#define hv_get_raw_timer() rdtsc_ordered() + static inline notrace u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg, u64 *cur_tsc) {