All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Provide HVM guest RTC change notification
@ 2007-03-20 17:56 Ben Thomas
  2007-03-21 15:00 ` Christian Limpach
  0 siblings, 1 reply; 4+ messages in thread
From: Ben Thomas @ 2007-03-20 17:56 UTC (permalink / raw)
  To: xen-devel

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

This patch restores the capabilities initially provided in
changeset 10010.  When the RTC code was moved into the hypervisor
(a good move), the control plane lost the ability to read the
current time offset as well as to receive notification of changes
to the RTC time base by a guest. This patch reintroduces these.

Additionally, there is a small window at initialization time in
which the time offset may be set but not noticed or used. If
xc_domain_set_time_offset is called before the domain is started,
the offset won't be noticed until the next second update. An HVM
guest could read the RTC in this window and get an unintended
result. This patch closes the window.

The patch builds upon the existing change notification mechanism
provided by VIRQ_DOM_EXC. I stopped short of renaming the
releaseDomain watch to something like domainEvent as it wasn't
clear who might be relying upon the existing name. A patch for
that would be easy to create, though.

Last, the code introduced into qemu by 10010 is no longer really
connected to anything and could be removed. It's found in
tools/ioemu/patches/domain-timeoffset and can probably be excised
at some suitable point. Currently, it's relatively harmless.


Signed-off-by: Ben Thomas (ben@virtualiron.com)

-- 
------------------------------------------------------------------------
Ben Thomas                                         Virtual Iron Software
bthomas@virtualiron.com                            Tower 1, Floor 2
978-849-1214                                       900 Chelmsford Street
                                                    Lowell, MA 01851

[-- Attachment #2: xen-timeoffset_change.patch --]
[-- Type: text/x-patch, Size: 11139 bytes --]

Provide notification and reading of RTC change by HVM guest

If the HVM guest changes the current date/time in the RTC, signal
this to the control plan via the DOM_EXC virq. Allow the control
plane to obtain the new time via a a new call to read the time
offset for a specific domain.

Close initial condition window. If the offset is set before the
domain starts, it may not be used until after the first second
update. This leaves a small hole in which the wrong values could
be read.

Signed-off-by: Ben Thomas (ben@virtualiron.com)

diff -r 9646036c745d tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c	Mon Mar 19 15:00:30 2007 -0400
+++ b/tools/libxc/xc_domain.c	Tue Mar 20 10:55:51 2007 -0400
@@ -180,6 +180,7 @@ int xc_domain_getinfo(int xc_handle,
         info->blocked  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_blocked);
         info->running  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_running);
         info->hvm      = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hvm_guest);
+        info->time_chg = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_time_chg);
 
         info->shutdown_reason =
             (domctl.u.getdomaininfo.flags>>XEN_DOMINF_shutdownshift) &
@@ -425,6 +426,26 @@ int xc_domain_set_time_offset(int xc_han
     domctl.domain = (domid_t)domid;
     domctl.u.settimeoffset.time_offset_seconds = time_offset_seconds;
     return do_domctl(xc_handle, &domctl);
+}
+
+int xc_domain_get_time_offset(int xc_handle,
+                              uint32_t domid,
+                              int32_t *time_offset_seconds)
+{
+    int rc;
+    DECLARE_DOMCTL;
+
+    if (time_offset_seconds == NULL)
+        return -EINVAL;
+
+    domctl.cmd = XEN_DOMCTL_gettimeoffset;
+    domctl.domain = (domid_t)domid;
+
+    rc = do_domctl(xc_handle, &domctl);
+
+    *time_offset_seconds = domctl.u.gettimeoffset.time_offset_seconds;
+
+    return rc;
 }
 
 int xc_domain_memory_increase_reservation(int xc_handle,
diff -r 9646036c745d tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h	Mon Mar 19 15:00:30 2007 -0400
+++ b/tools/libxc/xenctrl.h	Mon Mar 19 15:01:11 2007 -0400
@@ -150,7 +150,7 @@ typedef struct xc_dominfo {
     uint32_t      ssidref;
     unsigned int  dying:1, crashed:1, shutdown:1,
                   paused:1, blocked:1, running:1,
-                  hvm:1;
+                  hvm:1, time_chg:1;
     unsigned int  shutdown_reason; /* only meaningful if shutdown==1 */
     unsigned long nr_pages;
     unsigned long shared_info_frame;
@@ -487,6 +487,10 @@ int xc_domain_set_time_offset(int xc_han
 int xc_domain_set_time_offset(int xc_handle,
                               uint32_t domid,
                               int32_t time_offset_seconds);
+
+int xc_domain_get_time_offset(int xc_handle,
+                              uint32_t domid,
+                              int32_t *time_offset_seconds);
 
 int xc_domain_memory_increase_reservation(int xc_handle,
                                           uint32_t domid,
diff -r 9646036c745d tools/xenstore/xenstored_domain.c
--- a/tools/xenstore/xenstored_domain.c	Mon Mar 19 15:00:30 2007 -0400
+++ b/tools/xenstore/xenstored_domain.c	Mon Mar 19 15:10:42 2007 -0400
@@ -195,6 +195,8 @@ static void domain_cleanup(void)
 				domain->shutdown = 1;
 				notify = 1;
 			}
+			if (dominfo.time_chg)
+				notify = 1;
 			if (!dominfo.dying)
 				continue;
 		}
diff -r 9646036c745d xen/arch/x86/hvm/rtc.c
--- a/xen/arch/x86/hvm/rtc.c	Mon Mar 19 15:00:30 2007 -0400
+++ b/xen/arch/x86/hvm/rtc.c	Tue Mar 20 13:16:20 2007 -0400
@@ -27,8 +27,24 @@
 #include <asm/hvm/io.h>
 #include <asm/hvm/support.h>
 #include <asm/current.h>
+#include <xen/event.h>
 
 /* #define DEBUG_RTC */
+
+/* Nonzero if YEAR is a leap year */
+#define __isleap(year) \
+  ((year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0))
+
+/* How many days are in each month.  */
+static const unsigned short int __mon_lengths[2][12] = {
+    /* Normal years.  */
+    {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31},
+    /* Leap years.  */
+    {31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}
+};
+
+static void rtc_copy_date(RTCState *s);
+static unsigned long RTC_mktime(struct tm *tbuf);
 
 void rtc_periodic_cb(struct vcpu *v, void *opaque)
 {
@@ -73,6 +89,13 @@ static int rtc_ioport_write(void *opaque
 static int rtc_ioport_write(void *opaque, uint32_t addr, uint32_t data)
 {
     RTCState *s = opaque;
+
+    /* Before the first write, make sure we're completely initialized */
+    if ( unlikely(!s->accessed) )
+    {
+        rtc_copy_date(s);
+        s->accessed = 1;
+    }
 
     if ( (addr & 1) == 0 )
     {
@@ -157,7 +180,10 @@ static void rtc_set_time(RTCState *s)
 static void rtc_set_time(RTCState *s)
 {
     struct tm *tm = &s->current_tm;
-    
+    unsigned long	new_time;
+    unsigned long	current_time;
+    int32_t		new_offset;
+
     tm->tm_sec = from_bcd(s, s->hw.cmos_data[RTC_SECONDS]);
     tm->tm_min = from_bcd(s, s->hw.cmos_data[RTC_MINUTES]);
     tm->tm_hour = from_bcd(s, s->hw.cmos_data[RTC_HOURS] & 0x7f);
@@ -168,6 +194,18 @@ static void rtc_set_time(RTCState *s)
     tm->tm_mday = from_bcd(s, s->hw.cmos_data[RTC_DAY_OF_MONTH]);
     tm->tm_mon = from_bcd(s, s->hw.cmos_data[RTC_MONTH]) - 1;
     tm->tm_year = from_bcd(s, s->hw.cmos_data[RTC_YEAR]) + 100;
+
+    current_time = get_localtime(s->pt.vcpu->domain) -
+                   s->pt.vcpu->domain->time_offset_seconds;
+    new_time = RTC_mktime(tm);
+    new_offset = new_time - current_time;
+    if (new_offset != s->time_offset_seconds) {
+      s->time_offset_seconds = new_offset;
+      s->pt.vcpu->domain->time_offset_seconds = new_offset;
+      set_bit(_DOMF_time_chg, &s->pt.vcpu->domain->domain_flags);
+      send_guest_global_virq(dom0, VIRQ_DOM_EXC);
+    }
+
 }
 
 static void rtc_copy_date(RTCState *s)
@@ -328,6 +366,13 @@ static uint32_t rtc_ioport_read(void *op
     if ( (addr & 1) == 0 )
         return 0xff;
 
+    /* Before the first read, make sure we're completely initialized */
+    if ( unlikely(!s->accessed) )
+    {
+        rtc_copy_date(s);
+        s->accessed = 1;
+    }
+
     switch ( s->hw.cmos_index )
     {
     case RTC_SECONDS:
@@ -434,7 +479,6 @@ void rtc_init(struct vcpu *v, int base)
 void rtc_init(struct vcpu *v, int base)
 {
     RTCState *s = &v->domain->arch.hvm_domain.pl_time.vrtc;
-
     s->pt.vcpu = v;
     s->hw.cmos_data[RTC_REG_A] = RTC_REF_CLCK_32KHZ | 6; /* ~1kHz */
     s->hw.cmos_data[RTC_REG_B] = RTC_24H;
@@ -462,3 +506,36 @@ void rtc_deinit(struct domain *d)
     kill_timer(&s->second_timer);
     kill_timer(&s->second_timer2);
 }
+
+/* This routine returns value like mktime, but is not anywhere
+   near as rigorous. It assumes reasonable input values from the
+   RTC structures. Some structures borrowed from gmtime in time.c */
+
+static unsigned long RTC_mktime(struct tm *tbuf)
+{
+  int i;
+  unsigned long result;
+
+  if (tbuf == NULL)
+    return -1;
+
+  result = 0;
+  for (i=70; i < tbuf->tm_year; i++)
+    result += (__isleap(i+1900) ? 366 :365);
+
+  for (i = 0; i < tbuf->tm_mon; i++)
+    result += __mon_lengths[__isleap(tbuf->tm_year+1900)][i];
+
+  result += tbuf->tm_mday - 1;
+    
+  result *= 24;
+  result += tbuf->tm_hour;
+
+  result *= 60;
+  result += tbuf->tm_min;
+
+  result *= 60;
+  result += tbuf->tm_sec;
+
+  return result;
+}
diff -r 9646036c745d xen/common/domctl.c
--- a/xen/common/domctl.c	Mon Mar 19 15:00:30 2007 -0400
+++ b/xen/common/domctl.c	Tue Mar 20 10:56:19 2007 -0400
@@ -117,6 +117,7 @@ void getdomaininfo(struct domain *d, str
         ((d->domain_flags & DOMF_dying)      ? XEN_DOMINF_dying    : 0) |
         ((d->domain_flags & DOMF_shutdown)   ? XEN_DOMINF_shutdown : 0) |
         ((d->domain_flags & DOMF_ctrl_pause) ? XEN_DOMINF_paused   : 0) |
+        ((d->domain_flags & DOMF_time_chg)   ? XEN_DOMINF_time_chg : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
     if ( is_hvm_domain(d) )
@@ -711,6 +712,25 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
     }
     break;
 
+    case XEN_DOMCTL_gettimeoffset:
+    {
+        struct domain *d;
+
+        ret = -ESRCH;
+        d = rcu_lock_domain_by_id(op->domain);
+        if ( d != NULL )
+        {
+            op->u.gettimeoffset.time_offset_seconds = d->time_offset_seconds;
+            ret = 0;
+            if ( copy_to_guest(u_domctl, op, 1) )
+                ret = -EFAULT;
+
+            clear_bit(_DOMF_time_chg, &d->domain_flags);
+            rcu_unlock_domain(d);
+        }
+    }
+    break;
+
     default:
         ret = arch_do_domctl(op, u_domctl);
         break;
diff -r 9646036c745d xen/include/asm-x86/hvm/vpt.h
--- a/xen/include/asm-x86/hvm/vpt.h	Mon Mar 19 15:00:30 2007 -0400
+++ b/xen/include/asm-x86/hvm/vpt.h	Tue Mar 20 11:18:22 2007 -0400
@@ -93,6 +93,7 @@ typedef struct RTCState {
     struct timer second_timer2;
     struct periodic_time pt;
     int32_t time_offset_seconds;
+    unsigned int accessed:1;
 } RTCState;
 
 #define FREQUENCE_PMTIMER  3579545  /* Timer should run at 3.579545 MHz */
diff -r 9646036c745d xen/include/public/domctl.h
--- a/xen/include/public/domctl.h	Mon Mar 19 15:00:30 2007 -0400
+++ b/xen/include/public/domctl.h	Mon Mar 19 15:01:11 2007 -0400
@@ -85,6 +85,10 @@ struct xen_domctl_getdomaininfo {
  /* Domain is currently running.            */
 #define _XEN_DOMINF_running   5
 #define XEN_DOMINF_running    (1U<<_XEN_DOMINF_running)
+/* Domain has a time offset change         */
+#define _XEN_DOMINF_time_chg  6
+#define XEN_DOMINF_time_chg   (1U<<_XEN_DOMINF_time_chg)
+
  /* CPU to which this domain is bound.      */
 #define XEN_DOMINF_cpumask      255
 #define XEN_DOMINF_cpushift       8
@@ -388,6 +392,13 @@ struct xen_domctl_settimeoffset {
 };
 typedef struct xen_domctl_settimeoffset xen_domctl_settimeoffset_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_settimeoffset_t);
+
+#define XEN_DOMCTL_gettimeoffset     37
+struct xen_domctl_gettimeoffset {
+    int32_t  time_offset_seconds; /* applied to domain wallclock time */
+};
+typedef struct xen_domctl_gettimeoffset xen_domctl_gettimeoffset_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_gettimeoffset_t);
 
  
 #define XEN_DOMCTL_gethvmcontext     33
@@ -453,6 +464,7 @@ struct xen_domctl {
         struct xen_domctl_hypercall_init    hypercall_init;
         struct xen_domctl_arch_setup        arch_setup;
         struct xen_domctl_settimeoffset     settimeoffset;
+        struct xen_domctl_gettimeoffset     gettimeoffset;
         struct xen_domctl_real_mode_area    real_mode_area;
         struct xen_domctl_hvmcontext        hvmcontext;
         struct xen_domctl_address_size      address_size;
diff -r 9646036c745d xen/include/xen/sched.h
--- a/xen/include/xen/sched.h	Mon Mar 19 15:00:30 2007 -0400
+++ b/xen/include/xen/sched.h	Mon Mar 19 15:01:11 2007 -0400
@@ -474,6 +474,9 @@ extern struct domain *domain_list;
  /* Domain is a compatibility one? */
 #define _DOMF_compat           6
 #define DOMF_compat            (1UL<<_DOMF_compat)
+ /* Domain time offset has changed */
+#define _DOMF_time_chg         7
+#define DOMF_time_chg          (1UL<<_DOMF_time_chg)
 
 static inline int vcpu_runnable(struct vcpu *v)
 {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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

* Re: [PATCH] Provide HVM guest RTC change notification
  2007-03-20 17:56 [PATCH] Provide HVM guest RTC change notification Ben Thomas
@ 2007-03-21 15:00 ` Christian Limpach
  2007-03-21 15:55   ` Ben Thomas
  2007-03-26 14:37   ` Ben Thomas
  0 siblings, 2 replies; 4+ messages in thread
From: Christian Limpach @ 2007-03-21 15:00 UTC (permalink / raw)
  To: Ben Thomas; +Cc: xen-devel

On 3/20/07, Ben Thomas <bthomas@virtualiron.com> wrote:
> This patch restores the capabilities initially provided in
> changeset 10010.  When the RTC code was moved into the hypervisor
> (a good move), the control plane lost the ability to read the
> current time offset as well as to receive notification of changes
> to the RTC time base by a guest. This patch reintroduces these.
>
> Additionally, there is a small window at initialization time in
> which the time offset may be set but not noticed or used. If
> xc_domain_set_time_offset is called before the domain is started,
> the offset won't be noticed until the next second update. An HVM
> guest could read the RTC in this window and get an unintended
> result. This patch closes the window.

Isn't the "s->time_offset_seconds !=
s->pt.vcpu->domain->time_offset_seconds" condition an appropriate test
whether you need to call rtc_copy_date?

> The patch builds upon the existing change notification mechanism
> provided by VIRQ_DOM_EXC. I stopped short of renaming the
> releaseDomain watch to something like domainEvent as it wasn't
> clear who might be relying upon the existing name. A patch for
> that would be easy to create, though.

The domain exception virq is already quite overloaded as is and it
causes xend to do a quite expensive scan of all the domain
information.

I think using an ioreq to qemu would be a more appropriate mechanism
to signal that the guest has changed the timeoffset.  The ioreq would
include the delta by which the offset was changed.  qemu could then
update the timeoffset stored in xenstore, since we would want to make
the changed timeoffset persist across reboots.

    christian

> Last, the code introduced into qemu by 10010 is no longer really
> connected to anything and could be removed. It's found in
> tools/ioemu/patches/domain-timeoffset and can probably be excised
> at some suitable point. Currently, it's relatively harmless.
>
>
> Signed-off-by: Ben Thomas (ben@virtualiron.com)
>
> --
> ------------------------------------------------------------------------
> Ben Thomas                                         Virtual Iron Software
> bthomas@virtualiron.com                            Tower 1, Floor 2
> 978-849-1214                                       900 Chelmsford Street
>                                                    Lowell, MA 01851
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>

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

* Re: [PATCH] Provide HVM guest RTC change notification
  2007-03-21 15:00 ` Christian Limpach
@ 2007-03-21 15:55   ` Ben Thomas
  2007-03-26 14:37   ` Ben Thomas
  1 sibling, 0 replies; 4+ messages in thread
From: Ben Thomas @ 2007-03-21 15:55 UTC (permalink / raw)
  To: Christian.Limpach; +Cc: xen-devel

Christian Limpach wrote:
> On 3/20/07, Ben Thomas <bthomas@virtualiron.com> wrote:
>> This patch restores the capabilities initially provided in
>> changeset 10010.  When the RTC code was moved into the hypervisor
>> (a good move), the control plane lost the ability to read the
>> current time offset as well as to receive notification of changes
>> to the RTC time base by a guest. This patch reintroduces these.
>>
>> Additionally, there is a small window at initialization time in
>> which the time offset may be set but not noticed or used. If
>> xc_domain_set_time_offset is called before the domain is started,
>> the offset won't be noticed until the next second update. An HVM
>> guest could read the RTC in this window and get an unintended
>> result. This patch closes the window.
> 
> Isn't the "s->time_offset_seconds !=
> s->pt.vcpu->domain->time_offset_seconds" condition an appropriate test
> whether you need to call rtc_copy_date?

It depends upon where you want this test. I didn't want it for
every access to the RTC registers, and something is needed to
set up the initial conditions.  A different approach would have
handled it when the RTC is set up, but that appears to be at
domain building time and it wasn't clear to me that there was a way
to pass the information without changing the domain builder API.
Given an existing mechanism to set the RTC time, it seemed an
interesting approach to use that and simply handle the initial
condition.

Changing the register access routines to unconditionally check for
a time offset change seemed too intrusive and at conflict
with the per-second change mechanism.  But, as long as it all works,
I'm open to anything.

The unlikely check for the accessed flag seemed a simple, low-cost
way to handle the initialization without unduly influencing other
portions of the mechanism.

>> The patch builds upon the existing change notification mechanism
>> provided by VIRQ_DOM_EXC. I stopped short of renaming the
>> releaseDomain watch to something like domainEvent as it wasn't
>> clear who might be relying upon the existing name. A patch for
>> that would be easy to create, though.
> 
> The domain exception virq is already quite overloaded as is and it
> causes xend to do a quite expensive scan of all the domain
> information.
> 
> I think using an ioreq to qemu would be a more appropriate mechanism
> to signal that the guest has changed the timeoffset.  The ioreq would
> include the delta by which the offset was changed.  qemu could then
> update the timeoffset stored in xenstore, since we would want to make
> the changed timeoffset persist across reboots.
 >    christian

I completely agree about the persistence, and that's the reason I
sent in the patch that resulted in c/s 10010.  I'm just trying to get
back to that level of capability.

The domain exception seemed like a reasonable approach. It may
be overloaded in the "number of things it can signal", but it
seems like a low-occurrence set of events.  And, the code checking
for that type of event easily accesses the additional flag.  Last,
but not least, using this particular virq completely removes all
qemu involvement.  Cutting back on qemu changes doesn't seem like
a bad side-effect.

Thanks,
-b



>> Last, the code introduced into qemu by 10010 is no longer really
>> connected to anything and could be removed. It's found in
>> tools/ioemu/patches/domain-timeoffset and can probably be excised
>> at some suitable point. Currently, it's relatively harmless.
>>
>>
>> Signed-off-by: Ben Thomas (ben@virtualiron.com)


-- 
------------------------------------------------------------------------
Ben Thomas                                         Virtual Iron Software
bthomas@virtualiron.com                            Tower 1, Floor 2
978-849-1214                                       900 Chelmsford Street
                                                    Lowell, MA 01851

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

* Re: [PATCH] Provide HVM guest RTC change notification
  2007-03-21 15:00 ` Christian Limpach
  2007-03-21 15:55   ` Ben Thomas
@ 2007-03-26 14:37   ` Ben Thomas
  1 sibling, 0 replies; 4+ messages in thread
From: Ben Thomas @ 2007-03-26 14:37 UTC (permalink / raw)
  To: Christian.Limpach; +Cc: xen-devel

Hi,

What's the status of this discussion/patch ? It would be nice to
get some closure on this. I know everyone's busy.

My understanding of the current concerns from Christian are (and
these are my interpretation of his comments):

- simply use a inequality test in time offsets to trigger setting
   the new time offset.

   My rationale for the as-submitted patch, was that there is an
   existing per-second mechanism that tries to emulate the set and
   uip bits and timing.  Time changes appear to happen during this
   per-second mechanism.  While it looks simple to just modify the
   time if the offset is detected to have changed, it would appear
   that this would negatively impact readers of the RTC and
   circumvent the work done by the per-second mechanism.  More
   simply, it wasn't clear to me that modifying the time wouldn't
   break guests unless the per-second mechanism was utilized.

- don't use the VIRQ mechanism for time-offset (RTC write)
   notifications.  Use an ioreq to qemu.

   The as-submitted patch used the VIRQ mechanism for a few
   reasons.  By building upon the domain event mechanism/virq,
   this low frequency event is handled in a fashion quite
   like other low-frequency domain events such as shutdown and
   crash. I was looking for a low-cost, low-impact, fire-and-forget
   means to signal an RTC change.

   The ioreq approach is interesting.  I'm uncertain about it for
   the following reasons, which some small additional explanation
   would help me with.  Adding an ioreq for time change seems quite
   out of character with the other ioreq events.  Additionally,
   qemu processing (and writing a store entry) are likely to be
   rather expensive.  Wouldn't this also cause undue delay in the
   guest awaiting the completion of the ioreq ?

It is quite possible that I have misunderstood Christian's comments.
If so, I'd love to better understand the issues. I really don't care
what the final implementation is, other than the usual expectations
of robustness, reasonable performance and clarity. Perhaps with a
bit more information, this can be cleared up and closed up.  I'd like
to get back to the capabilities that existed before the RTC code
was moved into the hypervisor.

Thanks !
-b




Christian Limpach wrote:
> On 3/20/07, Ben Thomas <bthomas@virtualiron.com> wrote:
>> This patch restores the capabilities initially provided in
>> changeset 10010.  When the RTC code was moved into the hypervisor
>> (a good move), the control plane lost the ability to read the
>> current time offset as well as to receive notification of changes
>> to the RTC time base by a guest. This patch reintroduces these.
>>
>> Additionally, there is a small window at initialization time in
>> which the time offset may be set but not noticed or used. If
>> xc_domain_set_time_offset is called before the domain is started,
>> the offset won't be noticed until the next second update. An HVM
>> guest could read the RTC in this window and get an unintended
>> result. This patch closes the window.
> 
> Isn't the "s->time_offset_seconds !=
> s->pt.vcpu->domain->time_offset_seconds" condition an appropriate test
> whether you need to call rtc_copy_date?
> 
>> The patch builds upon the existing change notification mechanism
>> provided by VIRQ_DOM_EXC. I stopped short of renaming the
>> releaseDomain watch to something like domainEvent as it wasn't
>> clear who might be relying upon the existing name. A patch for
>> that would be easy to create, though.
> 
> The domain exception virq is already quite overloaded as is and it
> causes xend to do a quite expensive scan of all the domain
> information.
> 
> I think using an ioreq to qemu would be a more appropriate mechanism
> to signal that the guest has changed the timeoffset.  The ioreq would
> include the delta by which the offset was changed.  qemu could then
> update the timeoffset stored in xenstore, since we would want to make
> the changed timeoffset persist across reboots.
> 
>    christian
> 
>> Last, the code introduced into qemu by 10010 is no longer really
>> connected to anything and could be removed. It's found in
>> tools/ioemu/patches/domain-timeoffset and can probably be excised
>> at some suitable point. Currently, it's relatively harmless.
>>
>>
>> Signed-off-by: Ben Thomas (ben@virtualiron.com)


-- 
------------------------------------------------------------------------
Ben Thomas                                         Virtual Iron Software
bthomas@virtualiron.com                            Tower 1, Floor 2
978-849-1214                                       900 Chelmsford Street
                                                    Lowell, MA 01851

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

end of thread, other threads:[~2007-03-26 14:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-20 17:56 [PATCH] Provide HVM guest RTC change notification Ben Thomas
2007-03-21 15:00 ` Christian Limpach
2007-03-21 15:55   ` Ben Thomas
2007-03-26 14:37   ` Ben Thomas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.