question about hvm vhpet

question about hvm vhpet

[Tian, Kevin]

I'm a bit confused whether vhpet is enabled by default or not.

Per 17017, changelog says vhpet disabled by default, but the code
conflicts as below:
@@ -231,6 +231,8 @@ int hvm_domain_initialise(struct domain
     spin_lock_init(&d->arch.hvm_domain.pbuf_lock);
     spin_lock_init(&d->arch.hvm_domain.irq_lock);
     spin_lock_init(&d->arch.hvm_domain.uc_lock);
+
+    d->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] = 1;

Then if config file for hvm domain has no explicit 'hpet=0/1' option,
will xend issue a hypercall to set vhpet to '0' or the latter will be
left as '1'?

Thanks,
Kevin

RE: question about hvm vhpet

[Tian, Kevin]

>From: Tian, Kevin
>Sent: Friday, November 21, 2008 2:59 PM
>
>I'm a bit confused whether vhpet is enabled by default or not.
>
>Per 17017, changelog says vhpet disabled by default, but the code
>conflicts as below:
>@@ -231,6 +231,8 @@ int hvm_domain_initialise(struct domain
>     spin_lock_init(&d->arch.hvm_domain.pbuf_lock);
>     spin_lock_init(&d->arch.hvm_domain.irq_lock);
>     spin_lock_init(&d->arch.hvm_domain.uc_lock);
>+
>+    d->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] = 1;
>
>Then if config file for hvm domain has no explicit 'hpet=0/1' option,
>will xend issue a hypercall to set vhpet to '0' or the latter will be
>left as '1'?
>

It's the former. Anyway, let's keep code aligned with original
purpose.
----
Disable vhpet for hvm domain by default.

Signed-off-by Kevin Tian <kevin.tian@intel.com>

diff -r c70317be929c xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c    Fri Nov 21 07:39:30 2008 +0800
+++ b/xen/arch/x86/hvm/hvm.c    Fri Nov 21 07:40:01 2008 +0800
@@ -305,8 +305,6 @@ int hvm_domain_initialise(struct domain

     hvm_init_guest_time(d);

-    d->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] = 1;
-
     hvm_init_cacheattr_region_list(d);

     rc = paging_enable(d, PG_refcounts|PG_translate|PG_external);

Thanks,
Kevin

Re: RE: question about hvm vhpet

[Keir Fraser]

On 21/11/08 08:10, "Tian, Kevin" <kevin.tian@intel.com> wrote:

>> Then if config file for hvm domain has no explicit 'hpet=0/1' option,
>> will xend issue a hypercall to set vhpet to '0' or the latter will be
>> left as '1'?
>> 
> 
> It's the former. Anyway, let's keep code aligned with original
> purpose.

The hypervisor code is as it is for old saved guests which do not explicitly
specify HPET. They will have been booted with HPET unconditionally enabled,
and it must not disappear under their feet if they are restored on a newer
hypervisor!

 -- Keir

RE: RE: question about hvm vhpet

[Tian, Kevin]

>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
>Sent: Friday, November 21, 2008 5:03 PM
>On 21/11/08 08:10, "Tian, Kevin" <kevin.tian@intel.com> wrote:
>
>>> Then if config file for hvm domain has no explicit 
>'hpet=0/1' option,
>>> will xend issue a hypercall to set vhpet to '0' or the 
>latter will be
>>> left as '1'?
>>> 
>> 
>> It's the former. Anyway, let's keep code aligned with original
>> purpose.
>
>The hypervisor code is as it is for old saved guests which do 
>not explicitly
>specify HPET. They will have been booted with HPET 
>unconditionally enabled,
>and it must not disappear under their feet if they are 
>restored on a newer
>hypervisor!
>

Thanks for pointing out reason.
Kevin

RE: RE: question about hvm vhpet

[Dan Magenheimer]

Agreed.... at least unless/until Dave Winchell's hpet
work gets resubmitted and accepted, hpet is not reliable
as a timer for most guests.  And some OS's will
always automatically choose hpet if it is enabled
even, in some cases, if another clocksource was intended.

So off is better... but we should be aware that turning
it off may provoke significant timer issues for some
customers who upgrade.  The ideal solution is to get
Dave's code in and leave the vhpet default to on, but
I don't think Dave has had any time to work on this
(and I haven't had time either).

Dan

P.S. IIRC, a new timer_mode needs to be added that ensures
that two ticks are never delivered too close together,
as this will cause time to run too fast.

> -----Original Message-----
> From: Tian, Kevin [mailto:kevin.tian@intel.com]
> Sent: Friday, November 21, 2008 1:10 AM
> To: Tian, Kevin; xen-devel@lists.xensource.com
> Subject: [Xen-devel] RE: question about hvm vhpet
> 
> 
> >From: Tian, Kevin
> >Sent: Friday, November 21, 2008 2:59 PM
> >
> >I'm a bit confused whether vhpet is enabled by default or not.
> >
> >Per 17017, changelog says vhpet disabled by default, but the code
> >conflicts as below:
> >@@ -231,6 +231,8 @@ int hvm_domain_initialise(struct domain
> >     spin_lock_init(&d->arch.hvm_domain.pbuf_lock);
> >     spin_lock_init(&d->arch.hvm_domain.irq_lock);
> >     spin_lock_init(&d->arch.hvm_domain.uc_lock);
> >+
> >+    d->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] = 1;
> >
> >Then if config file for hvm domain has no explicit 'hpet=0/1' option,
> >will xend issue a hypercall to set vhpet to '0' or the latter will be
> >left as '1'?
> >
> 
> It's the former. Anyway, let's keep code aligned with original
> purpose.
> ----
> Disable vhpet for hvm domain by default.
> 
> Signed-off-by Kevin Tian <kevin.tian@intel.com>
> 
> diff -r c70317be929c xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c    Fri Nov 21 07:39:30 2008 +0800
> +++ b/xen/arch/x86/hvm/hvm.c    Fri Nov 21 07:40:01 2008 +0800
> @@ -305,8 +305,6 @@ int hvm_domain_initialise(struct domain
> 
>      hvm_init_guest_time(d);
> 
> -    d->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] = 1;
> -
>      hvm_init_cacheattr_region_list(d);
> 
>      rc = paging_enable(d, PG_refcounts|PG_translate|PG_external);
> 
> Thanks,
> Kevin
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

Re: RE: question about hvm vhpet

[Keir Fraser]

On 21/11/08 15:26, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> Agreed.... at least unless/until Dave Winchell's hpet
> work gets resubmitted and accepted, hpet is not reliable
> as a timer for most guests.  And some OS's will
> always automatically choose hpet if it is enabled
> even, in some cases, if another clocksource was intended.
> 
> So off is better... but we should be aware that turning
> it off may provoke significant timer issues for some
> customers who upgrade.  The ideal solution is to get
> Dave's code in and leave the vhpet default to on, but
> I don't think Dave has had any time to work on this
> (and I haven't had time either).

vhpet is now based on vpt.c.

 -- Keir

RE: RE: question about hvm vhpet

[Dan Magenheimer]

> vhpet is now based on vpt.c.

Yes, understood.  IIRC the main problem is that a new
timer mode is needed that guarantees that no two
ticks can be delivered too close together (and doesn't
ever care if they are delivered too far apart).
I think I had suggested the mode be called "no_fast_ticks".

However, my memory may be faulty here, so I cc'ed Dave
hoping he would chime in.

Dan

Re: RE: question about hvm vhpet

[Dave Winchell]

Dan Magenheimer wrote:

>>vhpet is now based on vpt.c.
>>    
>>
>
>Yes, understood.  IIRC the main problem is that a new
>timer mode is needed that guarantees that no two
>ticks can be delivered too close together (and doesn't
>ever care if they are delivered too far apart).
>I think I had suggested the mode be called "no_fast_ticks".
>
>However, my memory may be faulty here, so I cc'ed Dave
>hoping he would chime in.
>
>Dan
>  
>
Hi Dan, Keir, Kevin:

Yes, I have not had the time to work on the hpet resubmission.
I hope to be able to do so soon.

Dan, the no_fast_ticks policy is one of the things that made
our hpet implementation work well.

Keir, thanks for doing the layering work. When I get back to working
on this I'll start with what you have and characterize it.
Then go from there.

-Dave