Re: [PATCH-for-10.0 1/1] goldfish_rtc: keep time offset when resetting

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

On 21/3/25 17:10, Heinrich Schuchardt wrote:
> On 21.03.25 17:08, Philippe Mathieu-Daudé wrote:
>> Hi Heinrich,
>>
>> On 21/3/25 09:12, Heinrich Schuchardt wrote:
>>> Currently resetting leads to resynchronizing the Goldfish RTC with the
>>> system clock of the host. In real hardware an RTC reset would not change
>>> the wall time. Other RTCs like pl031 do not show this behavior.
>>>
>>> Move the synchronization of the RTC with the system clock to the 
>>> instance
>>> realization.
>>>
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 9a5b40b8427 ("hw: rtc: Add Goldfish RTC device")
>>
>>> Reported-by: Frederik Du Toit Lotter <fred.lotter@canonical.com>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>>> ---
>>>   hw/rtc/goldfish_rtc.c | 14 +++++++-------
>>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/rtc/goldfish_rtc.c b/hw/rtc/goldfish_rtc.c
>>> index 0f1b53e0e4..203a343511 100644
>>> --- a/hw/rtc/goldfish_rtc.c
>>> +++ b/hw/rtc/goldfish_rtc.c
>>> @@ -239,15 +239,8 @@ static const VMStateDescription 
>>> goldfish_rtc_vmstate = {
>>>   static void goldfish_rtc_reset(DeviceState *dev)
>>>   {
>>>       GoldfishRTCState *s = GOLDFISH_RTC(dev);
>>> -    struct tm tm;
>>>       timer_del(s->timer);
>>> -
>>> -    qemu_get_timedate(&tm, 0);
>>> -    s->tick_offset = mktimegm(&tm);
>>> -    s->tick_offset *= NANOSECONDS_PER_SECOND;
>>> -    s->tick_offset -= qemu_clock_get_ns(rtc_clock);
>>> -    s->tick_offset_vmstate = 0;
>>>       s->alarm_next = 0;
>>>       s->alarm_running = 0;
>>>       s->irq_pending = 0;
>>> @@ -258,6 +251,7 @@ static void goldfish_rtc_realize(DeviceState *d, 
>>> Error **errp)
>>>   {
>>>       SysBusDevice *dev = SYS_BUS_DEVICE(d);
>>>       GoldfishRTCState *s = GOLDFISH_RTC(d);
>>> +    struct tm tm;
>>>       memory_region_init_io(&s->iomem, OBJECT(s),
>>>                             &goldfish_rtc_ops[s->big_endian], s,
>>> @@ -267,6 +261,12 @@ static void goldfish_rtc_realize(DeviceState *d, 
>>> Error **errp)
>>>       sysbus_init_irq(dev, &s->irq);
>>>       s->timer = timer_new_ns(rtc_clock, goldfish_rtc_interrupt, s);
>>> +
>>> +    qemu_get_timedate(&tm, 0);
>>> +    s->tick_offset = mktimegm(&tm);
>>> +    s->tick_offset *= NANOSECONDS_PER_SECOND;
>>> +    s->tick_offset -= qemu_clock_get_ns(rtc_clock);
>>
>> OK
>>
>>> +    s->tick_offset_vmstate = 0;
>>
>> This last line is pointless. Otherwise:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Thanks for reviewing. Is the DeviceState structure fill with 0x00 when 
> allocated?

The QOM hierarchy of this device is:

GOLDFISH_RTC -> SYS_BUS_DEVICE -> DEVICE -> OBJECT

When objects are created, object_new() ends up calling
object_initialize_with_type() which calls memset(0).

Objects initialized "in place" via object_initialize_child()
also end calling object_initialize() -> object_initialize_with_type()
then memset(0).

So yes, QOM-based objects have their state zero-initialized.

Regards,

Phil.