Re: [bug report] KVM: x86: Unify TSC logic (sleeping in atomic?)

[Sean Christopherson <seanjc@google.com>]

+sched folks

On Fri, Jan 17, 2025, Dan Carpenter wrote:
> I don't know why I'm seeing this static checker warning only now.  All
> this code looks to be about 15 years old...
> 
> Commit e48672fa25e8 ("KVM: x86: Unify TSC logic") from Aug 19, 2010
> (linux-next), leads to the following Smatch static checker warning:

That's not the problematic commit.  This popped because commit 8722903cbb8f
("sched: Define sched_clock_irqtime as static key") in the tip tree turned
sched_clock_irqtime into a static key (it was a simple "int").

https://lore.kernel.org/all/20250103022409.2544-2-laoar.shao@gmail.com

> 	arch/x86/kernel/tsc.c:1214 mark_tsc_unstable()
> 	warn: sleeping in atomic context
> 
> The code path is:
> 
> vcpu_load() <- disables preempt
> -> kvm_arch_vcpu_load()
>    -> mark_tsc_unstable() <- sleeps
> 
> virt/kvm/kvm_main.c
>    166  void vcpu_load(struct kvm_vcpu *vcpu)
>    167  {
>    168          int cpu = get_cpu();
>                           ^^^^^^^^^^
> This get_cpu() disables preemption.
> 
>    169  
>    170          __this_cpu_write(kvm_running_vcpu, vcpu);
>    171          preempt_notifier_register(&vcpu->preempt_notifier);
>    172          kvm_arch_vcpu_load(vcpu, cpu);
>    173          put_cpu();
>    174  }
> 
> arch/x86/kvm/x86.c
>   4979          if (unlikely(vcpu->cpu != cpu) || kvm_check_tsc_unstable()) {
>   4980                  s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
>   4981                                  rdtsc() - vcpu->arch.last_host_tsc;
>   4982                  if (tsc_delta < 0)
>   4983                          mark_tsc_unstable("KVM discovered backwards TSC");
>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> It seems pretty unlikely that we'll get a backwards tsc.

TSC will go "backwards" if the host is suspended, in which case host TSC gets
reset to 0.

>     1215         pr_info("Marking TSC unstable due to %s\n", reason);
>     1216 
>     1217         clocksource_mark_unstable(&clocksource_tsc_early);
>     1218         clocksource_mark_unstable(&clocksource_tsc);
>     1219 }
> 
> kernel/jump_label.c
>    245  void static_key_disable(struct static_key *key)
>    246  {
>    247          cpus_read_lock();
>                 ^^^^^^^^^^^^^^^^
> This lock has a might_sleep() in it which triggers the static checker
> warning.
> 
>    248          static_key_disable_cpuslocked(key);
>    249          cpus_read_unlock();
>    250  }
> 
> regards,
> dan carpenter