On Wed, Nov 16, 2016 at 01:24:11PM +0100, Igor Mammedov wrote:
> On Tue, 15 Nov 2016 15:34:45 -0200
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > On Tue, Nov 15, 2016 at 01:17:16PM +0100, Igor Mammedov wrote:
> > [...]
> > > @@ -1265,6 +1267,8 @@ void pc_machine_done(Notifier *notifier, void *data)
> > > if (pcms->fw_cfg) {
> > > pc_build_smbios(pcms->fw_cfg);
> > > pc_build_feature_control_file(pcms);
> > > + /* update FW_CFG_NB_CPUS to account for -device added CPUs */
> > > + fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
> > > }
> > >
> > > if (pcms->apic_id_limit > 255) {
> > > @@ -1342,7 +1346,7 @@ void xen_load_linux(PCMachineState *pcms)
> > > assert(MACHINE(pcms)->kernel_filename != NULL);
> > >
> > > fw_cfg = fw_cfg_init_io(FW_CFG_IO_BASE);
> > > - fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
> > > + fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
> > > rom_set_fw(fw_cfg);
> > >
> > > load_linux(pcms, fw_cfg);
> > > @@ -1824,9 +1828,10 @@ static void pc_cpu_plug(HotplugHandler
> > > *hotplug_dev,
> > > }
> > > }
> > >
> > > + /* increment the number of CPUs */
> > > + pcms->boot_cpus++;
> > > if (dev->hotplugged) {
> > > - /* increment the number of CPUs */
> > > - rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f)
> > > + 1);
> > > + rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
> > > }
> > >
> > > found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
> > > @@ -1880,7 +1885,10 @@ static void pc_cpu_unplug_cb(HotplugHandler
> > > *hotplug_dev,
> > > found_cpu->cpu = NULL;
> > > object_unparent(OBJECT(dev));
> > >
> > > - rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) - 1);
> > > + /* decrement the number of CPUs */
> > > + pcms->boot_cpus--;
> > > + /* Update the number of CPUs in CMOS */
> > > + rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
> >
> > Don't we need to call fw_cfg_modify_i16() on hotplug/hot-unplug,
> > too?
> Indeed, it should be updated
> otherwise it will hang on reboot in BIOS waiting for wrong number of CPUs
> if CPUs count is above 256.
>
> the same bug has been present in the reverted
> "pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs"
The "etc/boot-cpus" patch changed boot_cpus_le on the plug/unplug
callbacks.
> Thanks for noticing it!
> I'll post v3 as reply to this thread.
Thanks!
--
Eduardo
