linux-kernel - Re: [PATCH v2 01/10] ARC: timer: rtc: implement read loop in "C" vs. inline asm

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 3 Nov 2016 22:52:21 +0100
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:     Noam Camus <noamca@...lanox.com>, tglx@...utronix.de,
        linux-snps-arc@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Alexey.Brodkin@...opsys.com, stable@...r.kernel.org
Subject: Re: [PATCH v2 01/10] ARC: timer: rtc: implement read loop in "C" vs.
 inline asm

On Thu, Nov 03, 2016 at 02:31:32PM -0700, Vineet Gupta wrote:
> The current code doesn't even compile ....

Give a better description in the log, especially if this patch is supposed to
go to stable@
 
> CC: stable@...r.kernel.org
> Signed-off-by: Vineet Gupta <vgupta@...opsys.com>
> ---
>  arch/arc/kernel/time.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index f927b8dc6edd..1a117b999c0c 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -152,14 +152,17 @@ static cycle_t arc_read_rtc(struct clocksource *cs)
>  		cycle_t  full;
>  	} stamp;
>  
> -
> -	__asm__ __volatile(
> -	"1:						\n"
> -	"	lr		%0, [AUX_RTC_LOW]	\n"
> -	"	lr		%1, [AUX_RTC_HIGH]	\n"
> -	"	lr		%2, [AUX_RTC_CTRL]	\n"
> -	"	bbit0.nt	%2, 31, 1b		\n"
> -	: "=r" (stamp.low), "=r" (stamp.high), "=r" (status));
> +        /*
> +         * hardware has an internal state machine which tracks readout of
> +         * low/high and updates the CTRL.status if
> +         *  - interrupt/exception taken between the two reads
> +         *  - high increments after low has been read
> +         */
> +	do {
> +		stamp.low = read_aux_reg(AUX_RTC_LOW);
> +		stamp.high = read_aux_reg(AUX_RTC_HIGH);
> +		status = read_aux_reg(AUX_RTC_CTRL);
> +	} while (!(status & _BITUL(31)));

Is the condition correct ? If I refer to your previous answer, the bit will be
set for status if the counter wrapped up. So in this case, we won't exit the
loop until we wrap up, no ?

>  	return stamp.full;
>  }
> -- 
> 2.7.4
>