Re: [PATCH] x86/hyperv: Set X86_FEATURE_TSC_RELIABLE unconditionally

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 12, 2024 at 07:48:06PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx> Sent: Tuesday, November 12, 2024 10:18 AM
> > 
> > Enable X86_FEATURE_TSC_RELIABLE by default as X86_FEATURE_TSC_RELIABLE is
> > independent from invariant TSC and should have never been gated by the
> > HV_ACCESS_TSC_INVARIANT privilege.
> 
> I think originally X86_FEATURE_TSC_RELIABLE was gated by the Hyper-V
> TSC Invariant feature because otherwise VM live migration may cause
> the TSC value reported by the RDTSC/RDTSCP instruction in the guest
> to abruptly change frequency and value. In such cases, the TSC isn't
> useable by the kernel or user space.
> 
> Enabling the Hyper-V TSC Invariant feature fixes that by using the
> hardware scaling available in more recent processors to automatically
> fixup the TSC value returned by RDTSC/RDTSCP in the guest.
> 
> Is there a practical problem that is fixed by always enabling
> X86_FEATURE_TSC_RELIABLE?
> 

The particular problem is that HV_ACCESS_TSC_INVARIANT is not set for the
nested root, which in turn leads to keeping tsc clocksource watchdog
thread and TSC sycn check timer around.

But the live migration concern you raised is indeed still out there.

Thank you for the input Michael, I'll think more about it.

Stanislav

> Michael
> 
> > 
> > To elaborate, the HV_ACCESS_TSC_INVARIANT privilege allows certain types of
> > guests to opt-in to invariant TSC by writing the
> > HV_X64_MSR_TSC_INVARIANT_CONTROL register. Not all guests will have this
> > privilege and the hypervisor will automatically opt-in certain types of
> > guests (e.g. EXO partitions) to invariant TSC, but this functionality is
> > unrelated to the TSC reliability.
> > 
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  arch/x86/kernel/cpu/mshyperv.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> > index d18078834ded..14412afcc398 100644
> > --- a/arch/x86/kernel/cpu/mshyperv.c
> > +++ b/arch/x86/kernel/cpu/mshyperv.c
> > @@ -515,7 +515,7 @@ static void __init ms_hyperv_init_platform(void)
> >  	machine_ops.crash_shutdown = hv_machine_crash_shutdown;
> >  #endif
> >  #endif
> > -	if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT) {
> > +	if (ms_hyperv.features & HV_ACCESS_TSC_INVARIANT)
> >  		/*
> >  		 * Writing to synthetic MSR 0x40000118 updates/changes the
> >  		 * guest visible CPUIDs. Setting bit 0 of this MSR  enables
> > @@ -526,8 +526,8 @@ static void __init ms_hyperv_init_platform(void)
> >  		 * is called.
> >  		 */
> >  		wrmsrl(HV_X64_MSR_TSC_INVARIANT_CONTROL, HV_EXPOSE_INVARIANT_TSC);
> > -		setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > -	}
> > +
> > +	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
> > 
> >  	/*
> >  	 * Generation 2 instances don't support reading the NMI status from
> > 
> > 
> 




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux