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

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

On Thu, 18 Jun 2026 00:24:12 +0800
Guo Ren <guoren@kernel.org> wrote:

> On Wed, Jun 17, 2026 at 3:51 AM David Laight
> <david.laight.linux@gmail.com> wrote:
> >
> > 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.  
> You're right — (hi + 1):0 is enough for hi-lo-hi. Thank you for
> sticking to your point; it helped me think more carefully and
> understand the subtlety.

Don't worry, I'll have written the loop in the past.
Probably best is read old_hi-lo-hi then if (hi != old_hi) lo = 0

	David

> 
> >
> > 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.  
> 
> 
> --
> Best Regards
>  Guo Ren