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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BYAPR21MB1688424DD14B46712F2992E9D7079@BYAPR21MB1688.namprd21.prod.outlook.com>
Date:   Wed, 16 Nov 2022 22:34:35 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        Andy Lutomirski <luto@...nel.org>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: RE: [PATCH] clocksource/drivers/hyper-v: Include asm/hyperv-tlfs.h
 not asm/mshyperv.h

From: Thomas Gleixner <tglx@...utronix.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@...utronix.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@...rosoft.com>

> 
> > Signed-off-by: Thomas Gleixner <tglx@...utronix.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);