[Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Igor Mammedov]

Commit 080ac219cc7d9c55adf925c3545b7450055ad625
   pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs

added "etc/boot-cpus" fw_cfg file durung 2.8 merge window, however
QEMU alredy had similar legacy FW_CFG_NB_CPUS fw_cfg entry that
should do practically the same. Considering FW_CFG_NB_CPUS's been
around for a long time and is used by external projects (firmwares)
we can't replace it with 'etc/boot-cpus' fw_cfg file.

Drop redundant 'etc/boot-cpus' fw_cfg file  and reuse FW_CFG_NB_CPUS
instead.

So here goes QEMU part of fixup

CC: Eduardo Habkost <ehabkost@redhat.com>
CC: mst@redhat.com
CC: Stefan Hajnoczi <stefanha@gmail.com>
CC: "Kevin O'Connor" <kevin@koconnor.net>
CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Laszlo Ersek <lersek@redhat.com>

Igor Mammedov (2):
  fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
  pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead

 include/hw/i386/pc.h  |  4 ++--
 hw/arm/virt.c         |  4 +++-
 hw/i386/pc.c          | 20 ++++++++------------
 hw/nvram/fw_cfg.c     |  1 -
 hw/ppc/mac_newworld.c |  1 +
 hw/ppc/mac_oldworld.c |  1 +
 hw/sparc/sun4m.c      |  1 +
 hw/sparc64/sun4u.c    |  1 +
 8 files changed, 17 insertions(+), 16 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH for-2.8 1/2] fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()

[Igor Mammedov]

PC will use this field in other way, so move it outside the common
code so PC could set a different value, i.e. all CPUs
regardless of where they are coming from (-smp X | -device cpu...).

It's quick and dirty hack as it could be implemented in more generic
way in MashineClass. But do it in simple way since only PC is affected
so far.

Later we can generalize it when another affected target gets support
for -device cpu.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/arm/virt.c         | 4 +++-
 hw/i386/pc.c          | 2 ++
 hw/nvram/fw_cfg.c     | 1 -
 hw/ppc/mac_newworld.c | 1 +
 hw/ppc/mac_oldworld.c | 1 +
 hw/sparc/sun4m.c      | 1 +
 hw/sparc64/sun4u.c    | 1 +
 7 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 54a8b28..d04e4ac 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -929,9 +929,11 @@ static void create_fw_cfg(const VirtBoardInfo *vbi, AddressSpace *as)
 {
     hwaddr base = vbi->memmap[VIRT_FW_CFG].base;
     hwaddr size = vbi->memmap[VIRT_FW_CFG].size;
+    FWCfgState *fw_cfg;
     char *nodename;
 
-    fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
+    fw_cfg = fw_cfg_init_mem_wide(base + 8, base, 8, base + 16, as);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
 
     nodename = g_strdup_printf("/fw-cfg@%" PRIx64, base);
     qemu_fdt_add_subnode(vbi->fdt, nodename);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2c37a78..93016d0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -744,6 +744,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
     int i, j;
 
     fw_cfg = fw_cfg_init_io_dma(FW_CFG_IO_BASE, FW_CFG_IO_BASE + 4, as);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
 
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
      *
@@ -1350,6 +1351,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);
     rom_set_fw(fw_cfg);
 
     load_linux(pcms, fw_cfg);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 1f0c3e9..3ebecb2 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -884,7 +884,6 @@ static void fw_cfg_init1(DeviceState *dev)
     fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
     fw_cfg_add_bytes(s, FW_CFG_UUID, &qemu_uuid, 16);
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)!machine->enable_graphics);
-    fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
     fw_cfg_bootsplash(s);
     fw_cfg_reboot(s);
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7d25106..2bfdb64 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -466,6 +466,7 @@ static void ppc_core99_init(MachineState *machine)
     /* No PCI init: the BIOS will do it */
 
     fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, machine_arch);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 4479487..56282c5 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -319,6 +319,7 @@ static void ppc_heathrow_init(MachineState *machine)
     /* No PCI init: the BIOS will do it */
 
     fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, ARCH_HEATHROW);
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 6224288..f5b6efd 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -1033,6 +1033,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
                  hwdef->ecc_version);
 
     fw_cfg = fw_cfg_init_mem(CFG_ADDR, CFG_ADDR + 2);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 271d8bc..4663315 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -855,6 +855,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                            (uint8_t *)&nd_table[0].macaddr);
 
     fw_cfg = fw_cfg_init_io(BIOS_CFG_IOPORT);
+    fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)max_cpus);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
     fw_cfg_add_i16(fw_cfg, FW_CFG_MACHINE_ID, hwdef->machine_id);
-- 
2.7.4

[Qemu-devel] [PATCH for-2.8 2/2] pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead

[Igor Mammedov]

Since "etc/boot-cpus" fw_cfg file and should have the same value
i.e. NR of of all present at boot CPUs (-smp X | -device cpu ...)

Reuse legacy FW_CFG_NB_CPUS since it can't be removed
and drop "etc/boot-cpus" fw_cfg file that's been added in this
release cycle and hasn't been shipped with released QEMU so far.

Follow up patch for SeaBIOS will have to go in stable branch
and updated SeaBIOS should be pulled in QEMU-2.8 release.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/i386/pc.h |  4 ++--
 hw/i386/pc.c         | 18 ++++++------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8eb517f..d88a9c2 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -36,7 +36,7 @@
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
- * @boot_cpus_le: number of present VCPUs, referenced by 'etc/boot-cpus' fw_cfg
+ * @boot_cpus: number of present VCPUs, value goes to fw_cfg FW_CFG_NB_CPUS
  */
 struct PCMachineState {
     /*< private >*/
@@ -71,7 +71,7 @@ struct PCMachineState {
     bool apic_xrupt_override;
     unsigned apic_id_limit;
     CPUArchIdList *possible_cpus;
-    uint16_t boot_cpus_le;
+    uint16_t boot_cpus;
 
     /* NUMA information: */
     uint64_t numa_nodes;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 93016d0..c80f7a9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1244,7 +1244,7 @@ void pc_machine_done(Notifier *notifier, void *data)
     PCIBus *bus = pcms->bus;
 
     /* set the number of CPUs */
-    rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
+    rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
 
     if (bus) {
         int extra_hosts = 0;
@@ -1265,15 +1265,9 @@ void pc_machine_done(Notifier *notifier, void *data)
 
     acpi_setup();
     if (pcms->fw_cfg) {
-        MachineClass *mc = MACHINE_GET_CLASS(pcms);
-
         pc_build_smbios(pcms->fw_cfg);
         pc_build_feature_control_file(pcms);
-
-        if (mc->max_cpus > 255) {
-            fw_cfg_add_file(pcms->fw_cfg, "etc/boot-cpus", &pcms->boot_cpus_le,
-                            sizeof(pcms->boot_cpus_le));
-        }
+        fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
     }
 
     if (pcms->apic_id_limit > 255) {
@@ -1834,10 +1828,10 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
     }
 
     /* increment the number of CPUs */
-    pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) + 1);
+    pcms->boot_cpus++;
     if (dev->hotplugged) {
         /* Update the number of CPUs in CMOS */
-        rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
+        rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
     }
 
     found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
@@ -1892,9 +1886,9 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
     object_unparent(OBJECT(dev));
 
     /* decrement the number of CPUs */
-    pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) - 1);
+    pcms->boot_cpus--;
     /* Update the number of CPUs in CMOS */
-    rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
+    rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
  out:
     error_propagate(errp, local_err);
 }
-- 
2.7.4

Re: [Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On Fri, Nov 11, 2016 at 04:21:10PM +0100, Igor Mammedov wrote:
> 
> Commit 080ac219cc7d9c55adf925c3545b7450055ad625
>    pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
> 
> added "etc/boot-cpus" fw_cfg file durung 2.8 merge window, however
> QEMU alredy had similar legacy FW_CFG_NB_CPUS fw_cfg entry that
> should do practically the same. Considering FW_CFG_NB_CPUS's been
> around for a long time and is used by external projects (firmwares)
> we can't replace it with 'etc/boot-cpus' fw_cfg file.
> 
> Drop redundant 'etc/boot-cpus' fw_cfg file  and reuse FW_CFG_NB_CPUS
> instead.
> 
> So here goes QEMU part of fixup
> 
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: mst@redhat.com
> CC: Stefan Hajnoczi <stefanha@gmail.com>
> CC: "Kevin O'Connor" <kevin@koconnor.net>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Laszlo Ersek <lersek@redhat.com>
> 
> Igor Mammedov (2):
>   fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
>   pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead
> 
>  include/hw/i386/pc.h  |  4 ++--
>  hw/arm/virt.c         |  4 +++-
>  hw/i386/pc.c          | 20 ++++++++------------
>  hw/nvram/fw_cfg.c     |  1 -
>  hw/ppc/mac_newworld.c |  1 +
>  hw/ppc/mac_oldworld.c |  1 +
>  hw/sparc/sun4m.c      |  1 +
>  hw/sparc64/sun4u.c    |  1 +
>  8 files changed, 17 insertions(+), 16 deletions(-)

Thanks, this should go through Eduardo's tree.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Michael S. Tsirkin]

On Fri, Nov 11, 2016 at 04:21:10PM +0100, Igor Mammedov wrote:
> 
> Commit 080ac219cc7d9c55adf925c3545b7450055ad625
>    pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
> 
> added "etc/boot-cpus" fw_cfg file durung 2.8 merge window, however
> QEMU alredy had similar legacy FW_CFG_NB_CPUS fw_cfg entry that
> should do practically the same. Considering FW_CFG_NB_CPUS's been
> around for a long time and is used by external projects (firmwares)
> we can't replace it with 'etc/boot-cpus' fw_cfg file.
> 
> Drop redundant 'etc/boot-cpus' fw_cfg file  and reuse FW_CFG_NB_CPUS
> instead.
> 
> So here goes QEMU part of fixup

I agree we shouldn't commit to a bad host/guest API
but I think we need to format it differently.
First revert the boot-cpus patch for 2.8.
On top of that, add a patch fixing FW_CFG_NB_CPUS.


> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: mst@redhat.com
> CC: Stefan Hajnoczi <stefanha@gmail.com>
> CC: "Kevin O'Connor" <kevin@koconnor.net>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Laszlo Ersek <lersek@redhat.com>
> 
> Igor Mammedov (2):
>   fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
>   pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead
> 
>  include/hw/i386/pc.h  |  4 ++--
>  hw/arm/virt.c         |  4 +++-
>  hw/i386/pc.c          | 20 ++++++++------------
>  hw/nvram/fw_cfg.c     |  1 -
>  hw/ppc/mac_newworld.c |  1 +
>  hw/ppc/mac_oldworld.c |  1 +
>  hw/sparc/sun4m.c      |  1 +
>  hw/sparc64/sun4u.c    |  1 +
>  8 files changed, 17 insertions(+), 16 deletions(-)
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Gerd Hoffmann]

  Hi,

> I agree we shouldn't commit to a bad host/guest API
> but I think we need to format it differently.
> First revert the boot-cpus patch for 2.8.
> On top of that, add a patch fixing FW_CFG_NB_CPUS.

Agree, doing a plain revert of 080ac219cc7d9c55adf925c3545b7450055ad625
is better.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Igor Mammedov]

On Mon, 14 Nov 2016 22:46:14 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Nov 11, 2016 at 04:21:10PM +0100, Igor Mammedov wrote:
> > 
> > Commit 080ac219cc7d9c55adf925c3545b7450055ad625
> >    pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
> > 
> > added "etc/boot-cpus" fw_cfg file durung 2.8 merge window, however
> > QEMU alredy had similar legacy FW_CFG_NB_CPUS fw_cfg entry that
> > should do practically the same. Considering FW_CFG_NB_CPUS's been
> > around for a long time and is used by external projects (firmwares)
> > we can't replace it with 'etc/boot-cpus' fw_cfg file.
> > 
> > Drop redundant 'etc/boot-cpus' fw_cfg file  and reuse FW_CFG_NB_CPUS
> > instead.
> > 
> > So here goes QEMU part of fixup  
> 
> I agree we shouldn't commit to a bad host/guest API
> but I think we need to format it differently.
> First revert the boot-cpus patch for 2.8.
> On top of that, add a patch fixing FW_CFG_NB_CPUS.
Sure, I'll respin.

> 
> 
> > CC: Eduardo Habkost <ehabkost@redhat.com>
> > CC: mst@redhat.com
> > CC: Stefan Hajnoczi <stefanha@gmail.com>
> > CC: "Kevin O'Connor" <kevin@koconnor.net>
> > CC: Gerd Hoffmann <kraxel@redhat.com>
> > CC: Laszlo Ersek <lersek@redhat.com>
> > 
> > Igor Mammedov (2):
> >   fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
> >   pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead
> > 
> >  include/hw/i386/pc.h  |  4 ++--
> >  hw/arm/virt.c         |  4 +++-
> >  hw/i386/pc.c          | 20 ++++++++------------
> >  hw/nvram/fw_cfg.c     |  1 -
> >  hw/ppc/mac_newworld.c |  1 +
> >  hw/ppc/mac_oldworld.c |  1 +
> >  hw/sparc/sun4m.c      |  1 +
> >  hw/sparc64/sun4u.c    |  1 +
> >  8 files changed, 17 insertions(+), 16 deletions(-)
> > 
> > -- 
> > 2.7.4