All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Aleksa Paunovic via B4 Relay"
	<devnull+aleksa.paunovic.htecgroup.com@kernel.org>
Cc: krzk+dt@kernel.org, devicetree@vger.kernel.org,
	conor+dt@kernel.org, robh@kernel.org
Subject: Re: [PATCH RESEND v7 2/3] riscv: clocksource: Add readq options to clocksource mmio
Date: Thu, 14 May 2026 06:21:54 +0000	[thread overview]
Message-ID: <20260514062155.4E206C2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <20260513-riscv-time-mmio-v7-2-f8925ffc226b@htecgroup.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `readq_relaxed()` to read a live 64-bit clocksource counter is racy and unsafe on 32-bit systems, leading to torn reads and broken timekeeping.
--

commit 21048c9043069be78a033341f4f13739837c7833
Author: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>

riscv: clocksource: Add readq options to clocksource mmio

This commit adds generic read functions for 64-bit memory-mapped
clocksources covering both up and down counters.

> diff --git a/drivers/clocksource/mmio.c b/drivers/clocksource/mmio.c
> index 9de7515318312..6329d8ce2c091 100644
> --- a/drivers/clocksource/mmio.c
> +++ b/drivers/clocksource/mmio.c
[ ... ]
> @@ -17,6 +17,20 @@ static inline struct clocksource_mmio *to_mmio_clksrc(struct clocksource *c)
>  	return container_of(c, struct clocksource_mmio, clksrc);
>  }
>  
> +#if defined(readq_relaxed)

Is it safe to use readq_relaxed() directly for a live 64-bit clocksource
counter on 32-bit systems?

readq_relaxed() is not guaranteed to be atomic on 32-bit platforms. For
example, 32-bit SuperH implements __raw_readq as *(volatile u64 *), which
results in two separate 32-bit memory accesses. Other 32-bit platforms
might define it via <linux/io-64-nonatomic-lo-hi.h>.

> +
> +u64 clocksource_mmio_readq_up(struct clocksource *c)
> +{
> +	return (u64)readq_relaxed(to_mmio_clksrc(c)->reg);

If the lower 32 bits of the live hardware counter roll over between the two
32-bit reads, will the resulting 64-bit value be severely corrupted (torn)?

To safely support 64-bit MMIO clocksources on 32-bit platforms, a software
retry loop is typically used. Otherwise, shouldn't the generic functions be
strictly guarded by #ifdef CONFIG_64BIT to prevent accidental usage on
32-bit systems, rather than just #if defined(readq_relaxed)?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-riscv-time-mmio-v7-0-f8925ffc226b@htecgroup.com?part=2

  reply	other threads:[~2026-05-14  6:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 15:29 [PATCH RESEND v7 0/3] riscv: Use GCR.U timer device as clocksource Aleksa Paunovic via B4 Relay
2026-05-13 15:29 ` Aleksa Paunovic
2026-05-13 15:29 ` Aleksa Paunovic via B4 Relay
2026-05-13 15:29 ` [PATCH RESEND v7 1/3] dt-bindings: timer: mips,p8700-gcru Aleksa Paunovic via B4 Relay
2026-05-13 15:29   ` Aleksa Paunovic
2026-05-13 15:29   ` Aleksa Paunovic via B4 Relay
2026-05-14  5:53   ` sashiko-bot
2026-05-13 15:29 ` [PATCH RESEND v7 2/3] riscv: clocksource: Add readq options to clocksource mmio Aleksa Paunovic via B4 Relay
2026-05-13 15:29   ` Aleksa Paunovic
2026-05-13 15:29   ` Aleksa Paunovic via B4 Relay
2026-05-14  6:21   ` sashiko-bot [this message]
2026-05-13 15:29 ` [PATCH RESEND v7 3/3] riscv: clocksource: Add p8700-gcru driver Aleksa Paunovic via B4 Relay
2026-05-13 15:29   ` Aleksa Paunovic
2026-05-13 15:29   ` Aleksa Paunovic via B4 Relay
2026-05-14  6:52   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260514062155.4E206C2BCB7@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=devnull+aleksa.paunovic.htecgroup.com@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.