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

[Laurent Vivier]

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.

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.

Thanks,
Laurent