On Wed, Jul 13, 2016 at 09:56:25AM +0200, Igor Mammedov wrote:
> On Tue, 12 Jul 2016 14:18:22 -0300
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > On Tue, Jul 12, 2016 at 02:48:43PM +0200, Igor Mammedov wrote:
> > > On Tue, 12 Jul 2016 00:29:08 -0300
> > > Eduardo Habkost <ehabk...@redhat.com> wrote:
> > >
> > > > On Wed, Jul 06, 2016 at 08:20:45AM +0200, Igor Mammedov wrote:
> > > > > currently present CPUs counter in CMOS only contains
> > > > > smp_cpus (i.e. initial CPUs specified with -smp X) and
> > > > > doesn't account for CPUs created with -device.
> > > > > If VM is started with additional CPUs added with
> > > > > -device, it will hang in BIOS waiting for condition
> > > > > smp_cpus == counted_cpus
> > > > > forever as counted_cpus will include -device CPUs as well
> > > > > and be more than smp_cpus.
> > > > >
> > > > > make present CPUs counter in CMOS to count all CPUs
> > > > > (initial and coldplugged with -device) by delaying
> > > > > it to machine done time when it possible to count
> > > > > CPUs added with -device.
> > > >
> > > > Do you plan to fix the remaining code using smp_cpus? e.g.:
> > > perhaps after Drew's -smp refactoring or maybe during it,
> > > it looks like good candidate for that series,
> > > I'll work with Drew on that.
> > >
> > > >
> > > > 1) x86_cpu_realizefn():
> > > > if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || smp_cpus > 1) {
> > > > x86_cpu_apic_create(cpu, &local_err);
> > > > [...]
> > > > (Maybe we should simply make realizefn fail if we try to create a
> > > > second CPU
> > > > using -device or device_add without CPUID_APIC)
> > > wouldn't that break some setups that doing it but still able
> > > to boot?
> >
> > It would, that's why we need to do it only in the case of -device
> > or device_add. What about something like:
> > if (!(cpu->env.features[FEAT_1_EDX] & CPUID_APIC) && smp_cpus < 2
> > &&total_cpus_already_created() > 0) {
> > error_setg("we can't create a new VCPU without an APIC");
> > return;
> -device/device_add is orthogonal here, cpu-add is also affected.
>
> I'd woul make it a warning instead of hard error:
> "broken configuration, only 1st CPU will work when creating multiple CPUs
> with APIC disabled,
> to fix it remove -apic/apic=off from -cpu option"
I forgot about cpu-add. You're right.
>
> > }
> >
> > I believe this logic should be moved to PC code, eventually. Or
> > at least the process should be controlled by PC code (by setting
> > a force-apic-creation property in X86CPU, for example).
> smp_cpus check should definitely be part of PC code.
>
> PS:
> You do not consider above as part of this series, do you?
No, it's just something to be done later.
--
Eduardo
