In a TDX VM without paravisor, currently the default timer is the Hyper-V timer, which depends on the slow VM Reference Counter MSR: the Hyper-V TSC page is not enabled in such a VM because the VM uses Invariant TSC as a better clocksource and it's challenging to mark the Hyper-V TSC page shared in very early boot.
Lower the rating of the Hyper-V timer so the local APIC timer becomes the the default timer in such a VM. This change should cause no perceivable performance difference.
Cc: stable@vger.kernel.org # 6.6+ Signed-off-by: Dexuan Cui decui@microsoft.com --- arch/x86/kernel/cpu/mshyperv.c | 6 +++++- drivers/clocksource/hyperv_timer.c | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e0fd57a8ba840..745af47ca0459 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -449,9 +449,13 @@ static void __init ms_hyperv_init_platform(void) ms_hyperv.hints &= ~HV_X64_APIC_ACCESS_RECOMMENDED;
if (!ms_hyperv.paravisor_present) { - /* To be supported: more work is required. */ + /* Use Invariant TSC as a better clocksource. */ ms_hyperv.features &= ~HV_MSR_REFERENCE_TSC_AVAILABLE;
+ /* Use the Ref Counter in case Invariant TSC is unavailable. */ + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT)) + pr_warn("Hyper-V: Invariant TSC is unavailable\n"); + /* HV_MSR_CRASH_CTL is unsupported. */ ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index b2a080647e413..99177835cadec 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -137,7 +137,21 @@ static int hv_stimer_init(unsigned int cpu) ce->name = "Hyper-V clockevent"; ce->features = CLOCK_EVT_FEAT_ONESHOT; ce->cpumask = cpumask_of(cpu); - ce->rating = 1000; + + /* + * Lower the rating of the Hyper-V timer in a TDX VM without paravisor, + * so the local APIC timer (lapic_clockevent) is the default timer in + * such a VM. The Hyper-V timer is not preferred in such a VM because + * it depends on the slow VM Reference Counter MSR (the Hyper-V TSC + * page is not enbled in such a VM because the VM uses Invariant TSC + * as a better clocksource and it's challenging to mark the Hyper-V + * TSC page shared in very early boot). + */ + if (!ms_hyperv.paravisor_present && hv_isolation_type_tdx()) + ce->rating = 90; + else + ce->rating = 1000; + ce->set_state_shutdown = hv_ce_shutdown; ce->set_state_oneshot = hv_ce_set_oneshot; ce->set_next_event = hv_ce_set_next_event;
On 6/18/2024 5:25 PM, Dexuan Cui wrote:
In a TDX VM without paravisor, currently the default timer is the Hyper-V timer, which depends on the slow VM Reference Counter MSR: the Hyper-V TSC page is not enabled in such a VM because the VM uses Invariant TSC as a better clocksource and it's challenging to mark the Hyper-V TSC page shared in very early boot.
Lower the rating of the Hyper-V timer so the local APIC timer becomes the the default timer in such a VM. This change should cause no perceivable performance difference.
Cc: stable@vger.kernel.org # 6.6+ Signed-off-by: Dexuan Cui decui@microsoft.com
arch/x86/kernel/cpu/mshyperv.c | 6 +++++- drivers/clocksource/hyperv_timer.c | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e0fd57a8ba840..745af47ca0459 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -449,9 +449,13 @@ static void __init ms_hyperv_init_platform(void) ms_hyperv.hints &= ~HV_X64_APIC_ACCESS_RECOMMENDED; if (!ms_hyperv.paravisor_present) {
/* To be supported: more work is required. */
/* Use Invariant TSC as a better clocksource. */ ms_hyperv.features &= ~HV_MSR_REFERENCE_TSC_AVAILABLE;
/* Use the Ref Counter in case Invariant TSC is unavailable. */
if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
pr_warn("Hyper-V: Invariant TSC is unavailable\n");
/* HV_MSR_CRASH_CTL is unsupported. */ ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index b2a080647e413..99177835cadec 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -137,7 +137,21 @@ static int hv_stimer_init(unsigned int cpu) ce->name = "Hyper-V clockevent"; ce->features = CLOCK_EVT_FEAT_ONESHOT; ce->cpumask = cpumask_of(cpu);
- ce->rating = 1000;
- /*
* Lower the rating of the Hyper-V timer in a TDX VM without paravisor,
* so the local APIC timer (lapic_clockevent) is the default timer in
* such a VM. The Hyper-V timer is not preferred in such a VM because
* it depends on the slow VM Reference Counter MSR (the Hyper-V TSC
* page is not enbled in such a VM because the VM uses Invariant TSC
* as a better clocksource and it's challenging to mark the Hyper-V
* TSC page shared in very early boot).
*/
- if (!ms_hyperv.paravisor_present && hv_isolation_type_tdx())
ce->rating = 90;
- else
ce->rating = 1000;
- ce->set_state_shutdown = hv_ce_shutdown; ce->set_state_oneshot = hv_ce_set_oneshot; ce->set_next_event = hv_ce_set_next_event;
LGTM.
Reviewed-by: Roman Kisel romank@linux.microsoft.com
From: Dexuan Cui decui@microsoft.com Sent: Tuesday, June 18, 2024 5:25 PM
In a TDX VM without paravisor, currently the default timer is the Hyper-V timer, which depends on the slow VM Reference Counter MSR: the Hyper-V TSC page is not enabled in such a VM because the VM uses Invariant TSC as a better clocksource and it's challenging to mark the Hyper-V TSC page shared in very early boot.
Lower the rating of the Hyper-V timer so the local APIC timer becomes the the default timer in such a VM. This change should cause no perceivable performance difference.
Cc: stable@vger.kernel.org # 6.6+ Signed-off-by: Dexuan Cui decui@microsoft.com
arch/x86/kernel/cpu/mshyperv.c | 6 +++++- drivers/clocksource/hyperv_timer.c | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e0fd57a8ba840..745af47ca0459 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -449,9 +449,13 @@ static void __init ms_hyperv_init_platform(void) ms_hyperv.hints &= ~HV_X64_APIC_ACCESS_RECOMMENDED;
if (!ms_hyperv.paravisor_present) {
/* To be supported: more work is required. */
/* Use Invariant TSC as a better clocksource. */
I got confused by this comment, partly because I've forgotten the meaning of the ms_hyperv.feature flags. :-( Perhaps you could be more explicit in the comment and say "Mark the Hyper-V TSC page feature as disabled in a TDX VM so that the Invariant TSC, which is a better clocksource anyway, is used instead."
ms_hyperv.features &= ~HV_MSR_REFERENCE_TSC_AVAILABLE;
/* Use the Ref Counter in case Invariant TSC is unavailable. */
if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT))
pr_warn("Hyper-V: Invariant TSC is unavailable\n");
The above comment was even more confusing, because the code block doesn't do anything except print a message. The code doesn't force the use of the Ref Counter. I'd suggest something like: "The Invariant TSC is expected to be available, but if not, print a warning message. The slower Hyper-V MSR-based Ref Counter should end up being the clocksource."
Michael
/* HV_MSR_CRASH_CTL is unsupported. */ ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c index b2a080647e413..99177835cadec 100644 --- a/drivers/clocksource/hyperv_timer.c +++ b/drivers/clocksource/hyperv_timer.c @@ -137,7 +137,21 @@ static int hv_stimer_init(unsigned int cpu) ce->name = "Hyper-V clockevent"; ce->features = CLOCK_EVT_FEAT_ONESHOT; ce->cpumask = cpumask_of(cpu);
- ce->rating = 1000;
- /*
* Lower the rating of the Hyper-V timer in a TDX VM without paravisor,
* so the local APIC timer (lapic_clockevent) is the default timer in
* such a VM. The Hyper-V timer is not preferred in such a VM because
* it depends on the slow VM Reference Counter MSR (the Hyper-V TSC
* page is not enbled in such a VM because the VM uses Invariant TSC
* as a better clocksource and it's challenging to mark the Hyper-V
* TSC page shared in very early boot).
*/
- if (!ms_hyperv.paravisor_present && hv_isolation_type_tdx())
ce->rating = 90;
- else
ce->rating = 1000;
- ce->set_state_shutdown = hv_ce_shutdown; ce->set_state_oneshot = hv_ce_set_oneshot; ce->set_next_event = hv_ce_set_next_event;
-- 2.25.1
From: Michael Kelley mhklinux@outlook.com Sent: Thursday, June 20, 2024 2:19 PM To: Dexuan Cui decui@microsoft.com; KY Srinivasan [...]
--- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -449,9 +449,13 @@ static void __init ms_hyperv_init_platform(void) ms_hyperv.hints &=
~HV_X64_APIC_ACCESS_RECOMMENDED;
if (!ms_hyperv.paravisor_present) {
/* To be supported: more work is required.
*/
/* Use Invariant TSC as a better
clocksource. */
I got confused by this comment, partly because I've forgotten the meaning of the ms_hyperv.feature flags. :-( Perhaps you could be more explicit in the comment and say "Mark the Hyper-V TSC page feature as disabled in a TDX VM so that the Invariant TSC, which is a better clocksource anyway, is used instead."
ms_hyperv.features &=
~HV_MSR_REFERENCE_TSC_AVAILABLE;
/* Use the Ref Counter in case Invariant
TSC is unavailable. */
if (!(ms_hyperv.features &
HV_ACCESS_TSC_INVARIANT))
pr_warn("Hyper-V: Invariant TSC is
unavailable\n");
The above comment was even more confusing, because the code block doesn't do anything except print a message. The code doesn't force the use of the Ref Counter. I'd suggest something like: "The Invariant TSC is expected to be available, but if not, print a warning message. The slower Hyper-V MSR-based Ref Counter should end up being the clocksource."
Michael
Thanks for the good "comments"! :-)
I'm going to post v2 with the change below.
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c index e0fd57a8ba840..954b7cbfa2f02 100644 --- a/arch/x86/kernel/cpu/mshyperv.c +++ b/arch/x86/kernel/cpu/mshyperv.c @@ -449,9 +449,23 @@ static void __init ms_hyperv_init_platform(void) ms_hyperv.hints &= ~HV_X64_APIC_ACCESS_RECOMMENDED;
if (!ms_hyperv.paravisor_present) { - /* To be supported: more work is required. */ + /* + * Mark the Hyper-V TSC page feature as disabled + * in a TDX VM without paravisor so that the + * Invariant TSC, which is a better clocksource + * anyway, is used instead. + */ ms_hyperv.features &= ~HV_MSR_REFERENCE_TSC_AVAILABLE;
+ /* + * The Invariant TSC is expected to be available + * in a TDX VM without paravisor, but if not, + * print a warning message. The slower Hyper-V MSR-based + * Ref Counter should end up being the clocksource. + */ + if (!(ms_hyperv.features & HV_ACCESS_TSC_INVARIANT)) + pr_warn("Hyper-V: Invariant TSC is unavailable\n"); + /* HV_MSR_CRASH_CTL is unsupported. */ ms_hyperv.misc_features &= ~HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE;
linux-stable-mirror@lists.linaro.org