Re: [PATCH] hw/openrisc: Fixed undercounting of TTCR in continuous mode

[Stafford Horne <shorne@gmail.com>]

On Mon, Jun 10, 2024 at 07:29:15PM +0000, Joel Holdsworth wrote:
> Hi Stafford, thanks for your response.
> 
> > - You sent this 2 times, is the only change in v2 the sender address?
> 
> Yes, I was just having some difficulty with Git and SMTP. Should be fixed now.

OK.

> >> In the existing design, TTCR is prone to undercounting when running in
> >> continuous mode. This manifests as a timer interrupt appearing to
> >> trigger a few cycles prior to the deadline set in SPR_TTMR_TP.
> 
> > This is a good find, I have noticed the timer is off when running on OpenRISC
> > but never tracked it down to this undercounting issue.  I also notice
> > unexplained RCU stalls when running in Linux when tere is no load, this timer
> issue might be related.
> 
> > Did you notice this via other system symptoms when running OpenRISC or just via
> > code auditing of QEMU?
> 
> I'm working on an OpenRISC port of Zephyr. The under-counting issue causes
> consistent deadlocks in my experiments with the test suite. I wouldn't be
> surprised if it causes problems for other OS's.

Thats cool.  I got around to testing the patch with Linux, unfortunately I didnt
see an improvement in the lockups I have been seeing during boot time.  But I am
sure this is a step in the right direction.

> > In QEMU there is a function clock_ns_to_ticks(). Could this maybe be used
> > instead to give us more standard fix?
> 
> Seems like a good idea, and I now have some nearly-complete patch that brings
> hw/openrisc/cputimer.c into closer alignment with
> target/mips/sysemu/cp0_timer.c.

Hi, I was waiting for this second version patch, v2.  Have you ever completed it?

> However, don't we run into problems with undercounting with clock_ns_to_ticks,
> because if I understand correctly it will round ticks down, not up?, which is
> the problem I was trying to avoid in the first place.

You might be right, but if that is the case maybe it's a but to raise to the
maintainers directly.  I was planning to look into this more cosely after you
sent the followup patch.

-Stafford