[PATCH 4/5] x86/time: gtsc_to_gtime() is HVM-only

Jan Beulich posted 5 patches 1 month ago
[PATCH 4/5] x86/time: gtsc_to_gtime() is HVM-only
Posted by Jan Beulich 1 month ago
Omit the function when HVM=n. With that the !HVM logic can also go away;
leave an assertion.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2840,14 +2840,13 @@ uint64_t gtime_to_gtsc(const struct doma
     return scale_delta(time, &d->arch.ns_to_vtsc);
 }
 
+#ifdef CONFIG_HVM
 uint64_t gtsc_to_gtime(const struct domain *d, uint64_t tsc)
 {
-    u64 time = scale_delta(tsc, &d->arch.vtsc_to_ns);
-
-    if ( !is_hvm_domain(d) )
-        time += d->arch.vtsc_offset;
-    return time;
+    ASSERT(is_hvm_domain(d));
+    return scale_delta(tsc, &d->arch.vtsc_to_ns);
 }
+#endif /* CONFIG_HVM */
 
 uint64_t pv_soft_rdtsc(const struct vcpu *v, const struct cpu_user_regs *regs)
 {
Re: [PATCH 4/5] x86/time: gtsc_to_gtime() is HVM-only
Posted by Roger Pau Monné 3 weeks, 4 days ago
On Tue, Jan 06, 2026 at 02:59:43PM +0100, Jan Beulich wrote:
> Omit the function when HVM=n. With that the !HVM logic can also go away;
> leave an assertion.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -2840,14 +2840,13 @@ uint64_t gtime_to_gtsc(const struct doma
>      return scale_delta(time, &d->arch.ns_to_vtsc);
>  }
>  
> +#ifdef CONFIG_HVM
>  uint64_t gtsc_to_gtime(const struct domain *d, uint64_t tsc)
>  {
> -    u64 time = scale_delta(tsc, &d->arch.vtsc_to_ns);
> -
> -    if ( !is_hvm_domain(d) )
> -        time += d->arch.vtsc_offset;
> -    return time;
> +    ASSERT(is_hvm_domain(d));
> +    return scale_delta(tsc, &d->arch.vtsc_to_ns);
>  }
> +#endif /* CONFIG_HVM */

Seeing this is solely used by the vlapic code, did you consider
keeping scale_delta() non-static, and then moving gtsc_to_gtime() into
vlapic.c?

It would result in less ifdefery overall.

Thanks, Roger.
Re: [PATCH 4/5] x86/time: gtsc_to_gtime() is HVM-only
Posted by Roger Pau Monné 3 weeks, 4 days ago
On Wed, Jan 14, 2026 at 10:43:38AM +0100, Roger Pau Monné wrote:
> On Tue, Jan 06, 2026 at 02:59:43PM +0100, Jan Beulich wrote:
> > Omit the function when HVM=n. With that the !HVM logic can also go away;
> > leave an assertion.
> > 
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > --- a/xen/arch/x86/time.c
> > +++ b/xen/arch/x86/time.c
> > @@ -2840,14 +2840,13 @@ uint64_t gtime_to_gtsc(const struct doma
> >      return scale_delta(time, &d->arch.ns_to_vtsc);
> >  }
> >  
> > +#ifdef CONFIG_HVM
> >  uint64_t gtsc_to_gtime(const struct domain *d, uint64_t tsc)
> >  {
> > -    u64 time = scale_delta(tsc, &d->arch.vtsc_to_ns);
> > -
> > -    if ( !is_hvm_domain(d) )
> > -        time += d->arch.vtsc_offset;
> > -    return time;
> > +    ASSERT(is_hvm_domain(d));
> > +    return scale_delta(tsc, &d->arch.vtsc_to_ns);
> >  }
> > +#endif /* CONFIG_HVM */
> 
> Seeing this is solely used by the vlapic code, did you consider
> keeping scale_delta() non-static, and then moving gtsc_to_gtime() into
> vlapic.c?
> 
> It would result in less ifdefery overall.

I see now this might not be appropriate, gtsc_to_gtime() is the pair
to gtime_to_gtsc(), which is used in other places.  It looks like
gtsc_to_gtime() could also gain users in order parts of the code, and
hence making static to vlapic might not be the best approach.

I'm not overly happy with adding more ifdefary to time.c, and the
asymmetry this creates with PV guest not having a gtsc_to_gtime(), but
it's otherwise dead code, so:

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.