Thomas Gleixner <tglx@xxxxxxxxxxxxx> writes: > On Wed, May 12 2021 at 10:46, Vitaly Kuznetsov wrote: >> Mohammed reports (https://bugzilla.kernel.org/show_bug.cgi?id=213029) >> the commit e4ab4658f1cf ("clocksource/drivers/hyper-v: Handle vDSO >> differences inline") broke vDSO on x86. The problem appears to be that >> VDSO_CLOCKMODE_HVCLOCK is an enum value in 'enum vdso_clock_mode' and >> '#ifdef VDSO_CLOCKMODE_HVCLOCK' branch evaluates to false (it is not >> a define). Replace it with CONFIG_X86 as it is the only arch which >> has this mode currently. >> >> Reported-by: Mohammed Gamal <mgamal@xxxxxxxxxx> >> Fixes: e4ab4658f1cf ("clocksource/drivers/hyper-v: Handle vDSO differences inline") >> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> >> --- >> drivers/clocksource/hyperv_timer.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c >> index 977fd05ac35f..e17421f5e47d 100644 >> --- a/drivers/clocksource/hyperv_timer.c >> +++ b/drivers/clocksource/hyperv_timer.c >> @@ -419,7 +419,7 @@ static void resume_hv_clock_tsc(struct clocksource *arg) >> hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr); >> } >> >> -#ifdef VDSO_CLOCKMODE_HVCLOCK >> +#ifdef CONFIG_X86 >> static int hv_cs_enable(struct clocksource *cs) >> { >> vclocks_set_used(VDSO_CLOCKMODE_HVCLOCK); >> @@ -435,7 +435,7 @@ static struct clocksource hyperv_cs_tsc = { >> .flags = CLOCK_SOURCE_IS_CONTINUOUS, >> .suspend= suspend_hv_clock_tsc, >> .resume = resume_hv_clock_tsc, >> -#ifdef VDSO_CLOCKMODE_HVCLOCK >> +#ifdef CONFIG_X86 >> .enable = hv_cs_enable, >> .vdso_clock_mode = VDSO_CLOCKMODE_HVCLOCK, >> #else > > That's lame as it needs to be patched differently once ARM64 gains > support. What about the below? > The solution I liked the most was Michael's: no need to do anything except for adding VDSO_CLOCKMODE_HVCLOCK to the enum. Too bad it didn't work) You proposal seems to be slightly better than mine: when adding VDSO_CLOCKMODE_HVCLOCK to ARM the change will be limited to drivers/clocksource/hyperv_timer.c will stay intact. I'll send v2 shortly, thanks! > Thanks, > > tglx > --- > --- a/arch/x86/include/asm/vdso/clocksource.h > +++ b/arch/x86/include/asm/vdso/clocksource.h > @@ -7,4 +7,6 @@ > VDSO_CLOCKMODE_PVCLOCK, \ > VDSO_CLOCKMODE_HVCLOCK > > +#define HAVE_VDSO_CLOCKMODE_HVCLOCK > + > #endif /* __ASM_VDSO_CLOCKSOURCE_H */ > --- a/drivers/clocksource/hyperv_timer.c > +++ b/drivers/clocksource/hyperv_timer.c > @@ -419,7 +419,7 @@ static void resume_hv_clock_tsc(struct c > hv_set_register(HV_REGISTER_REFERENCE_TSC, tsc_msr); > } > > -#ifdef VDSO_CLOCKMODE_HVCLOCK > +#ifdef HAVE_VDSO_CLOCKMODE_HVCLOCK > static int hv_cs_enable(struct clocksource *cs) > { > vclocks_set_used(VDSO_CLOCKMODE_HVCLOCK); > @@ -435,7 +435,7 @@ static struct clocksource hyperv_cs_tsc > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > .suspend= suspend_hv_clock_tsc, > .resume = resume_hv_clock_tsc, > -#ifdef VDSO_CLOCKMODE_HVCLOCK > +#ifdef HAVE_VDSO_CLOCKMODE_HVCLOCK > .enable = hv_cs_enable, > .vdso_clock_mode = VDSO_CLOCKMODE_HVCLOCK, > #else > -- Vitaly
