linux-kernel - Re: [PATCH 1/7] ARM: vdso: Remove dependency with the arch

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: <20190408155849.GM6139@lakrids.cambridge.arm.com>
Date:   Mon, 8 Apr 2019 16:58:49 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Marc Zyngier <marc.zyngier@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Russell King <linux@....linux.org.uk>,
        Will Deacon <will.deacon@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Valentin Schneider <valentin.schneider@....com>
Subject: Re: [PATCH 1/7] ARM: vdso: Remove dependency with the arch_timer
 driver internals

On Mon, Apr 08, 2019 at 04:49:01PM +0100, Marc Zyngier wrote:
> THe VDSO code uses the kernel helper that was originally designed

Nit: s/THe/The/

> to abstract the access between 32 and 64bit systems. It worked so
> far because this function is declared as 'inline'.
> 
> As we're about to revamp that part of the code, the VDSO would
> break. Let's fix it by doing what should have been done from
> the start, a proper system register access.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@....com>
> ---
>  arch/arm/include/asm/cp15.h   | 2 ++
>  arch/arm/vdso/vgettimeofday.c | 5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/cp15.h b/arch/arm/include/asm/cp15.h
> index 07e27f212dc7..d2453e2d3f1f 100644
> --- a/arch/arm/include/asm/cp15.h
> +++ b/arch/arm/include/asm/cp15.h
> @@ -68,6 +68,8 @@
>  #define BPIALL				__ACCESS_CP15(c7, 0, c5, 6)
>  #define ICIALLU				__ACCESS_CP15(c7, 0, c5, 0)
>  
> +#define CNTVCT				__ACCESS_CP15_64(1, c14)

This encoding looks right to me per ARM DDI 0406C.d section B4.1.34. The
rest also looks sound, so with that typo fixed:

Reviewed-by: Mark Rutland <mark.rutland@....com>

Mark.

> +
>  extern unsigned long cr_alignment;	/* defined in entry-armv.S */
>  
>  static inline unsigned long get_cr(void)
> diff --git a/arch/arm/vdso/vgettimeofday.c b/arch/arm/vdso/vgettimeofday.c
> index a9dd619c6c29..7bdbf5d5c47d 100644
> --- a/arch/arm/vdso/vgettimeofday.c
> +++ b/arch/arm/vdso/vgettimeofday.c
> @@ -18,9 +18,9 @@
>  #include <linux/compiler.h>
>  #include <linux/hrtimer.h>
>  #include <linux/time.h>
> -#include <asm/arch_timer.h>
>  #include <asm/barrier.h>
>  #include <asm/bug.h>
> +#include <asm/cp15.h>
>  #include <asm/page.h>
>  #include <asm/unistd.h>
>  #include <asm/vdso_datapage.h>
> @@ -123,7 +123,8 @@ static notrace u64 get_ns(struct vdso_data *vdata)
>  	u64 cycle_now;
>  	u64 nsec;
>  
> -	cycle_now = arch_counter_get_cntvct();
> +	isb();
> +	cycle_now = read_sysreg(CNTVCT);
>  
>  	cycle_delta = (cycle_now - vdata->cs_cycle_last) & vdata->cs_mask;
>  
> -- 
> 2.20.1
>