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 an unconditional NULL pointer dereference during suspend, hibernation, or kexec operations in Xen guests. --
commit 07d607146953d7a7f53a708d78545184418c340c Author: Sean Christopherson <[email protected]> Subject: x86/xen/time: Nullify x86_platform's sched_clock save/restore hooks This commit nullifies the x86_platform sched_clock save/restore hooks when setting up Xen's PV clock. It aims to clarify that these common hooks are not used when running as a Xen guest. > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c > index 3d3165eef8214..21d366d01985e 100644 > --- a/arch/x86/xen/time.c > +++ b/arch/x86/xen/time.c > @@ -568,6 +568,12 @@ static void __init xen_init_time_common(void) > xen_sched_clock_offset = xen_clocksource_read(); > static_call_update(pv_steal_clock, xen_steal_clock); > paravirt_set_sched_clock(xen_sched_clock); > + /* > + * Xen has paravirtualized suspend/resume and so doesn't use the common > + * x86 sched_clock save/restore hooks. > + */ > + x86_platform.save_sched_clock_state = NULL; > + x86_platform.restore_sched_clock_state = NULL; Does setting these hooks to NULL lead to an unconditional pointer dereference regression during suspend, hibernation, or kexec operations in Xen guests? Looking at save_processor_state() and __restore_processor_state() in arch/x86/power/cpu.c, the hooks are called directly without being checked for NULL first: arch/x86/power/cpu.c:save_processor_state() { __save_processor_state(&saved_context); x86_platform.save_sched_clock_state(); } arch/x86/power/cpu.c:__restore_processor_state() { ... do_fpu_end(); tsc_verify_tsc_adjust(true); x86_platform.restore_sched_clock_state(); ... } Will operations that trigger a generic CPU state save or restore on a Xen guest execute a call on these NULL pointers, causing a kernel panic regression? Should these instead be assigned to an empty stub function to safely bypass them? > > tsc_register_calibration_routines(xen_tsc_khz, NULL); > x86_platform.get_wallclock = xen_get_wallclock; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
