Re: [PATCH 1/2] xen/x86: don't send IPI to sync TSC when it is reliable

[Stefano Stabellini <stefano.stabellini@xxxxxxx>]

On Tue, 8 Jul 2025, Alejandro Vallejo wrote:
> On Tue Jul 8, 2025 at 2:07 AM CEST, Stefano Stabellini wrote:
> > On real time configuration with the null scheduler, we shouldn't
> > interrupt the guest execution unless strictly necessary: the guest could
> > be a real time guest (e.g. FreeRTOS) and interrupting its execution
> > could lead to a missed deadline.
> >
> > The principal source of interruptions is IPIs. Remove the unnecessary
> > IPI on all physical CPUs to sync the TSC when the TSC is known to be
> > reliable.
> >
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> > ---
> >  xen/arch/x86/time.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> > index 59129f419d..bfd022174a 100644
> > --- a/xen/arch/x86/time.c
> > +++ b/xen/arch/x86/time.c
> > @@ -2303,6 +2303,10 @@ static void cf_check time_calibration(void *unused)
> >          local_irq_enable();
> >      }
> >  
> > +    if ( boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> > +         boot_cpu_has(X86_FEATURE_TSC_RELIABLE) )
> > +        return;
> > +
> 
> This should check "(tsc_flags & TSC_RELIABLE_SOCKET)" as well. The TSCs might
> still be unsynchronized across sockets.
>
> I'm still quite confused as to how Xen (mis)handles time, but wouldn't this 
> need
> to go inside the branch above? If the clocksource is not the TSC as well the 
> TSC
> can still drift with respect to the actual clocksource (PIT, HPET or ACPI 
> timer).

I can move it inside the previous if


> If so, we could probably do an early return in the branch above ignoring the
> conditions (they are required for picking the TSC clocksource already, 
> including
> synchronization across sockets).
> 
> Another matter is whether we could drop the "master_stime" write. Would we
> care about it at all?

I'll drop it.

Is this what you had in mind?


diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 59129f419d..d72e640f72 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2297,11 +2297,7 @@ static void cf_check time_calibration(void *unused)
     };
 
     if ( clocksource_is_tsc() )
-    {
-        local_irq_disable();
-        r.master_stime = read_platform_stime(&r.master_tsc_stamp);
-        local_irq_enable();
-    }
+        return;
 
     cpumask_copy(&r.cpu_calibration_map, &cpu_online_map);