Re: [PATCH 03/15] hw/timer/arm_timer: Add missing sp804_unrealize() handler

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 8/6/23 16:41, Peter Maydell wrote:
> On Wed, 31 May 2023 at 21:36, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Release the IRQs allocated in sp804_realize() in the
>> corresponding sp804_unrealize() handler.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/timer/arm_timer.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)


>> +static void sp804_unrealize(DeviceState *dev)
>> +{
>> +    SP804State *s = SP804(dev);
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(s->timer); i++) {
>> +        qemu_free_irq(s->timer[i]->irq);
>> +    }
>> +}
> 
> I don't really see the purpose in this. It doesn't actually
> avoid a leak if we ever destroy an SP804, because
> s->timer[i] itself is memory allocated by arm_timer_init()
> which we don't clean up (the arm_timer_state not being
> a qdev). If we did convert arm_timer_state to qdev
> then these interrupts should turn into being sysbus irqs
> and gpio inputs on the relevant devices. (In fact if you
> were willing to take the migration-compat hit you
> could just have the sp804 create a 2 input OR gate and
> wire the arm_timer irqs up to it and use the output
> of the OR gate as the sp804 outbound interrupt line.)

Thank for the suggestion, I didn't notice the OR.