Xen devel - Feb 2008 - [PATCH] new hvm platform vhpet enable parameter

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

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

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

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


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

> 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


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

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



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

> 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


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

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



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

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
> 

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

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


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

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

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

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


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

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

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

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



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