Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Setting `x86_platform.save_sched_clock_state` and 
`x86_platform.restore_sched_clock_state` to `NULL` causes a kernel panic during 
suspend and resume operations.
--

commit 2ff4708308b8e09ffda052258a76f33f136c2ea8
Author: Sean Christopherson <[email protected]>

x86/vmware: Nullify save/restore hooks when using VMware's sched_clock

This commit sets the sched_clock save and restore hooks to null when
using the VMware sched_clock implementation. It aims to bypass these
hooks during suspend and resume operations for VMware guests.

> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index b88d9ca01202d..b5cb66ca022b8 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c

[ ... ]

> @@ -347,8 +347,11 @@ static void __init vmware_paravirt_ops_setup(void)
>  
>       vmware_cyc2ns_setup();
>  
> -     if (vmw_sched_clock)
> +     if (vmw_sched_clock) {
>               paravirt_set_sched_clock(vmware_sched_clock);
> +             x86_platform.save_sched_clock_state = NULL;
> +             x86_platform.restore_sched_clock_state = NULL;

Will this cause a kernel panic during suspend and resume operations?

In arch/x86/power/cpu.c, both save_processor_state() and
__restore_processor_state() unconditionally call these function pointers
without checking if they are populated:

arch/x86/power/cpu.c:save_processor_state() {
    ...
    x86_platform.save_sched_clock_state();
    ...
}

Since VMware guests support standard x86 ACPI S3 and hibernate pathways,
executing a null function pointer here will crash the kernel during suspend
or hibernate.

Could these be assigned to an empty dummy function like x86_init_noop
instead of null to safely make them a no-op?

> +     }
>  
>       if (vmware_is_stealclock_available()) {
>               has_steal_clock = true;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=16

Reply via email to