Re: [PATCH v2 08/11] target/openrisc: Enable MTTCG

[Stafford Horne]

On Mon, Jul 04, 2022 at 03:37:04PM +0530, Richard Henderson wrote:
> On 7/4/22 02:58, Stafford Horne wrote:
> >       case TO_SPR(10, 1): /* TTCR */
> > -        cpu_openrisc_count_update(cpu);
> > +        if (cpu_openrisc_timer_has_advanced(cpu)) {
> > +            qemu_mutex_lock_iothread();
> > +            cpu_openrisc_count_update(cpu);
> > +            qemu_mutex_unlock_iothread();
> > +        }
> 
> Lock around the whole if, I think.  Otherwise looks good.

Well, actually the cpu_openrisc_timer_has_advanced read is done once outside the
lock as an optimization to avoid taking the lock when it is not needed. i.e. if
we have 4 cores that all try to update the clock at the same time in theory only
one will have to take the lock and update the shared timer.

But I do see that could be flawed as after it takes the lock the timer could
have been updated by then.  Ill move it inside and see if there is any
perfromance hit / increase in the sync-profile.

> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~