* [Qemu-devel] [PATCHv2] Add rtc reset function.
@ 2009-06-09 9:26 Gleb Natapov
2009-06-09 10:23 ` [Qemu-devel] " Jan Kiszka
2009-06-09 15:45 ` [Qemu-devel] " Marcelo Tosatti
0 siblings, 2 replies; 6+ messages in thread
From: Gleb Natapov @ 2009-06-09 9:26 UTC (permalink / raw)
To: qemu-devel
On reset:
Periodic Interrupt Enable (PIE) bit is cleared to zero
Alarm Interrupt Enable (AIE) bit is cleared to zero
Update ended Interrupt Flag (UF) bit is cleared to zero
Interrupt Request status Flag (IRQF) bit is cleared to zero
Periodic Interrupt Flag (PF) bit is cleared to zero
Alarm Interrupt Flag (AF) bit is cleared to zero
Square Wave output Enable (SQWE) zero
Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 888b85a..8c446e1 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -57,6 +57,11 @@
#define REG_B_SQWE 0x08
#define REG_B_DM 0x04
+#define REG_C_UF 0x10
+#define REG_C_IRQF 0x80
+#define REG_C_PF 0x40
+#define REG_C_AF 0x20
+
struct RTCState {
uint8_t cmos_data[128];
uint8_t cmos_index;
@@ -568,6 +573,21 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
}
#endif
+static void rtc_reset(void *opaque)
+{
+ RTCState *s = opaque;
+
+ s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
+ s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
+
+ qemu_irq_lower(s->irq);
+
+#ifdef TARGET_I386
+ if (rtc_td_hack)
+ s->irq_coalesced = 0;
+#endif
+}
+
RTCState *rtc_init_sqw(int base, qemu_irq irq, qemu_irq sqw_irq, int base_year)
{
RTCState *s;
@@ -606,6 +626,8 @@ RTCState *rtc_init_sqw(int base, qemu_irq irq, qemu_irq sqw_irq, int base_year)
if (rtc_td_hack)
register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
#endif
+ qemu_register_reset(rtc_reset, 0, s);
+
return s;
}
@@ -721,5 +743,6 @@ RTCState *rtc_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq,
if (rtc_td_hack)
register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
#endif
+ qemu_register_reset(rtc_reset, 0, s);
return s;
}
--
Gleb.
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PATCHv2] Add rtc reset function.
2009-06-09 9:26 [Qemu-devel] [PATCHv2] Add rtc reset function Gleb Natapov
@ 2009-06-09 10:23 ` Jan Kiszka
2009-06-09 10:39 ` Gleb Natapov
2009-06-09 15:45 ` [Qemu-devel] " Marcelo Tosatti
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-06-09 10:23 UTC (permalink / raw)
To: Gleb Natapov; +Cc: qemu-devel
Gleb Natapov wrote:
> On reset:
> Periodic Interrupt Enable (PIE) bit is cleared to zero
> Alarm Interrupt Enable (AIE) bit is cleared to zero
> Update ended Interrupt Flag (UF) bit is cleared to zero
> Interrupt Request status Flag (IRQF) bit is cleared to zero
> Periodic Interrupt Flag (PF) bit is cleared to zero
> Alarm Interrupt Flag (AF) bit is cleared to zero
> Square Wave output Enable (SQWE) zero
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 888b85a..8c446e1 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -57,6 +57,11 @@
> #define REG_B_SQWE 0x08
> #define REG_B_DM 0x04
>
> +#define REG_C_UF 0x10
> +#define REG_C_IRQF 0x80
> +#define REG_C_PF 0x40
> +#define REG_C_AF 0x20
> +
> struct RTCState {
> uint8_t cmos_data[128];
> uint8_t cmos_index;
> @@ -568,6 +573,21 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
> }
> #endif
>
> +static void rtc_reset(void *opaque)
> +{
> + RTCState *s = opaque;
> +
> + s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
> + s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
> +
> + qemu_irq_lower(s->irq);
> +
> +#ifdef TARGET_I386
> + if (rtc_td_hack)
> + s->irq_coalesced = 0;
Sorry, missed that on first run: Isn't it safer, clearer, more beautiful
to reset this unconditionally?
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PATCHv2] Add rtc reset function.
2009-06-09 10:23 ` [Qemu-devel] " Jan Kiszka
@ 2009-06-09 10:39 ` Gleb Natapov
2009-06-09 11:43 ` Dor Laor
0 siblings, 1 reply; 6+ messages in thread
From: Gleb Natapov @ 2009-06-09 10:39 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel
On Tue, Jun 09, 2009 at 12:23:21PM +0200, Jan Kiszka wrote:
> > +static void rtc_reset(void *opaque)
> > +{
> > + RTCState *s = opaque;
> > +
> > + s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
> > + s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
> > +
> > + qemu_irq_lower(s->irq);
> > +
> > +#ifdef TARGET_I386
> > + if (rtc_td_hack)
> > + s->irq_coalesced = 0;
>
> Sorry, missed that on first run: Isn't it safer, clearer, more beautiful
> to reset this unconditionally?
>
It doesn't really matter. From safety point of view the code is safe.
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] Re: [PATCHv2] Add rtc reset function.
2009-06-09 10:39 ` Gleb Natapov
@ 2009-06-09 11:43 ` Dor Laor
0 siblings, 0 replies; 6+ messages in thread
From: Dor Laor @ 2009-06-09 11:43 UTC (permalink / raw)
To: Gleb Natapov; +Cc: Jan Kiszka, Anthony Liguori, qemu-devel
Gleb Natapov wrote:
> On Tue, Jun 09, 2009 at 12:23:21PM +0200, Jan Kiszka wrote:
>
>>> +static void rtc_reset(void *opaque)
>>> +{
>>> + RTCState *s = opaque;
>>> +
>>> + s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
>>> + s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
>>> +
>>> + qemu_irq_lower(s->irq);
>>> +
>>> +#ifdef TARGET_I386
>>> + if (rtc_td_hack)
>>> + s->irq_coalesced = 0;
>>>
>> Sorry, missed that on first run: Isn't it safer, clearer, more beautiful
>> to reset this unconditionally?
>>
>>
> It doesn't really matter. From safety point of view the code is safe.
>
>
Actually we should drop this rtc-td-hack flag entirely.
The time drift fix is now only compiled for x86 so it will be harmless
for other arch.
Users are affected by not calling it although it is a must for every OS
that uses rtc clock source
and cannot deal with drifts (aka all windows, and some of the Linux guests).
Anthony, will you accept such a patch to remove rtc-td-hack or at least
change it's default?
> --
> Gleb.
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCHv2] Add rtc reset function.
2009-06-09 9:26 [Qemu-devel] [PATCHv2] Add rtc reset function Gleb Natapov
2009-06-09 10:23 ` [Qemu-devel] " Jan Kiszka
@ 2009-06-09 15:45 ` Marcelo Tosatti
2009-06-09 15:48 ` Gleb Natapov
1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Tosatti @ 2009-06-09 15:45 UTC (permalink / raw)
To: Gleb Natapov; +Cc: qemu-devel
On Tue, Jun 09, 2009 at 12:26:05PM +0300, Gleb Natapov wrote:
> On reset:
> Periodic Interrupt Enable (PIE) bit is cleared to zero
> Alarm Interrupt Enable (AIE) bit is cleared to zero
> Update ended Interrupt Flag (UF) bit is cleared to zero
> Interrupt Request status Flag (IRQF) bit is cleared to zero
> Periodic Interrupt Flag (PF) bit is cleared to zero
> Alarm Interrupt Flag (AF) bit is cleared to zero
> Square Wave output Enable (SQWE) zero
>
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 888b85a..8c446e1 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -57,6 +57,11 @@
> #define REG_B_SQWE 0x08
> #define REG_B_DM 0x04
>
> +#define REG_C_UF 0x10
> +#define REG_C_IRQF 0x80
> +#define REG_C_PF 0x40
> +#define REG_C_AF 0x20
> +
> struct RTCState {
> uint8_t cmos_data[128];
> uint8_t cmos_index;
> @@ -568,6 +573,21 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
> }
> #endif
>
> +static void rtc_reset(void *opaque)
> +{
> + RTCState *s = opaque;
> +
> + s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
> + s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
> +
> + qemu_irq_lower(s->irq);
> +
> +#ifdef TARGET_I386
> + if (rtc_td_hack)
> + s->irq_coalesced = 0;
> +#endif
> +}
Don't you need to cancel the timer before clearing cmos_data[]?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCHv2] Add rtc reset function.
2009-06-09 15:45 ` [Qemu-devel] " Marcelo Tosatti
@ 2009-06-09 15:48 ` Gleb Natapov
0 siblings, 0 replies; 6+ messages in thread
From: Gleb Natapov @ 2009-06-09 15:48 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: qemu-devel
On Tue, Jun 09, 2009 at 12:45:33PM -0300, Marcelo Tosatti wrote:
> On Tue, Jun 09, 2009 at 12:26:05PM +0300, Gleb Natapov wrote:
> > On reset:
> > Periodic Interrupt Enable (PIE) bit is cleared to zero
> > Alarm Interrupt Enable (AIE) bit is cleared to zero
> > Update ended Interrupt Flag (UF) bit is cleared to zero
> > Interrupt Request status Flag (IRQF) bit is cleared to zero
> > Periodic Interrupt Flag (PF) bit is cleared to zero
> > Alarm Interrupt Flag (AF) bit is cleared to zero
> > Square Wave output Enable (SQWE) zero
> >
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> > index 888b85a..8c446e1 100644
> > --- a/hw/mc146818rtc.c
> > +++ b/hw/mc146818rtc.c
> > @@ -57,6 +57,11 @@
> > #define REG_B_SQWE 0x08
> > #define REG_B_DM 0x04
> >
> > +#define REG_C_UF 0x10
> > +#define REG_C_IRQF 0x80
> > +#define REG_C_PF 0x40
> > +#define REG_C_AF 0x20
> > +
> > struct RTCState {
> > uint8_t cmos_data[128];
> > uint8_t cmos_index;
> > @@ -568,6 +573,21 @@ static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
> > }
> > #endif
> >
> > +static void rtc_reset(void *opaque)
> > +{
> > + RTCState *s = opaque;
> > +
> > + s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
> > + s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
> > +
> > + qemu_irq_lower(s->irq);
> > +
> > +#ifdef TARGET_I386
> > + if (rtc_td_hack)
> > + s->irq_coalesced = 0;
> > +#endif
> > +}
>
> Don't you need to cancel the timer before clearing cmos_data[]?
I believe when this callback runs guest is already stopped, so there is
not difference.
--
Gleb.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-06-09 16:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-09 9:26 [Qemu-devel] [PATCHv2] Add rtc reset function Gleb Natapov
2009-06-09 10:23 ` [Qemu-devel] " Jan Kiszka
2009-06-09 10:39 ` Gleb Natapov
2009-06-09 11:43 ` Dor Laor
2009-06-09 15:45 ` [Qemu-devel] " Marcelo Tosatti
2009-06-09 15:48 ` Gleb Natapov
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).