lkml.org 
[lkml]   [2022]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectRe: [PATCH 38/44] KVM: Disable CPU hotplug during hardware enabling
Date
On Thu, 2022-11-10 at 01:33 +0000, Huang, Kai wrote:
> > @@ -9283,7 +9283,13 @@ static int
> > kvm_x86_check_processor_compatibility(struct kvm_x86_init_ops *ops)
> >   int cpu = smp_processor_id();
> >   struct cpuinfo_x86 *c = &cpu_data(cpu);
> >  
> > - WARN_ON(!irqs_disabled());
> > + /*
> > + * Compatibility checks are done when loading KVM and when enabling
> > + * hardware, e.g. during CPU hotplug, to ensure all online CPUs are
> > + * compatible, i.e. KVM should never perform a compatibility check
> > on
> > + * an offline CPU.
> > + */
> > + WARN_ON(!irqs_disabled() && cpu_active(cpu));
> >  
>
> Also, the logic of:
>
> !irqs_disabled() && cpu_active(cpu)
>
> is quite weird.
>
> The original "WARN(!irqs_disabled())" is reasonable because in STARTING
> section
> the IRQ is indeed disabled.
>
> But this doesn't make sense anymore after we move to ONLINE section, in which
> IRQ has already been enabled (see start_secondary()).  IIUC the WARN_ON()
> doesn't get exploded is purely because there's an additional cpu_active(cpu)
> check.
>
> So, a more reasonable check should be something like:
>
> WARN_ON(irqs_disabled() || cpu_active(cpu) || !cpu_online(cpu));
>
> Or we can simply do:
>
> WARN_ON(!cpu_online(cpu) || cpu_active(cpu));
>
> (because I don't know whether it's possible IRQ can somehow get disabled in
> ONLINE section).
>
> Btw above is purely based on code analysis, but I haven't done any test.

Hmm.. I wasn't thinking thoroughly. I forgot CPU compatibility check also
happens on all online cpus when loading KVM. For this case, IRQ is disabled and
cpu_active() is true. For the hotplug case, IRQ is enabled but cpu_active() is
false.

So WARN_ON(!irqs_disabled() && cpu_active(cpu)) looks reasonable. Sorry for the
noise. Just needed some time to connect the comment with the code.
\
 
 \ /
  Last update: 2022-11-10 03:13    [W:0.458 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site