linux-kernel - Re: [PATCH v2 0/3] Fix arm_arch_timer clockmode when vDSO disabled

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] [day] [month] [year] [list]

Date:   Mon, 24 Feb 2020 10:12:52 +0000
From:   Vincenzo Frascino <vincenzo.frascino@....com>
To:     Thomas Gleixner <tglx@...utronix.de>, Marc Zyngier <maz@...nel.org>
Cc:     linux-arch@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, catalin.marinas@....com,
        will.deacon@....com, linux@...linux.org.uk, luto@...nel.org,
        m.szyprowski@...sung.com, Mark.Rutland@....com
Subject: Re: [PATCH v2 0/3] Fix arm_arch_timer clockmode when vDSO disabled

On 2/24/20 9:12 AM, Thomas Gleixner wrote:
> Marc Zyngier <maz@...nel.org> writes:
>> On Fri, 21 Feb 2020 18:18:46 +0000
>> Vincenzo Frascino <vincenzo.frascino@....com> wrote:
>>>
>>> This patch series addresses the issue defining a default arch clockmode
>>> for arm and arm64 and using it to initialize the arm_arch_timer.
>>
>> arm only. arm64 is just fine.
> 
> Right. ARM64 unconditionaly enables VDSO
> 
>>
>> This doesn't apply to -rc2, and is rather against next.
> 
> More precise it's against tip timers/core which has the VDSO changes
> which caused this fallout.
>

Agree, I will fix it in the next iteration.

>>> Vincenzo Frascino (3):
>>>   arm: clocksource: Add VDSO default clockmode
>>>   arm64: clocksource: Add VDSO default clockmode
>>>   clocksource: Fix arm_arch_timer clockmode when vDSO disabled
>>
>> Please squash the three patches into a single one. There is zero point
>> in having 3 patches for something that small.
> 
> I really don't see why we need all this redefine foo. What's wrong with
> the obvious?
> 
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -69,7 +69,12 @@ static enum arch_timer_ppi_nr arch_timer
>  static bool arch_timer_c3stop;
>  static bool arch_timer_mem_use_virtual;
>  static bool arch_counter_suspend_stop;
> +
> +#ifdef CONFIG_GENERIC_GETTIMEOFDAY
>  static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_ARCHTIMER;
> +#else
> +static enum vdso_clock_mode vdso_default = VDSO_CLOCKMODE_NONE;
> +#endif
>  
>  static cpumask_t evtstrm_available = CPU_MASK_NONE;
>  static bool evtstrm_enable = IS_ENABLED(CONFIG_ARM_ARCH_TIMER_EVTSTREAM);
> 

Nothing, I agree :) I think we over engineered here a bit.

-- 
Regards,
Vincenzo