qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] Generic tick reinjection control
@ 2012-01-23 19:15 Jan Kiszka
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property Jan Kiszka
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Jan Kiszka @ 2012-01-23 19:15 UTC (permalink / raw)
  To: Anthony Liguori, qemu-devel; +Cc: Marcelo Tosatti, Avi Kivity

QEMU currently supports lost tick compensation for the periodic RTC
timer. It is controlled via -rtc driftfix=slew|none. However, the next
periodic timer with compensation qualities is approaching: KVM's
in-kernel PIT.

A previous discussion [1] showed that we need to introduce per device
control. And we likely also want a global default. Both features require
a generic standardized way to specify the compensation mode.

So this series lays the ground for that by adding a qdev property type
to select from four possible lost tick compensation policies (see patch
1 one for details). And this new property is then applied on the RTC
device.

[1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85339

Jan Kiszka (2):
  qdev: Introduce lost tick policy property
  mc146818rtc: Use lost_tick_policy property

 hw/mc146818rtc.c     |   26 +++++++++++++++++------
 hw/qdev-properties.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    7 ++++++
 qemu-common.h        |    7 ++++++
 sysemu.h             |    1 -
 vl.c                 |   28 ++++++++++++++++++++----
 6 files changed, 111 insertions(+), 13 deletions(-)

-- 
1.7.3.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property
  2012-01-23 19:15 [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Jan Kiszka
@ 2012-01-23 19:15 ` Jan Kiszka
  2012-01-23 20:01   ` Daniel P. Berrange
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 2/2] mc146818rtc: Use lost_tick_policy property Jan Kiszka
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2012-01-23 19:15 UTC (permalink / raw)
  To: Anthony Liguori, qemu-devel; +Cc: Marcelo Tosatti, Avi Kivity

Potentially tick-generating timer devices will gain a common property:
lock_tick_policy. It allows to encode 4 different ways how to deal with
tick events the guest did not process in time:

discard - ignore lost ticks (e.g. if the guest compensates for them
          already)
delay   - replay all lost ticks in a row once the guest accepts them
          again
merge   - if multiple ticks are lost, all of them are merged into one
          which is replayed once the guest accepts it again
slew    - lost ticks are gradually replayed at a higher frequency than
          the original tick

Not all timer device will need to support all modes. However, all need
to accept the configuration via this common property.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/qdev-properties.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |    7 ++++++
 qemu-common.h        |    7 ++++++
 3 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index ea3b2df..97ed0ce 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -885,6 +885,55 @@ PropertyInfo qdev_prop_macaddr = {
     .set   = set_generic,
 };
 
+
+/* --- lost tick policy --- */
+
+static const struct {
+    const char *name;
+    LostTickPolicy code;
+} lost_tick_policy_table[] = {
+    { .name = "discard", .code = LOST_TICK_DISCARD },
+    { .name = "delay", .code = LOST_TICK_DELAY },
+    { .name = "merge", .code = LOST_TICK_MERGE },
+    { .name = "slew", .code = LOST_TICK_SLEW },
+};
+
+static int parse_lost_tick_policy(DeviceState *dev, Property *prop,
+                                  const char *str)
+{
+    LostTickPolicy *ptr = qdev_get_prop_ptr(dev, prop);
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(lost_tick_policy_table); i++) {
+        if (!strcasecmp(str, lost_tick_policy_table[i].name)) {
+            *ptr = lost_tick_policy_table[i].code;
+            break;
+        }
+    }
+    if (i == ARRAY_SIZE(lost_tick_policy_table)) {
+        return -EINVAL;
+    }
+    return 0;
+}
+
+static int print_lost_tick_policy(DeviceState *dev, Property *prop, char *dest,
+                                  size_t len)
+{
+    LostTickPolicy *ptr = qdev_get_prop_ptr(dev, prop);
+
+    return snprintf(dest, len, "%s", lost_tick_policy_table[*ptr].name);
+}
+
+PropertyInfo qdev_prop_losttickpolicy = {
+    .name  = "lost_tick_policy",
+    .type  = PROP_TYPE_LOSTTICKPOLICY,
+    .size  = sizeof(LostTickPolicy),
+    .parse = parse_lost_tick_policy,
+    .print = print_lost_tick_policy,
+    .get   = get_generic,
+    .set   = set_generic,
+};
+
 /* --- pci address --- */
 
 /*
@@ -1127,6 +1176,12 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
     qdev_prop_set(dev, name, value, PROP_TYPE_MACADDR);
 }
 
+void qdev_prop_set_losttickpolicy(DeviceState *dev, const char *name,
+                                  LostTickPolicy *value)
+{
+    qdev_prop_set(dev, name, value, PROP_TYPE_LOSTTICKPOLICY);
+}
+
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_PTR);
diff --git a/hw/qdev.h b/hw/qdev.h
index 6b58dd8..26b7fae 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -145,6 +145,7 @@ enum PropertyType {
     PROP_TYPE_UINT64,
     PROP_TYPE_TADDR,
     PROP_TYPE_MACADDR,
+    PROP_TYPE_LOSTTICKPOLICY,
     PROP_TYPE_DRIVE,
     PROP_TYPE_CHR,
     PROP_TYPE_STRING,
@@ -288,6 +289,7 @@ extern PropertyInfo qdev_prop_string;
 extern PropertyInfo qdev_prop_chr;
 extern PropertyInfo qdev_prop_ptr;
 extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
 extern PropertyInfo qdev_prop_drive;
 extern PropertyInfo qdev_prop_netdev;
 extern PropertyInfo qdev_prop_vlan;
@@ -348,6 +350,9 @@ extern PropertyInfo qdev_prop_pci_devfn;
     DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
 #define DEFINE_PROP_MACADDR(_n, _s, _f)         \
     DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
+#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
+                        LostTickPolicy)
 
 #define DEFINE_PROP_END_OF_LIST()               \
     {}
@@ -370,6 +375,8 @@ void qdev_prop_set_vlan(DeviceState *dev, const char *name, VLANState *value);
 int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) QEMU_WARN_UNUSED_RESULT;
 void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value);
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
+void qdev_prop_set_losttickpolicy(DeviceState *dev, const char *name,
+                                  LostTickPolicy *value);
 /* FIXME: Remove opaque pointer properties.  */
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 void qdev_prop_set_defaults(DeviceState *dev, Property *props);
diff --git a/qemu-common.h b/qemu-common.h
index 6ab7dfb..8b69a9e 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -250,6 +250,13 @@ typedef struct QEMUSGList QEMUSGList;
 
 typedef uint64_t pcibus_t;
 
+typedef enum LostTickPolicy {
+    LOST_TICK_DISCARD,
+    LOST_TICK_DELAY,
+    LOST_TICK_MERGE,
+    LOST_TICK_SLEW,
+} LostTickPolicy;
+
 void tcg_exec_init(unsigned long tb_size);
 bool tcg_enabled(void);
 
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Qemu-devel] [PATCH 2/2] mc146818rtc: Use lost_tick_policy property
  2012-01-23 19:15 [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Jan Kiszka
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property Jan Kiszka
@ 2012-01-23 19:15 ` Jan Kiszka
  2012-01-23 19:24 ` [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Anthony Liguori
  2012-02-01 22:10 ` Anthony Liguori
  3 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2012-01-23 19:15 UTC (permalink / raw)
  To: Anthony Liguori, qemu-devel; +Cc: Marcelo Tosatti, Avi Kivity

Allow to configure the MC146818 RTC via the new lost tick policy
property and replace rtc_td_hack with this mechanism.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/mc146818rtc.c |   26 +++++++++++++++++++-------
 sysemu.h         |    1 -
 vl.c             |   28 +++++++++++++++++++++++-----
 3 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 657fa10..48e0964 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -101,6 +101,7 @@ typedef struct RTCState {
     QEMUTimer *second_timer;
     QEMUTimer *second_timer2;
     Notifier clock_reset_notifier;
+    LostTickPolicy lost_tick_policy;
 } RTCState;
 
 static void rtc_set_time(RTCState *s);
@@ -183,7 +184,7 @@ static void rtc_periodic_timer(void *opaque)
     if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
         s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
 #ifdef TARGET_I386
-        if(rtc_td_hack) {
+        if (s->lost_tick_policy == LOST_TICK_SLEW) {
             if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
                 s->irq_reinject_on_ack_count = 0;		
             apic_reset_irq_delivered();
@@ -544,7 +545,7 @@ static int rtc_post_load(void *opaque, int version_id)
     RTCState *s = opaque;
 
     if (version_id >= 2) {
-        if (rtc_td_hack) {
+        if (s->lost_tick_policy == LOST_TICK_SLEW) {
             rtc_coalesced_timer_update(s);
         }
     }
@@ -589,7 +590,7 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
     qemu_mod_timer(s->second_timer2, s->next_second_time);
     rtc_timer_update(s, now);
 #ifdef TARGET_I386
-    if (rtc_td_hack) {
+    if (s->lost_tick_policy == LOST_TICK_SLEW) {
         rtc_coalesced_timer_update(s);
     }
 #endif
@@ -605,8 +606,9 @@ static void rtc_reset(void *opaque)
     qemu_irq_lower(s->irq);
 
 #ifdef TARGET_I386
-    if (rtc_td_hack)
-	    s->irq_coalesced = 0;
+    if (s->lost_tick_policy == LOST_TICK_SLEW) {
+        s->irq_coalesced = 0;
+    }
 #endif
 }
 
@@ -654,12 +656,20 @@ static int rtc_initfn(ISADevice *dev)
 
     rtc_set_date_from_host(dev);
 
-    s->periodic_timer = qemu_new_timer_ns(rtc_clock, rtc_periodic_timer, s);
 #ifdef TARGET_I386
-    if (rtc_td_hack)
+    switch (s->lost_tick_policy) {
+    case LOST_TICK_SLEW:
         s->coalesced_timer =
             qemu_new_timer_ns(rtc_clock, rtc_coalesced_timer, s);
+        break;
+    case LOST_TICK_DISCARD:
+        break;
+    default:
+        return -EINVAL;
+    }
 #endif
+
+    s->periodic_timer = qemu_new_timer_ns(rtc_clock, rtc_periodic_timer, s);
     s->second_timer = qemu_new_timer_ns(rtc_clock, rtc_update_second, s);
     s->second_timer2 = qemu_new_timer_ns(rtc_clock, rtc_update_second2, s);
 
@@ -707,6 +717,8 @@ static ISADeviceInfo mc146818rtc_info = {
     .init          = rtc_initfn,
     .qdev.props    = (Property[]) {
         DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980),
+        DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState,
+                                   lost_tick_policy, LOST_TICK_DISCARD),
         DEFINE_PROP_END_OF_LIST(),
     }
 };
diff --git a/sysemu.h b/sysemu.h
index caff268..9d5ce33 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -105,7 +105,6 @@ extern int graphic_depth;
 extern DisplayType display_type;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
-extern int rtc_td_hack;
 extern int alt_grab;
 extern int ctrl_grab;
 extern int usb_enabled;
diff --git a/vl.c b/vl.c
index 57378b6..14b1602 100644
--- a/vl.c
+++ b/vl.c
@@ -201,7 +201,6 @@ CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 int win2k_install_hack = 0;
-int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
@@ -540,9 +539,18 @@ static void configure_rtc(QemuOpts *opts)
     value = qemu_opt_get(opts, "driftfix");
     if (value) {
         if (!strcmp(value, "slew")) {
-            rtc_td_hack = 1;
+            static GlobalProperty slew_lost_ticks[] = {
+                {
+                    .driver   = "mc146818rtc",
+                    .property = "lost_tick_policy",
+                    .value    = "slew",
+                },
+                { /* end of list */ }
+            };
+
+            qdev_prop_register_global_list(slew_lost_ticks);
         } else if (!strcmp(value, "none")) {
-            rtc_td_hack = 0;
+            /* discard is default */
         } else {
             fprintf(stderr, "qemu: invalid option value '%s'\n", value);
             exit(1);
@@ -2836,9 +2844,19 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_win2k_hack:
                 win2k_install_hack = 1;
                 break;
-            case QEMU_OPTION_rtc_td_hack:
-                rtc_td_hack = 1;
+            case QEMU_OPTION_rtc_td_hack: {
+                static GlobalProperty slew_lost_ticks[] = {
+                    {
+                        .driver   = "mc146818rtc",
+                        .property = "lost_tick_policy",
+                        .value    = "slew",
+                    },
+                    { /* end of list */ }
+                };
+
+                qdev_prop_register_global_list(slew_lost_ticks);
                 break;
+            }
             case QEMU_OPTION_acpitable:
                 do_acpitable_option(optarg);
                 break;
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] Generic tick reinjection control
  2012-01-23 19:15 [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Jan Kiszka
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property Jan Kiszka
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 2/2] mc146818rtc: Use lost_tick_policy property Jan Kiszka
@ 2012-01-23 19:24 ` Anthony Liguori
  2012-02-01 22:10 ` Anthony Liguori
  3 siblings, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2012-01-23 19:24 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Dor Laor, Anthony Liguori, Marcelo Tosatti, qemu-devel,
	Avi Kivity

On 01/23/2012 01:15 PM, Jan Kiszka wrote:
> QEMU currently supports lost tick compensation for the periodic RTC
> timer. It is controlled via -rtc driftfix=slew|none. However, the next
> periodic timer with compensation qualities is approaching: KVM's
> in-kernel PIT.
>
> A previous discussion [1] showed that we need to introduce per device
> control. And we likely also want a global default. Both features require
> a generic standardized way to specify the compensation mode.
>
> So this series lays the ground for that by adding a qdev property type
> to select from four possible lost tick compensation policies (see patch
> 1 one for details). And this new property is then applied on the RTC
> device.

Looks good.  I'll take this through my tree.

Now we can just set the appropriate globals in order to enable a default rtc 
policy of slew.

Regards,

Anthony Liguori

> [1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85339
>
> Jan Kiszka (2):
>    qdev: Introduce lost tick policy property
>    mc146818rtc: Use lost_tick_policy property
>
>   hw/mc146818rtc.c     |   26 +++++++++++++++++------
>   hw/qdev-properties.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/qdev.h            |    7 ++++++
>   qemu-common.h        |    7 ++++++
>   sysemu.h             |    1 -
>   vl.c                 |   28 ++++++++++++++++++++----
>   6 files changed, 111 insertions(+), 13 deletions(-)
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property
  2012-01-23 19:15 ` [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property Jan Kiszka
@ 2012-01-23 20:01   ` Daniel P. Berrange
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrange @ 2012-01-23 20:01 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Anthony Liguori, Marcelo Tosatti, qemu-devel, Avi Kivity

On Mon, Jan 23, 2012 at 08:15:11PM +0100, Jan Kiszka wrote:
> Potentially tick-generating timer devices will gain a common property:
> lock_tick_policy. It allows to encode 4 different ways how to deal with
> tick events the guest did not process in time:
> 
> discard - ignore lost ticks (e.g. if the guest compensates for them
>           already)
> delay   - replay all lost ticks in a row once the guest accepts them
>           again
> merge   - if multiple ticks are lost, all of them are merged into one
>           which is replayed once the guest accepts it again
> slew    - lost ticks are gradually replayed at a higher frequency than
>           the original tick
> 
> Not all timer device will need to support all modes. However, all need
> to accept the configuration via this common property.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

That looks good - thank you for making the effort to come up with
something that maps so well to the libvirt model for this concept.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] Generic tick reinjection control
  2012-01-23 19:15 [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Jan Kiszka
                   ` (2 preceding siblings ...)
  2012-01-23 19:24 ` [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Anthony Liguori
@ 2012-02-01 22:10 ` Anthony Liguori
  3 siblings, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2012-02-01 22:10 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, qemu-devel, Avi Kivity

On 01/23/2012 01:15 PM, Jan Kiszka wrote:
> QEMU currently supports lost tick compensation for the periodic RTC
> timer. It is controlled via -rtc driftfix=slew|none. However, the next
> periodic timer with compensation qualities is approaching: KVM's
> in-kernel PIT.
>
> A previous discussion [1] showed that we need to introduce per device
> control. And we likely also want a global default. Both features require
> a generic standardized way to specify the compensation mode.
>
> So this series lays the ground for that by adding a qdev property type
> to select from four possible lost tick compensation policies (see patch
> 1 one for details). And this new property is then applied on the RTC
> device.
>
> [1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/85339

Applied.  Thanks.

Regards,

Anthony Liguori

>
> Jan Kiszka (2):
>    qdev: Introduce lost tick policy property
>    mc146818rtc: Use lost_tick_policy property
>
>   hw/mc146818rtc.c     |   26 +++++++++++++++++------
>   hw/qdev-properties.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/qdev.h            |    7 ++++++
>   qemu-common.h        |    7 ++++++
>   sysemu.h             |    1 -
>   vl.c                 |   28 ++++++++++++++++++++----
>   6 files changed, 111 insertions(+), 13 deletions(-)
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-01 22:14 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-23 19:15 [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Jan Kiszka
2012-01-23 19:15 ` [Qemu-devel] [PATCH 1/2] qdev: Introduce lost tick policy property Jan Kiszka
2012-01-23 20:01   ` Daniel P. Berrange
2012-01-23 19:15 ` [Qemu-devel] [PATCH 2/2] mc146818rtc: Use lost_tick_policy property Jan Kiszka
2012-01-23 19:24 ` [Qemu-devel] [PATCH 0/2] Generic tick reinjection control Anthony Liguori
2012-02-01 22:10 ` Anthony Liguori

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).