[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.8 v2 3/3] pc: fix FW_CFG_NB_CPUS to accoun
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH for-2.8 v2 3/3] pc: fix FW_CFG_NB_CPUS to account for -device added CPUs |
Date: |
Wed, 16 Nov 2016 14:03:10 +0100 |
On Wed, 16 Nov 2016 10:39:33 -0200
Eduardo Habkost <address@hidden> wrote:
> On Wed, Nov 16, 2016 at 01:24:11PM +0100, Igor Mammedov wrote:
> > On Tue, 15 Nov 2016 15:34:45 -0200
> > Eduardo Habkost <address@hidden> 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.
Ah yes,
I've forgotten that boot_cpus_le has been directly accessible by fwcfg
>
>
> > Thanks for noticing it!
> > I'll post v3 as reply to this thread.
>
> Thanks!
>