Re: [PATCH 2/2] MIPS: cevt-r4k: Offset counter value for clearing compare interrupt

[Thomas Bogendoerfer <tsbogend@alpha.franken.de>]

On Mon, Feb 27, 2023 at 01:22:47AM +0000, Jiaxun Yang wrote:
> 
> 
> > 2023年2月26日 23:23，Thomas Bogendoerfer <tsbogend@alpha.franken.de> 写道：
> > 
> > On Sat, Feb 25, 2023 at 10:10:08PM +0000, Jiaxun Yang wrote:
> >> In c0_compare_int_usable we clear compare interrupt by write value
> >> just read out from counter to compare register.
> >> 
> >> However sometimes if those all instructions are graduated together
> >> then it's possible that at the time compare register is written, the
> >> counter haven't progressed, thus the interrupt is triggered again.
> >> 
> >> It also applies to QEMU that instructions is execuated significantly
> >> faster then counter.
> >> 
> >> Offset the counter value a litlle bit to prevent that happen.
> >> 
> >> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> >> ---
> >> arch/mips/kernel/cevt-r4k.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
> >> index 32ec67c9ab67..bbc422376e97 100644
> >> --- a/arch/mips/kernel/cevt-r4k.c
> >> +++ b/arch/mips/kernel/cevt-r4k.c
> >> @@ -200,6 +200,8 @@ int c0_compare_int_usable(void)
> >> */
> >> if (c0_compare_int_pending()) {
> >> cnt = read_c0_count();
> >> + // Drawdown a little bit in case counter haven't progressed
> > 
> > no C++ comments
> > 
> >> + cnt -= COMPARE_INT_SEEN_TICKS;
> >> write_c0_compare(cnt);
> >> back_to_back_c0_hazard();
> >> while (read_c0_count() < (cnt  + COMPARE_INT_SEEN_TICKS))
> > 
> > this doesn't make sense. clearing of the interrupts happes because of
> > this loop. So either COMPARE_INT_SEEN_TICKS is too small or you are
> > hunting a different bug.
> 
> Clearing interrupt happens in write_c0_compare but the problem is the interrupt
> does really get cleared because it triggered again straight away.

the function you are patching is a test function whether counter/compare
is usable, so it's not in the normal timer handling. See a problem there
would more to broken hardware than to a bug in that function.

> Had confirmed issue on MIPS I6500 uarch simulation trace.

so not seen on real hardware ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]