[PATCH] new hvm platform vhpet enable parameter

[PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

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

HVM domains currently always have a virtual hpet (high-precision event timer)
enabled.  This patch adds an hvm platform variable "vhpet" to enable/disable
virtualization of the hpet for a guest.

Default is off (no vhpet) because vhpet is currently less accurate in keeping
time than PIT (because no timer_mode adjustments), and because AFAICT VMware
still doesn't implement virtual hpet.

A similar patch to enable/disable pmtimer is also needed but I thought
I'd post this one for discussion first.

This patch applies against and was tested with xen-3.1-testing.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

===================================
If Xen could save time in a bottle / then clocks wouldn't virtually skew /
It would save every tick / for VMs that aren't quick /
and Xen then would send them anew
(with apologies to the late great Jim Croce)

[-- Attachment #2: vhpet.patch --]
[-- Type: application/octet-stream, Size: 4645 bytes --]

diff -r 6741bbc08b25 tools/python/xen/xend/XendConfig.py
--- a/tools/python/xen/xend/XendConfig.py	Mon Feb 04 14:32:26 2008 +0000
+++ b/tools/python/xen/xend/XendConfig.py	Thu Feb 07 09:27:13 2008 -0700
@@ -124,7 +124,7 @@ XENAPI_PLATFORM_CFG = [ 'acpi', 'apic', 
 XENAPI_PLATFORM_CFG = [ 'acpi', 'apic', 'boot', 'device_model', 'display', 
                         'fda', 'fdb', 'keymap', 'isa', 'localtime', 'monitor', 
                         'nographic', 'pae', 'rtc_timeoffset', 'serial', 'sdl',
-                        'soundhw','stdvga', 'usb', 'usbdevice', 'vnc',
+                        'soundhw','stdvga', 'usb', 'usbdevice', 'vhpet', 'vnc',
                         'vncconsole', 'vncdisplay', 'vnclisten', 'timer_mode',
                         'vncpasswd', 'vncunused', 'xauthority']
 
diff -r 6741bbc08b25 tools/python/xen/xend/XendConstants.py
--- a/tools/python/xen/xend/XendConstants.py	Mon Feb 04 14:32:26 2008 +0000
+++ b/tools/python/xen/xend/XendConstants.py	Thu Feb 07 09:27:13 2008 -0700
@@ -44,6 +44,7 @@ HVM_PARAM_IOREQ_PFN    = 5
 HVM_PARAM_IOREQ_PFN    = 5
 HVM_PARAM_BUFIOREQ_PFN = 6
 HVM_PARAM_TIMER_MODE   = 10
+HVM_PARAM_VHPET        = 11
 
 restart_modes = [
     "restart",
diff -r 6741bbc08b25 tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Feb 04 14:32:26 2008 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py	Thu Feb 07 09:27:13 2008 -0700
@@ -1547,6 +1547,12 @@ class XendDomainInfo:
         if hvm and timer_mode is not None:
             xc.hvm_set_param(self.domid, HVM_PARAM_TIMER_MODE,
                              long(timer_mode))
+
+        # Optionally enable virtual HPET
+        vhpet = self.info["platform"].get("vhpet")
+        if hvm and vhpet is not None:
+            xc.hvm_set_param(self.domid, HVM_PARAM_VHPET,
+                             long(vhpet))
 
         # Set maximum number of vcpus in domain
         xc.domain_max_vcpus(self.domid, int(self.info['VCPUs_max']))
diff -r 6741bbc08b25 tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py	Mon Feb 04 14:32:26 2008 +0000
+++ b/tools/python/xen/xm/create.py	Thu Feb 07 09:27:13 2008 -0700
@@ -193,6 +193,10 @@ gopts.var('pae', val='PAE',
 gopts.var('pae', val='PAE',
           fn=set_int, default=1,
           use="Disable or enable PAE of HVM domain.")
+
+gopts.var('vhpet', val='VHPET',
+          fn=set_int, default=0,
+          use="Enable virtual high-precision event timer.")
 
 gopts.var('timer_mode', val='TIMER_MODE',
           fn=set_int, default=0,
@@ -731,7 +735,7 @@ def configure_hvm(config_image, vals):
              'localtime', 'serial', 'stdvga', 'isa', 'nographic', 'soundhw',
              'vnc', 'vncdisplay', 'vncunused', 'vncconsole', 'vnclisten',
              'sdl', 'display', 'xauthority', 'rtc_timeoffset', 'monitor',
-             'acpi', 'apic', 'usb', 'usbdevice', 'keymap' ]
+             'vhpet', 'acpi', 'apic', 'usb', 'usbdevice', 'keymap' ]
     for a in args:
         if a in vals.__dict__ and vals.__dict__[a] is not None:
             config_image.append([a, vals.__dict__[a]])
diff -r 6741bbc08b25 tools/python/xen/xm/xenapi_create.py
--- a/tools/python/xen/xm/xenapi_create.py	Mon Feb 04 14:32:26 2008 +0000
+++ b/tools/python/xen/xm/xenapi_create.py	Thu Feb 07 09:27:13 2008 -0700
@@ -755,7 +755,7 @@ class sxp2xml:
 
 
     def extract_platform(self, image, document):
-        platform_keys = ['acpi', 'apic', 'pae', 'timer_mode']
+        platform_keys = ['acpi', 'apic', 'pae', 'timer_mode', 'vhpet']
 
         def extract_platform_key(key):
             platform = document.createElement("platform")
diff -r 6741bbc08b25 xen/arch/x86/hvm/hpet.c
--- a/xen/arch/x86/hvm/hpet.c	Mon Feb 04 14:32:26 2008 +0000
+++ b/xen/arch/x86/hvm/hpet.c	Thu Feb 07 09:27:13 2008 -0700
@@ -328,6 +328,8 @@ static void hpet_write(
 
 static int hpet_range(struct vcpu *v, unsigned long addr)
 {
+    if (!(v->domain->arch.hvm_domain.params[HVM_PARAM_VHPET]))
+        return 0;
     return ((addr >= HPET_BASE_ADDRESS) &&
             (addr < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE)));
 }
diff -r 6741bbc08b25 xen/include/public/hvm/params.h
--- a/xen/include/public/hvm/params.h	Mon Feb 04 14:32:26 2008 +0000
+++ b/xen/include/public/hvm/params.h	Thu Feb 07 09:27:13 2008 -0700
@@ -79,6 +79,12 @@
 #define HVMPTM_no_missed_ticks_pending   2
 #define HVMPTM_one_missed_tick_pending   3
 
-#define HVM_NR_PARAMS          11
+/*
+ * Enable virtual HPET (high-precision event timer) (x86-only)
+ *  0=disabled, 1-enabled
+ */
+#define HVM_PARAM_VHPET 11
+
+#define HVM_NR_PARAMS          12
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

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

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

Re: [PATCH] new hvm platform vhpet enable parameter

[Samuel Thibault]

Dan Magenheimer, le Thu 07 Feb 2008 10:46:33 -0700, a écrit :
> HVM domains currently always have a virtual hpet (high-precision event timer)
> enabled.  This patch adds an hvm platform variable "vhpet" to enable/disable
> virtualization of the hpet for a guest.

Mmm, why a v in vhpet? There is no v in the acpi and apic options for
instance.

> This patch applies against and was tested with xen-3.1-testing.

Mmm, that's a bit old, maybe you should check it against 3.2

Samuel

Re: [PATCH] new hvm platform vhpet enable parameter

[Keir Fraser]

HPET is also advertised in the ACPI tables which would need to be gated.
Should be called vhpet not hpet as the 'v' is rather redundant.

Why would we want to do the same for pmtimer? Is it inaccurate?

 -- Keir

On 7/2/08 17:46, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> HVM domains currently always have a virtual hpet (high-precision event timer)
> enabled.  This patch adds an hvm platform variable "vhpet" to enable/disable
> virtualization of the hpet for a guest.
> 
> Default is off (no vhpet) because vhpet is currently less accurate in keeping
> time than PIT (because no timer_mode adjustments), and because AFAICT VMware
> still doesn't implement virtual hpet.
> 
> A similar patch to enable/disable pmtimer is also needed but I thought
> I'd post this one for discussion first.
> 
> This patch applies against and was tested with xen-3.1-testing.
> 
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> ===================================
> If Xen could save time in a bottle / then clocks wouldn't virtually skew /
> It would save every tick / for VMs that aren't quick /
> and Xen then would send them anew
> (with apologies to the late great Jim Croce)
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

> HPET is also advertised in the ACPI tables which would need 
> to be gated.

Could you point me in the right direction?  I see that a Linux
hvm kernel still prints a line indicating it has discovered
an HPET in the ACPI tables, but I couldn't find anything in
hvm code that would turn that off, and I just want to turn off
HPET per guest, not for Xen and all guests.

> Should be called vhpet not hpet as the 'v' is rather redundant.

OK, will fix

> Why would we want to do the same for pmtimer? Is it inaccurate?

If Linux were using it as a timekeeping source, I think it would
be (per earlier post by Dave Winchell), but on second look once
HPET is disabled, Linux discovers pmtimer but still uses PIT/TSC
based timekeeping.  So an equivalent pmtimer patch might be
needed for some future version of Linux but I guess we won't worry
about that now.

> From: Samuel Thibault [mailto:samuel.thibault@eu.citrix.com]
> > This patch applies against and was tested with xen-3.1-testing.
> 
> Mmm, that's a bit old, maybe you should check it against 3.2

Will do.  3.1 is my daily test/dev environment so I started there.

Dan

Re: [PATCH] new hvm platform vhpet enable parameter

[Keir Fraser]

On 7/2/08 18:29, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

>> HPET is also advertised in the ACPI tables which would need
>> to be gated.
> 
> Could you point me in the right direction?  I see that a Linux
> hvm kernel still prints a line indicating it has discovered
> an HPET in the ACPI tables, but I couldn't find anything in
> hvm code that would turn that off, and I just want to turn off
> HPET per guest, not for Xen and all guests.

Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to do this will
be to gate it on a flag set up in memory by hvmloader (we already do this
e.g., for com1 and com2 -- see construct_bios_info_table() in build.c in the
same directory). That might be a bit tricky as it probably needs a bit of
ASL hacking, which has a little learning curve. I can take a look maybe next
week.

 -- Keir

RE: [PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

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

> Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to 
> do this will
> be to gate it on a flag set up in memory by hvmloader (we 
> already do this
> e.g., for com1 and com2 -- see construct_bios_info_table() in 
> build.c in the
> same directory). That might be a bit tricky as it probably 
> needs a bit of
> ASL hacking, which has a little learning curve. I can take a 
> look maybe next
> week.

OK, here's the updated patch:
1) hpet instead of vhpet
2) against 3.2-testing tip

This will work without the acpi changes so could be checked in
independently.  Though it may be a bit misleading for the
guest to print out that it found an hpet in acpi and then
be unable to use it, the acpi part is largely cosmetic
and (as you point out) a bit tricky so better left for
your capable hands.

Thanks,
Dan

[-- Attachment #2: hpet.patch --]
[-- Type: application/octet-stream, Size: 4630 bytes --]

diff -r d26b1b777fde tools/python/xen/xend/XendConfig.py
--- a/tools/python/xen/xend/XendConfig.py	Wed Feb 06 10:06:35 2008 +0000
+++ b/tools/python/xen/xend/XendConfig.py	Thu Feb 07 13:32:26 2008 -0700
@@ -127,7 +127,7 @@ XENAPI_PLATFORM_CFG = [ 'acpi', 'apic', 
 XENAPI_PLATFORM_CFG = [ 'acpi', 'apic', 'boot', 'device_model', 'display', 
                         'fda', 'fdb', 'keymap', 'isa', 'localtime', 'monitor', 
                         'nographic', 'pae', 'rtc_timeoffset', 'serial', 'sdl',
-                        'soundhw','stdvga', 'usb', 'usbdevice', 'vnc',
+                        'soundhw','stdvga', 'usb', 'usbdevice', 'hpet', 'vnc',
                         'vncconsole', 'vncdisplay', 'vnclisten', 'timer_mode',
                         'vncpasswd', 'vncunused', 'xauthority', 'pci', 'vhpt',
                         'guest_os_type' ]
diff -r d26b1b777fde tools/python/xen/xend/XendConstants.py
--- a/tools/python/xen/xend/XendConstants.py	Wed Feb 06 10:06:35 2008 +0000
+++ b/tools/python/xen/xend/XendConstants.py	Thu Feb 07 13:32:26 2008 -0700
@@ -47,6 +47,7 @@ HVM_PARAM_VHPT_SIZE    = 8
 HVM_PARAM_VHPT_SIZE    = 8
 HVM_PARAM_BUFPIOREQ_PFN = 9
 HVM_PARAM_TIMER_MODE   = 10
+HVM_PARAM_HPET         = 11
 
 restart_modes = [
     "restart",
diff -r d26b1b777fde tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Wed Feb 06 10:06:35 2008 +0000
+++ b/tools/python/xen/xend/XendDomainInfo.py	Thu Feb 07 13:32:26 2008 -0700
@@ -1659,6 +1659,12 @@ class XendDomainInfo:
         if hvm and timer_mode is not None:
             xc.hvm_set_param(self.domid, HVM_PARAM_TIMER_MODE,
                              long(timer_mode))
+
+        # Optionally enable virtual HPET
+        hpet = self.info["platform"].get("hpet")
+        if hvm and hpet is not None:
+            xc.hvm_set_param(self.domid, HVM_PARAM_HPET,
+                             long(hpet))
 
         # Set maximum number of vcpus in domain
         xc.domain_max_vcpus(self.domid, int(self.info['VCPUs_max']))
diff -r d26b1b777fde tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py	Wed Feb 06 10:06:35 2008 +0000
+++ b/tools/python/xen/xm/create.py	Thu Feb 07 13:32:26 2008 -0700
@@ -193,6 +193,10 @@ gopts.var('pae', val='PAE',
 gopts.var('pae', val='PAE',
           fn=set_int, default=1,
           use="Disable or enable PAE of HVM domain.")
+
+gopts.var('hpet', val='HPET',
+          fn=set_int, default=0,
+          use="Enable virtual high-precision event timer.")
 
 gopts.var('timer_mode', val='TIMER_MODE',
           fn=set_int, default=0,
@@ -723,7 +727,7 @@ def configure_hvm(config_image, vals):
              'localtime', 'serial', 'stdvga', 'isa', 'nographic', 'soundhw',
              'vnc', 'vncdisplay', 'vncunused', 'vncconsole', 'vnclisten',
              'sdl', 'display', 'xauthority', 'rtc_timeoffset', 'monitor',
-             'acpi', 'apic', 'usb', 'usbdevice', 'keymap', 'pci',
+             'acpi', 'apic', 'usb', 'usbdevice', 'keymap', 'pci', 'hpet',
              'guest_os_type']
 
     for a in args:
diff -r d26b1b777fde tools/python/xen/xm/xenapi_create.py
--- a/tools/python/xen/xm/xenapi_create.py	Wed Feb 06 10:06:35 2008 +0000
+++ b/tools/python/xen/xm/xenapi_create.py	Thu Feb 07 13:32:26 2008 -0700
@@ -818,7 +818,7 @@ class sxp2xml:
 
 
     def extract_platform(self, image, document):
-        platform_keys = ['acpi', 'apic', 'pae', 'vhpt', 'timer_mode']
+        platform_keys = ['acpi', 'apic', 'pae', 'vhpt', 'timer_mode', 'hpet']
 
         def extract_platform_key(key):
             platform = document.createElement("platform")
diff -r d26b1b777fde xen/arch/x86/hvm/hpet.c
--- a/xen/arch/x86/hvm/hpet.c	Wed Feb 06 10:06:35 2008 +0000
+++ b/xen/arch/x86/hvm/hpet.c	Thu Feb 07 13:32:26 2008 -0700
@@ -353,6 +353,8 @@ static void hpet_write(
 
 static int hpet_range(struct vcpu *v, unsigned long addr)
 {
+    if (!(v->domain->arch.hvm_domain.params[HVM_PARAM_HPET]))
+        return 0;
     return ((addr >= HPET_BASE_ADDRESS) &&
             (addr < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE)));
 }
diff -r d26b1b777fde xen/include/public/hvm/params.h
--- a/xen/include/public/hvm/params.h	Wed Feb 06 10:06:35 2008 +0000
+++ b/xen/include/public/hvm/params.h	Thu Feb 07 13:32:26 2008 -0700
@@ -81,6 +81,12 @@
 #define HVMPTM_no_missed_ticks_pending   2
 #define HVMPTM_one_missed_tick_pending   3
 
-#define HVM_NR_PARAMS          11
+/*
+ * Enable virtual HPET (high-precision event timer) (x86-only)
+ *  0=disabled, 1-enabled
+ */
+#define HVM_PARAM_HPET 11
+
+#define HVM_NR_PARAMS          12
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

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

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

RE: [PATCH] new hvm platform vhpet enable parameter

[Stephen C. Tweedie]

Hi,

On Thu, 2008-02-07 at 11:29 -0700, Dan Magenheimer wrote:

> If Linux were using it as a timekeeping source, I think it would
> be (per earlier post by Dave Winchell), but on second look once
> HPET is disabled, Linux discovers pmtimer but still uses PIT/TSC
> based timekeeping.

Just one word of warning --- the Linux handling of PIT vs HPET vs
pmtimer has changed a lot over the years, and you might well find older
kernels handling these cases differently.  (I can't recall exact details
but I do remember seeing long discussions about the merits of the
different timers in different cases.)

--Stephen

RE: [PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

I see this patch hasn't been taken yet.  Is there something
else I need to do or are you not in agreement that the
acpi part is cosmetic?

Thanks,
Dan

> -----Original Message-----
> From: Dan Magenheimer [mailto:dan.magenheimer@oracle.com]
> Sent: Thursday, February 07, 2008 1:54 PM
> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> Subject: RE: [Xen-devel] [PATCH] new hvm platform vhpet 
> enable parameter
> 
> 
> > Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to 
> > do this will
> > be to gate it on a flag set up in memory by hvmloader (we 
> > already do this
> > e.g., for com1 and com2 -- see construct_bios_info_table() in 
> > build.c in the
> > same directory). That might be a bit tricky as it probably 
> > needs a bit of
> > ASL hacking, which has a little learning curve. I can take a 
> > look maybe next
> > week.
> 
> OK, here's the updated patch:
> 1) hpet instead of vhpet
> 2) against 3.2-testing tip
> 
> This will work without the acpi changes so could be checked in
> independently.  Though it may be a bit misleading for the
> guest to print out that it found an hpet in acpi and then
> be unable to use it, the acpi part is largely cosmetic
> and (as you point out) a bit tricky so better left for
> your capable hands.
> 
> Thanks,
> Dan
>

Re: [PATCH] new hvm platform vhpet enable parameter

[Keir Fraser]

It has been taken. Xen-unstable:17017.

 -- Keir


On 14/2/08 16:52, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> I see this patch hasn't been taken yet.  Is there something
> else I need to do or are you not in agreement that the
> acpi part is cosmetic?
> 
> Thanks,
> Dan
> 
>> -----Original Message-----
>> From: Dan Magenheimer [mailto:dan.magenheimer@oracle.com]
>> Sent: Thursday, February 07, 2008 1:54 PM
>> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
>> Subject: RE: [Xen-devel] [PATCH] new hvm platform vhpet
>> enable parameter
>> 
>> 
>>> Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to
>>> do this will
>>> be to gate it on a flag set up in memory by hvmloader (we
>>> already do this
>>> e.g., for com1 and com2 -- see construct_bios_info_table() in
>>> build.c in the
>>> same directory). That might be a bit tricky as it probably
>>> needs a bit of
>>> ASL hacking, which has a little learning curve. I can take a
>>> look maybe next
>>> week.
>> 
>> OK, here's the updated patch:
>> 1) hpet instead of vhpet
>> 2) against 3.2-testing tip
>> 
>> This will work without the acpi changes so could be checked in
>> independently.  Though it may be a bit misleading for the
>> guest to print out that it found an hpet in acpi and then
>> be unable to use it, the acpi part is largely cosmetic
>> and (as you point out) a bit tricky so better left for
>> your capable hands.
>> 
>> Thanks,
>> Dan
>> 
> 

RE: [PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

Thanks, sorry I missed it.

Glad I didn't have to deal with that ACPI asl stuff!  What a mess!

A question:  You added a snippet in arch/x86/hvm/hvm.c that
sets the HPET_ENABLED parameter on.  I don't have a machine running
xen-unstable at the moment so I can't verify, but this appears
to be turning the virtual hpet on by default.  Was this intended?


> -----Original Message-----
> From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
> Sent: Thursday, February 14, 2008 10:52 AM
> To: dan.magenheimer@oracle.com; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] new hvm platform vhpet 
> enable parameter
> 
> 
> It has been taken. Xen-unstable:17017.
> 
>  -- Keir
> 
> 
> On 14/2/08 16:52, "Dan Magenheimer" 
> <dan.magenheimer@oracle.com> wrote:
> 
> > I see this patch hasn't been taken yet.  Is there something
> > else I need to do or are you not in agreement that the
> > acpi part is cosmetic?
> >
> > Thanks,
> > Dan
> >
> >> -----Original Message-----
> >> From: Dan Magenheimer [mailto:dan.magenheimer@oracle.com]
> >> Sent: Thursday, February 07, 2008 1:54 PM
> >> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> >> Subject: RE: [Xen-devel] [PATCH] new hvm platform vhpet
> >> enable parameter
> >>
> >>
> >>> Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to
> >>> do this will
> >>> be to gate it on a flag set up in memory by hvmloader (we
> >>> already do this
> >>> e.g., for com1 and com2 -- see construct_bios_info_table() in
> >>> build.c in the
> >>> same directory). That might be a bit tricky as it probably
> >>> needs a bit of
> >>> ASL hacking, which has a little learning curve. I can take a
> >>> look maybe next
> >>> week.
> >>
> >> OK, here's the updated patch:
> >> 1) hpet instead of vhpet
> >> 2) against 3.2-testing tip
> >>
> >> This will work without the acpi changes so could be checked in
> >> independently.  Though it may be a bit misleading for the
> >> guest to print out that it found an hpet in acpi and then
> >> be unable to use it, the acpi part is largely cosmetic
> >> and (as you point out) a bit tricky so better left for
> >> your capable hands.
> >>
> >> Thanks,
> >> Dan
> >>
> >
> 
> 
>

Re: [PATCH] new hvm platform vhpet enable parameter

[Keir Fraser]

It enables it by default if the tools do not explicitly instruct either way.
In practice the tools will always explicitly instruct Xen for any new guest,
since xm picks default == 0. But it is important for old saved guest images
which will have no value for 'hpet': in this case we *must* enable the hpet
by default as the guest has probably already probed it.

I'm a bit undecided whether the tools should pick default of on or off. I
guess the default doesn't matter all that much, and it's better off by
default if it's not working that well.

I can easily add a similar switch for pmtimer.

 -- Keir

On 14/2/08 18:18, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> Thanks, sorry I missed it.
> 
> Glad I didn't have to deal with that ACPI asl stuff!  What a mess!
> 
> A question:  You added a snippet in arch/x86/hvm/hvm.c that
> sets the HPET_ENABLED parameter on.  I don't have a machine running
> xen-unstable at the moment so I can't verify, but this appears
> to be turning the virtual hpet on by default.  Was this intended?
> 
> 
>> -----Original Message-----
>> From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
>> Sent: Thursday, February 14, 2008 10:52 AM
>> To: dan.magenheimer@oracle.com; xen-devel@lists.xensource.com
>> Subject: Re: [Xen-devel] [PATCH] new hvm platform vhpet
>> enable parameter
>> 
>> 
>> It has been taken. Xen-unstable:17017.
>> 
>>  -- Keir
>> 
>> 
>> On 14/2/08 16:52, "Dan Magenheimer"
>> <dan.magenheimer@oracle.com> wrote:
>> 
>>> I see this patch hasn't been taken yet.  Is there something
>>> else I need to do or are you not in agreement that the
>>> acpi part is cosmetic?
>>> 
>>> Thanks,
>>> Dan
>>> 
>>>> -----Original Message-----
>>>> From: Dan Magenheimer [mailto:dan.magenheimer@oracle.com]
>>>> Sent: Thursday, February 07, 2008 1:54 PM
>>>> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
>>>> Subject: RE: [Xen-devel] [PATCH] new hvm platform vhpet
>>>> enable parameter
>>>> 
>>>> 
>>>>> Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to
>>>>> do this will
>>>>> be to gate it on a flag set up in memory by hvmloader (we
>>>>> already do this
>>>>> e.g., for com1 and com2 -- see construct_bios_info_table() in
>>>>> build.c in the
>>>>> same directory). That might be a bit tricky as it probably
>>>>> needs a bit of
>>>>> ASL hacking, which has a little learning curve. I can take a
>>>>> look maybe next
>>>>> week.
>>>> 
>>>> OK, here's the updated patch:
>>>> 1) hpet instead of vhpet
>>>> 2) against 3.2-testing tip
>>>> 
>>>> This will work without the acpi changes so could be checked in
>>>> independently.  Though it may be a bit misleading for the
>>>> guest to print out that it found an hpet in acpi and then
>>>> be unable to use it, the acpi part is largely cosmetic
>>>> and (as you point out) a bit tricky so better left for
>>>> your capable hands.
>>>> 
>>>> Thanks,
>>>> Dan
>>>> 
>>> 
>> 
>> 
>> 
> 

RE: [PATCH] new hvm platform vhpet enable parameter

[Dan Magenheimer]

> It enables it by default if the tools do not explicitly 
> instruct either way.... for old saved guest images...

OK, I see.  I'm glad you caught that.

But does that mean if a guest config file does not explicitly
have a "hpet = 0" line, the default will be on?  If so,
that's different from how I intended.  Perhaps the
code in XendDomainInfo _constructDomain should be modified
to set it to zero if "hpet is None"?  Else, I'll bet
guests created by virt-install will end up with hpet on.

Also it occurs to me it might be a good idea to add a
gdprintk in hvm.c so that whether the hpet is on or not
is visibly logged.

> I can easily add a similar switch for pmtimer.

Yes, from Dave Winchell's post today on another thread, it appears
that there are definitely some Linux versions that will default
to the pmtimer, which currently is not working well either.

I'm happy to submit a patch for pmtimer, but since you are probably
going to have to do the heavy lifting on the acpi asl stuff
anyway, you may as well.  But if you'd rather I give it a try,
let me know.

Thanks,
Dan

> -----Original Message-----
> From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
> Sent: Thursday, February 14, 2008 12:26 PM
> To: dan.magenheimer@oracle.com; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] new hvm platform vhpet 
> enable parameter
> 
> 
> It enables it by default if the tools do not explicitly 
> instruct either way.
> In practice the tools will always explicitly instruct Xen for 
> any new guest,
> since xm picks default == 0. But it is important for old 
> saved guest images
> which will have no value for 'hpet': in this case we *must* 
> enable the hpet
> by default as the guest has probably already probed it.
> 
> I'm a bit undecided whether the tools should pick default of 
> on or off. I
> guess the default doesn't matter all that much, and it's better off by
> default if it's not working that well.
> 
> I can easily add a similar switch for pmtimer.
> 
>  -- Keir
> 
> On 14/2/08 18:18, "Dan Magenheimer" 
> <dan.magenheimer@oracle.com> wrote:
> 
> > Thanks, sorry I missed it.
> >
> > Glad I didn't have to deal with that ACPI asl stuff!  What a mess!
> >
> > A question:  You added a snippet in arch/x86/hvm/hvm.c that
> > sets the HPET_ENABLED parameter on.  I don't have a machine running
> > xen-unstable at the moment so I can't verify, but this appears
> > to be turning the virtual hpet on by default.  Was this intended?
> >
> >
> >> -----Original Message-----
> >> From: Keir Fraser [mailto:Keir.Fraser@cl.cam.ac.uk]
> >> Sent: Thursday, February 14, 2008 10:52 AM
> >> To: dan.magenheimer@oracle.com; xen-devel@lists.xensource.com
> >> Subject: Re: [Xen-devel] [PATCH] new hvm platform vhpet
> >> enable parameter
> >>
> >>
> >> It has been taken. Xen-unstable:17017.
> >>
> >>  -- Keir
> >>
> >>
> >> On 14/2/08 16:52, "Dan Magenheimer"
> >> <dan.magenheimer@oracle.com> wrote:
> >>
> >>> I see this patch hasn't been taken yet.  Is there something
> >>> else I need to do or are you not in agreement that the
> >>> acpi part is cosmetic?
> >>>
> >>> Thanks,
> >>> Dan
> >>>
> >>>> -----Original Message-----
> >>>> From: Dan Magenheimer [mailto:dan.magenheimer@oracle.com]
> >>>> Sent: Thursday, February 07, 2008 1:54 PM
> >>>> To: 'Keir Fraser'; 'xen-devel@lists.xensource.com'
> >>>> Subject: RE: [Xen-devel] [PATCH] new hvm platform vhpet
> >>>> enable parameter
> >>>>
> >>>>
> >>>>> Yes, tools/firmware/hvmloader/acpi/dsdt.asl. The right way to
> >>>>> do this will
> >>>>> be to gate it on a flag set up in memory by hvmloader (we
> >>>>> already do this
> >>>>> e.g., for com1 and com2 -- see construct_bios_info_table() in
> >>>>> build.c in the
> >>>>> same directory). That might be a bit tricky as it probably
> >>>>> needs a bit of
> >>>>> ASL hacking, which has a little learning curve. I can take a
> >>>>> look maybe next
> >>>>> week.
> >>>>
> >>>> OK, here's the updated patch:
> >>>> 1) hpet instead of vhpet
> >>>> 2) against 3.2-testing tip
> >>>>
> >>>> This will work without the acpi changes so could be checked in
> >>>> independently.  Though it may be a bit misleading for the
> >>>> guest to print out that it found an hpet in acpi and then
> >>>> be unable to use it, the acpi part is largely cosmetic
> >>>> and (as you point out) a bit tricky so better left for
> >>>> your capable hands.
> >>>>
> >>>> Thanks,
> >>>> Dan
> >>>>
> >>>
> >>
> >>
> >>
> >
> 
> 
>

Re: [PATCH] new hvm platform vhpet enable parameter

[Keir Fraser]

On 14/2/08 19:48, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

>> It enables it by default if the tools do not explicitly
>> instruct either way.... for old saved guest images...
> 
> OK, I see.  I'm glad you caught that.
> 
> But does that mean if a guest config file does not explicitly
> have a "hpet = 0" line, the default will be on?  If so,
> that's different from how I intended.  Perhaps the
> code in XendDomainInfo _constructDomain should be modified
> to set it to zero if "hpet is None"?  Else, I'll bet
> guests created by virt-install will end up with hpet on.

Yes, that's possible. Possibly XendConfig.py should set a default. Or
virt-install itself should do so.

> Also it occurs to me it might be a good idea to add a
> gdprintk in hvm.c so that whether the hpet is on or not
> is visibly logged.

There are many config options for an HVM guest, none of which are logged by
Xen. It would be inconsistent to log just this one.

 -- Keir