Re: [PATCH] mos6522: update counters when timer interrupts are off

[Philippe Mathieu-Daudé]

On 11/25/19 3:56 PM, Laurent Vivier wrote:
> Le 25/11/2019 à 15:37, Philippe Mathieu-Daudé a écrit :
> > On 11/25/19 3:14 PM, Laurent Vivier wrote:
> > > Even if the interrupts are off, counters must be updated because
> > > they are running anyway and kernel can try to read them
> > > (it's the case with g3beige kernel).
> > >
> > > Reported-by: Andrew Randrianasulu <address@hidden>
> > > Signed-off-by: Laurent Vivier <address@hidden>
> > > ---
> > >    hw/misc/mos6522.c | 8 ++++++--
> > >    1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> > > index aa3bfe1afd..cecf0be59e 100644
> > > --- a/hw/misc/mos6522.c
> > > +++ b/hw/misc/mos6522.c
> > > @@ -113,6 +113,10 @@ static int64_t get_next_irq_time(MOS6522State *s,
> > > MOS6522Timer *ti,
> > >        int64_t d, next_time;
> > >        unsigned int counter;
> > >    
> >
> > Can you add a comment here such "Clock disabled. This is the longest
> > time before expiration" or better?
> >
> > Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> >
> > > +    if (ti->frequency == 0) {
> > > +        return INT64_MAX;
> > > +    }
> > > +
>
>
> In fact this is here for a deeper problem:
>
> frequency is not correctly initialized on reset.
>
> ti->frequency are initialized by cuda/pmu/mac_via after the parent reset
> (mos6522) but the parent reset calls set_counter() that uses
> ti->frequency to set the counters. The mos6522 reset initialize the
> ti->frequency from s->frequency but s->frequency is never initialized.

Ah, I see.

"How machines behave after a soft reset" is something I'd like to get 
tested more thoroughly (with Avocado eventually).

> It was hidden before because the timers were not updated if the
> interrupts are disabled, and now they are always updated.
>
> I didn't want to add a such complicated comment in the code and I will
> try to fix the problem later.

Good :)