[PATCH 2/2] x86/hvm: sync APs TSC with the BSP at creation only

[chunjie.zhu@xxxxxxxxxx]

On Fri, Oct 30, 2026 at 02:24:19 PM, Roger Pau Monne <roger@xxxxxxxxxxxxxx> 
wrote:

> The current logic syncs the APs' TSC with the BSP both at reset and when
> setting the vCPU state.  This doesn't match hardware behavior, where APs
> are only in sync at platform start.

> Move the TSC sync so it's done in hvm_vcpu_initialise(), that's called a
> single time at vCPU creation.  This matches the hardware behavior.
> INIT-SIPI and VCPUOP_initialise no longer re-sync a vCPU's TSC to vCPU 0's.
>
> On migration the TSC offset for offline vCPUs is not transferred
> (hvm_save_cpu_ctxt() skips offline vCPUs).  To avoid an unexpected skew
> synchronize offline vCPUs with the first online vCPU, taking into account
> the TSC_ADJUST value.
>
> The clearing of TSC_ADJUST is also made consistent with hardware behavior:
> clear it on S3 resume, but not across an INIT-SIPI or VCPUOP_initialise.
> Additionally, on S3 resume start all vCPUs from a shared TSC reading, so
> they are in sync.
>
> Fixes: c31f8336e64e ("[HVM] Initialise AP TSC to same as BP TSC.")

This looks wrong.  c31f8336e64e only added the sync to hvm_bringup_ap(),
which no longer exists.  The code removed here comes from bcba667e12e3
(INIT-SIPI) and 192df6f9122d (VCPUOP_initialise).

> Fixes: 6e0336351ed9 ("x86: Implement TSC adjust feature for HVM guest")
> Signed-off-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/domain.c |  5 -----
>  xen/arch/x86/hvm/hvm.c    | 19 +++++++++++--------
>  xen/arch/x86/hvm/save.c   | 34 ++++++++++++++++++++++++++++++++--
>  xen/arch/x86/time.c       |  5 ++---
>  4 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 978d8b222256..71f49ec7ce9f 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -312,11 +312,6 @@ int arch_set_info_hvm_guest(struct vcpu *v, const struct 
> vcpu_hvm_context *ctx)
>      hvm_set_segment_register(v, x86_seg_es, &es);
>      hvm_set_segment_register(v, x86_seg_tss, &tr);
>
> -    /* Sync AP's TSC with BSP's. */
> -    v->arch.hvm.cache_tsc_offset =
> -        d->vcpu[0]->arch.hvm.cache_tsc_offset;
> -    hvm_set_tsc_offset(v, v->arch.hvm.cache_tsc_offset);
> -
>      paging_update_paging_modes(v);
>
>      hvmemul_cancel(v);
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ef9c7cd05777..81e90cc8fc8e 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1667,6 +1667,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
>          /* Init guest TSC to start from zero. */
>          hvm_set_guest_tsc(v, 0);
>      }
> +    else
> +    {
> +        /* Start in step with vCPU 0, as all CPUs do at power-on. */
> +        v->arch.hvm.cache_tsc_offset = d->vcpu[0]->arch.hvm.cache_tsc_offset;
> +        hvm_set_tsc_offset(v, v->arch.hvm.cache_tsc_offset);
> +    }
>
>      return 0;
>
> @@ -4021,13 +4027,6 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, 
> uint16_t ip)
>      hvm_set_segment_register(v, x86_seg_gdt, &reg);
>      hvm_set_segment_register(v, x86_seg_idt, &reg);
>
> -    /* Sync AP's TSC with BSP's. */
> -    v->arch.hvm.cache_tsc_offset =
> -        v->domain->vcpu[0]->arch.hvm.cache_tsc_offset;
> -    hvm_set_tsc_offset(v, v->arch.hvm.cache_tsc_offset);
> -
> -    v->arch.hvm.msr_tsc_adjust = 0;
> -
>      paging_update_paging_modes(v);
>
>      v->arch.flags |= TF_kernel_mode;
> @@ -4079,9 +4078,13 @@ static void hvm_s3_resume(struct domain *d)
>      if ( test_and_clear_bool(d->arch.hvm.is_s3_suspended) )
>      {
>          struct vcpu *v;
> +        uint64_t at_tsc = rdtsc();
>
>          for_each_vcpu( d, v )
> -            hvm_set_guest_tsc(v, 0);
> +        {
> +            v->arch.hvm.msr_tsc_adjust = 0;
> +            hvm_set_guest_tsc_fixed(v, 0, at_tsc);
> +        }
>          domain_unpause(d);
>      }
>  }
> diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c
> index 8ab64057064e..65ebfd384d69 100644
> --- a/xen/arch/x86/hvm/save.c
> +++ b/xen/arch/x86/hvm/save.c
> @@ -289,6 +289,7 @@ int hvm_load(struct domain *d, bool real, 
> hvm_domain_context_t *h)
>  {
>      const struct hvm_save_header *hdr;
>      struct hvm_save_descriptor *desc;
> +    struct vcpu *v;
>      int rc;
>
>      if ( d->is_dying )
> @@ -302,8 +303,6 @@ int hvm_load(struct domain *d, bool real, 
> hvm_domain_context_t *h)
>      rc = arch_hvm_check(d, hdr);
>      if ( real )
>      {
> -        struct vcpu *v;
> -
>          ASSERT(!rc);
>
>          /*
> @@ -341,6 +340,37 @@ int hvm_load(struct domain *d, bool real, 
> hvm_domain_context_t *h)
>              /* Reset cursor for hvm_load(, true, ). */
>              if ( !real )
>                  h->cur = 0;
> +            else
> +            {
> +                const struct vcpu *base = NULL;
> +
> +                /*
> +                 * Offline vCPUs have no CPU record.  Adjust the TSC offset
> +                 * based on the first online vCPU, taking into account
> +                 * TSC_ADJUST which is saved regardless of whether the vCPU 
> is
> +                 * offline.
> +                 */
> +                for_each_vcpu(d, v)
> +                    if ( !test_bit(_VPF_down, &v->pause_flags) )
> +                    {
> +                        base = v;
> +                        break;
> +                    }
> +
> +                if ( !base )
> +                    return 0;

This silently skips the offline vCPUs.  If that ever causes a problem it
will be hard to diagnose, so please add a comment or a log message.

> +
> +                for_each_vcpu(d, v)
> +                    if ( test_bit(_VPF_down, &v->pause_flags) )
> +                    {
> +                        v->arch.hvm.cache_tsc_offset =
> +                            base->arch.hvm.cache_tsc_offset -
> +                            base->arch.hvm.msr_tsc_adjust +
> +                            v->arch.hvm.msr_tsc_adjust;
> +                        hvm_set_tsc_offset(v, v->arch.hvm.cache_tsc_offset);
> +                    }
> +            }
> +
>              return 0;
>          }
>
> diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
> index f37fddc61e47..79c7123c0a5b 100644
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -3087,9 +3087,8 @@ int tsc_set_info(struct domain *d,
>               * set_tsc_offset() is called from hvm_vcpu_initialise() before
>               * tsc_set_info(). New vtsc mode may require recomputing TSC
>               * offset.
> -             * We only need to do this for BSP during initial boot. APs will
> -             * call set_tsc_offset() later from hvm_vcpu_reset_state() and 
> they
> -             * will sync their TSC to BSP's sync_tsc.
> +             * We only need to do this for BSP during initial boot.  APs copy
> +             * vCPU 0's offset when created in hvm_vcpu_initialise().
>               */
>              d->arch.hvm.sync_tsc = rdtsc();
>              hvm_set_tsc_offset(d->vcpu[0],
> -- 
> 2.55.0
>
>
>