RE: [PATCH] clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h not asm/mshyperv.h

Michael Kelley (LINUX)
2022-11-12T21:56:06Z

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)
> {