Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[David Laight <david.laight.linux@gmail.com>]

On Tue, 16 Jun 2026 23:47:05 +0800
Guo Ren <guoren@kernel.org> wrote:

> On Tue, Jun 16, 2026 at 6:36 PM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > On Mon, 15 Jun 2026 12:38:17 +0000
> > Guo Ren <guoren@kernel.org> wrote:
> >  
> > > Hi Zhanpeng Zhang,  
> > ..  
> > > 3. Only performance-monitoring counters require 64-bit IO access or the
> > > high-low-high do-while retry strategy. For ordinary status and control
> > > MMIO registers, a single read is sufficient.  
> >
> > Actually this sequence should be enough for a counter:
> >         hi = read_hi();
> >         lo = read_lo();
> >         if (hi != read_hi()) {
> >                 // Pick a value that happened while doing the reads.
> >                 hi++;
> >                 lo = 0;
> >         }  
> This is not a free optimization — it does not preserve the same
> semantics as an atomic 64-bit read. There are at least two correctness
> issues:
> 
> 1. Loss of precision: If hi changed during the read, setting lo = 0
> discards the actual lo value. The correct approach is to re-read lo
> after detecting the change, not fabricate a value.
> 
> 2. Multiple overflows: This assumes at most one overflow occurs
> between reads. If two overflows happen (e.g., due to interrupt
> injection), hi++ will produce an incorrect result, silently corrupting
> the counter value.
> 
> Negligible benefit: hi changing between reads is an extremely rare
> event. Optimizing away the retry loop for such a rare case provides no
> meaningful performance gain. And if hi never stabilizes, that
> indicates a hardware failure — in which case hanging in the retry loop
> is actually the more appropriate behavior, as it makes the failure
> visible rather than silently producing garbage values.
> 
> The high-low-high do-while retry strategy exists precisely to handle
> these cases correctly. I don't think this proposed sequence is a valid
> replacement.
> 

The point is that if the value is an incrementing counter (of some form)
then the value is stale by the time the reading sequence completes.
So all the code can be assumed to do is return a value the counter had
sometime between when the code started and when it finished.
If hi changes then hi+1:0 must have happened while the code was running
so it is a safe return value.

It is likely that the reads are also much slower than memory reads.
If hi changes relatively infrequently (compared to the number of reads)
it may be worth saving the previously read value and avoiding the second
read of hi if it is the same as the previous read hi and lo has got larger.

	David


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv