Skip to content

Conversation

@Brian-Perkins
Copy link
Contributor

Hyper-V supports a restore partition time hypercall that resets the TSC to provided values. This call is typically made by a Windows guest when restoring from hibernate. The paravisor needs to support this call in order to allow the guest to be able to successfully perform the operation. Since the reference time is being changed, clocks need to be updated to maintain consistent foward progress.

Copilot AI review requested due to automatic review settings January 30, 2026 19:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements support for the Hyper-V restore partition time hypercall, which allows Windows guests to restore from hibernate by resetting the TSC to provided values. The implementation adds a new IOCTL that stops all CPUs, makes the hypercall to update the TSC, and updates clock state to maintain forward progress.

Changes:

  • Adds MSHV_RESTORE_PARTITION_TIME IOCTL and associated data structures in UAPI headers
  • Implements the IOCTL handler that uses stop_machine to safely update partition time
  • Exports hv_save_sched_clock_state and hv_restore_sched_clock_state functions for clock state management

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
include/uapi/linux/mshv.h Adds mshv_partition_time struct and MSHV_RESTORE_PARTITION_TIME IOCTL definition
include/uapi/hyperv/hvgdk_mini.h Adds HVCALL_RESTORE_PARTITION_TIME hypercall code and x86-64 specific hv_input_restore_partition_time struct
include/asm-generic/hyperv-tlfs.h Adds HVCALL_RESTORE_PARTITION_TIME hypercall code and hv_input_restore_partition_time struct
drivers/hv/mshv_vtl_main.c Implements the main IOCTL handler and stop_machine callback for restoring partition time
arch/x86/kernel/cpu/mshyperv.c Changes visibility of clock state save/restore functions from static to exported
arch/x86/include/asm/mshyperv.h Adds function declarations for exported clock state functions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1007 to +1013
struct hv_input_restore_partition_time {
u64 partition_id;
u32 tsc_sequence;
u32 reserved;
u64 reference_time_in_100_ns;
u64 tsc;
} __packed;
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate struct definition: struct hv_input_restore_partition_time is defined in both include/uapi/hyperv/hvgdk_mini.h (lines 1316-1322 with __u64/__u32 types) and include/asm-generic/hyperv-tlfs.h (lines 1007-1013 with u64/u32 types). This duplication could lead to compilation issues or confusion. Since hvgdk_mini.h is a UAPI header and uses UAPI types (__u64, __u32), while hyperv-tlfs.h is a kernel header using kernel types (u64, u32), only one definition should exist. Consider keeping the definition in the appropriate header based on where it's primarily used.

Suggested change
struct hv_input_restore_partition_time {
u64 partition_id;
u32 tsc_sequence;
u32 reserved;
u64 reference_time_in_100_ns;
u64 tsc;
} __packed;

Copilot uses AI. Check for mistakes.
Comment on lines +914 to +916
local_irq_save(irq_flags);
ret = stop_machine(restore_partition_time_with_cpus_stopped, &partition_time, cpumask_of(smp_processor_id()));
local_irq_restore(irq_flags);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary interrupt handling: The local_irq_save and local_irq_restore calls around stop_machine are redundant. The stop_machine function already handles interrupt disabling and restoration as part of its implementation. Calling local_irq_save before stop_machine is unnecessary and could potentially interfere with stop_machine's own interrupt handling. Consider removing these calls.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smp_processor_id() requires pre-emption to be disabled


// Stop other CPUs, using the current one to restore partition time.
local_irq_save(irq_flags);
ret = stop_machine(restore_partition_time_with_cpus_stopped, &partition_time, cpumask_of(smp_processor_id()));
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclear stop_machine usage: The code calls stop_machine(restore_partition_time_with_cpus_stopped, &partition_time, cpumask_of(smp_processor_id())), which stops all CPUs but only runs the callback on the current CPU. While this ensures no other CPUs are running during the TSC update (as indicated by the comment on line 878), it's worth verifying that making the HVCALL_RESTORE_PARTITION_TIME hypercall from only one CPU is sufficient to update the TSC for the entire system. If the TSC update is system-wide and a single hypercall is sufficient, the current implementation is correct. However, if each CPU needs to update its TSC state, the callback should run on all CPUs by passing NULL or cpu_online_mask instead.

Copilot uses AI. Check for mistakes.
Comment on lines 879 to 898
sched_clock_suspend();
timekeeping_suspend();
hv_save_sched_clock_state();

// Interrupts are disabled, make the hypercall to update the TSC.
input = *this_cpu_ptr(hyperv_pcpu_input_arg);
input->partition_id = HV_PARTITION_ID_SELF;
input->tsc_sequence = partition_time->tsc_sequence;
input->reserved = 0;
input->reference_time_in_100_ns = partition_time->reference_time_in_100_ns;
input->tsc = partition_time->tsc;
status = hv_do_hypercall(HVCALL_RESTORE_PARTITION_TIME, input, NULL);
if (!hv_result_success(status)) {
pr_err("HVCALL_RESTORE_PARTITION_TIME failed ! [Err: %#llx\n]", status);
result = -EINVAL;
}

// Restore clock state using current TSC value.
hv_restore_sched_clock_state();
timekeeping_resume();
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect use of internal timekeeping functions: The functions timekeeping_suspend() and timekeeping_resume() are internal kernel functions registered as syscore_ops callbacks (see kernel/time/timekeeping.c:1940-1943). They are not exported symbols and are designed to be called only by the kernel's suspend/resume infrastructure. Calling them directly from driver code bypasses the proper system suspend/resume mechanisms and may cause system instability. These functions acquire locks and modify global state that should only be changed during proper system suspend/resume. Consider using a different approach that doesn't require direct manipulation of timekeeping state, or coordinate with the timekeeping subsystem maintainers for a proper solution.

Copilot uses AI. Check for mistakes.
#include <asm/vmx.h>

#include "../../kernel/fpu/legacy.h"
#include "../../kernel/time/timekeeping.h"
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inclusion of internal kernel header: The file includes "../../kernel/time/timekeeping.h" using a relative path. This is an internal kernel header that is not part of the public kernel API. Driver code should not include internal headers using relative paths, as this breaks modularity and may cause build issues. The functions from this header (timekeeping_suspend, timekeeping_resume, sched_clock_suspend, sched_clock_resume) are also not exported symbols, which means they cannot be used from driver code that could potentially be built as a module.

Suggested change
#include "../../kernel/time/timekeeping.h"
#include <linux/timekeeping.h>

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant