On Sun, Jul 31, 2022 at 01:28:18PM -0500, Scott Cheloha wrote: > Apparently mips64, i.e. octeon and loongson, has the same problem as > powerpc/macppc and powerpc64. The timer interrupt is normally only > logically masked, not physically masked in the hardware, when we're > running at or above IPL_CLOCK. If we arrive at cp0_int5() when the > clock interrupt is logically masked we postpone all work until the > next tick. This is a problem for my WIP clock interrupt work.
I think the use of logical masking has been a design choice, not something dictated by the hardware. Physical masking should be possible, but some extra care would be needed to implement it, as the mips64 interrupt code is a bit clunky. > So, this patch is basically the same as what I did for macppc and what > I have proposed for powerpc64. > > - Add a new member, ci_timer_deferred, to mips64's cpu_info struct. > > While here, remove ci_pendingticks. We don't need it anymore. > > - If we get to cp0_int5() and our IPL is too high, set > cpu_info.ci_timer_deferred and return. > > - If we get to cp0_int5() and our IPL is low enough, clear > cpu_info.ci_timer_deferred and do clock interrupt work. > > - In splx(9), if the new IPL is low enough and cpu_info.ci_timer_deferred > is set, trigger the clock interrupt. > > The only big difference is that mips64 uses an equality comparison > when deciding whether to arm the timer interrupt, so it's really easy > to "miss" CP0.count when you're setting CP0.compare. > > To address this I've written a function, cp0_raise_int5(), that spins > until it is sure the timer interrupt will go off. The function needed > a bit of new code for reading CP0.cause, which I've added to > cp0access.S. I am using an initial offset of 16 cycles based on > experimentation with the machine I have access to, a 500Mhz CN50xx. > Values lower than 16 require more than one loop to arm the timer. If > that value is insufficient for other machines we can try passing the > initial offset as an argument to the function. It should not be necessary to make the initial offset variable. The offset is primarily a function of the length and content of the instruction sequence. Some unpredictability comes from cache misses and maybe branch prediction failures. > I wasn't sure where to put the prototype for cp0_raise_int5() so I > stuck it in mips64/cpu.h. If there's a better place for it, just say > so. Currently, mips64 clock.c is formulated as a proper driver. I think callers should not invoke its functions directly but use a hook instead. The MI mips64 code starts the clock through the md_startclock function pointer. Maybe there could be md_triggerclock. To reduce risk of confusion, I would rename cp0_raise_int5 to cp0_trigger_int5, as `raise' overlaps with the spl API. Also, ci_clock_deferred instead of ci_timer_deferred would look more consistent with the surrounding code.
