On Tue Jul 8, 2025 at 7:40 PM CEST, Stefano Stabellini wrote:
> 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.stabell...@amd.com>
>> > ---
>> > 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);
>
Yes, I think that would do.
Cheers,
Alejandro
