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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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 <randrianasulu@gmail.com>
>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>    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é <philmd@redhat.com>
>>
>>> +    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 :)