* [PATCH 1/1] goldfish_rtc: keep time offset when resetting
@ 2025-03-21 8:12 Heinrich Schuchardt
2025-03-21 16:00 ` Anup Patel
2025-03-21 16:08 ` [PATCH-for-10.0 " Philippe Mathieu-Daudé
0 siblings, 2 replies; 5+ messages in thread
From: Heinrich Schuchardt @ 2025-03-21 8:12 UTC (permalink / raw)
To: Anup Patel, Alistair Francis
Cc: Frederik Du Toit Lotter, qemu-riscv, qemu-devel,
Heinrich Schuchardt
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.
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);
+ s->tick_offset_vmstate = 0;
}
static const Property goldfish_rtc_properties[] = {
--
2.48.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] goldfish_rtc: keep time offset when resetting
2025-03-21 8:12 [PATCH 1/1] goldfish_rtc: keep time offset when resetting Heinrich Schuchardt
@ 2025-03-21 16:00 ` Anup Patel
2025-03-21 16:08 ` [PATCH-for-10.0 " Philippe Mathieu-Daudé
1 sibling, 0 replies; 5+ messages in thread
From: Anup Patel @ 2025-03-21 16:00 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Alistair Francis, Frederik Du Toit Lotter, qemu-riscv, qemu-devel
On Fri, Mar 21, 2025 at 1:43 PM Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> 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.
>
> Reported-by: Frederik Du Toit Lotter <fred.lotter@canonical.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
LGTM.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> 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);
> + s->tick_offset_vmstate = 0;
> }
>
> static const Property goldfish_rtc_properties[] = {
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-10.0 1/1] goldfish_rtc: keep time offset when resetting
2025-03-21 8:12 [PATCH 1/1] goldfish_rtc: keep time offset when resetting Heinrich Schuchardt
2025-03-21 16:00 ` Anup Patel
@ 2025-03-21 16:08 ` Philippe Mathieu-Daudé
2025-03-21 16:10 ` Heinrich Schuchardt
1 sibling, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-21 16:08 UTC (permalink / raw)
To: Heinrich Schuchardt, Anup Patel, Alistair Francis
Cc: Frederik Du Toit Lotter, qemu-riscv, qemu-devel, Laurent Vivier
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>
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-10.0 1/1] goldfish_rtc: keep time offset when resetting
2025-03-21 16:08 ` [PATCH-for-10.0 " Philippe Mathieu-Daudé
@ 2025-03-21 16:10 ` Heinrich Schuchardt
2025-03-21 17:43 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Heinrich Schuchardt @ 2025-03-21 16:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Frederik Du Toit Lotter, qemu-riscv, qemu-devel, Laurent Vivier,
Anup Patel, Alistair Francis
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?
Best regards
Heinrich
>
>> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH-for-10.0 1/1] goldfish_rtc: keep time offset when resetting
2025-03-21 16:10 ` Heinrich Schuchardt
@ 2025-03-21 17:43 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-21 17:43 UTC (permalink / raw)
To: Heinrich Schuchardt
Cc: Frederik Du Toit Lotter, qemu-riscv, qemu-devel, Laurent Vivier,
Anup Patel, Alistair Francis
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-21 17:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 8:12 [PATCH 1/1] goldfish_rtc: keep time offset when resetting Heinrich Schuchardt
2025-03-21 16:00 ` Anup Patel
2025-03-21 16:08 ` [PATCH-for-10.0 " Philippe Mathieu-Daudé
2025-03-21 16:10 ` Heinrich Schuchardt
2025-03-21 17:43 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).