[Qemu-devel] [PULL 0/3] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Qemu-devel] [PULL 0/3] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Eduardo Habkost]

Unfortunately not in time for -rc0, but we still want to remove
"etc/boot-cpus" before 2.8.0 is released.

The following changes since commit b0bcc86d2a87456f5a276f941dc775b265b309cf:

  Update version for v2.8.0-rc0 release (2016-11-15 20:55:12 +0000)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-pull-request

for you to fetch changes up to e3cadac073a99489df1627be56c3f487f5cb9e31:

  pc: fix FW_CFG_NB_CPUS to account for -device added CPUs (2016-11-16 12:10:00 -0200)

----------------------------------------------------------------
pc: remove redundant fw_cfg file "etc/boot-cpus"

----------------------------------------------------------------

Igor Mammedov (3):
  Revert "pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than
    255 CPUs"
  fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
  pc: fix FW_CFG_NB_CPUS to account for -device added CPUs

 hw/arm/virt.c         |  4 +++-
 hw/i386/pc.c          | 26 ++++++++++++--------------
 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 +
 include/hw/i386/pc.h  |  4 ++--
 8 files changed, 21 insertions(+), 18 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 1/3] Revert "pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs"

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

This reverts commit 080ac219cc7d9c55adf925c3545b7450055ad625.

Legacy FW_CFG_NB_CPUS will be reused instead of 'etc/boot-cpus'
fw_cfg file since it does the same and there is no point
to maintaing duplicate guest ABI, if it can be helped.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1479212236-183810-2-git-send-email-imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         | 44 +++++++++++++++-----------------------------
 include/hw/i386/pc.h |  2 --
 2 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a9b1950..c227ead 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1086,6 +1086,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
+static int pc_present_cpus_count(PCMachineState *pcms)
+{
+    int i, boot_cpus = 0;
+    for (i = 0; i < pcms->possible_cpus->len; i++) {
+        if (pcms->possible_cpus->cpus[i].cpu) {
+            boot_cpus++;
+        }
+    }
+    return boot_cpus;
+}
+
 static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id,
                           Error **errp)
 {
@@ -1222,19 +1233,6 @@ static void pc_build_feature_control_file(PCMachineState *pcms)
     fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
 }
 
-static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
-{
-    if (cpus_count > 0xff) {
-        /* If the number of CPUs can't be represented in 8 bits, the
-         * BIOS must use "etc/boot-cpus". Set RTC field to 0 just
-         * to make old BIOSes fail more predictably.
-         */
-        rtc_set_memory(rtc, 0x5f, 0);
-    } else {
-        rtc_set_memory(rtc, 0x5f, cpus_count - 1);
-    }
-}
-
 static
 void pc_machine_done(Notifier *notifier, void *data)
 {
@@ -1243,7 +1241,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_memory(pcms->rtc, 0x5f, pc_present_cpus_count(pcms) - 1);
 
     if (bus) {
         int extra_hosts = 0;
@@ -1264,15 +1262,8 @@ 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));
-        }
     }
 
     if (pcms->apic_id_limit > 255) {
@@ -1819,11 +1810,9 @@ 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);
     if (dev->hotplugged) {
-        /* Update the number of CPUs in CMOS */
-        rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
+        /* increment the number of CPUs */
+        rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) + 1);
     }
 
     found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
@@ -1877,10 +1866,7 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
     found_cpu->cpu = NULL;
     object_unparent(OBJECT(dev));
 
-    /* decrement the number of CPUs */
-    pcms->boot_cpus_le = cpu_to_le16(le16_to_cpu(pcms->boot_cpus_le) - 1);
-    /* Update the number of CPUs in CMOS */
-    rtc_set_cpus_count(pcms->rtc, le16_to_cpu(pcms->boot_cpus_le));
+    rtc_set_memory(pcms->rtc, 0x5f, rtc_get_memory(pcms->rtc, 0x5f) - 1);
  out:
     error_propagate(errp, local_err);
 }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8eb517f..e32e957 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -36,7 +36,6 @@
 /**
  * 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
  */
 struct PCMachineState {
     /*< private >*/
@@ -71,7 +70,6 @@ struct PCMachineState {
     bool apic_xrupt_override;
     unsigned apic_id_limit;
     CPUArchIdList *possible_cpus;
-    uint16_t boot_cpus_le;
 
     /* NUMA information: */
     uint64_t numa_nodes;
-- 
2.7.4

[Qemu-devel] [PULL 2/3] fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

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>
Message-Id: <1479212236-183810-3-git-send-email-imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@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 c227ead..e8757b4 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:
      *
@@ -1341,6 +1342,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] [PULL 3/3] pc: fix FW_CFG_NB_CPUS to account for -device added CPUs

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1479301481-197333-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c         | 44 +++++++++++++++++++++++++++-----------------
 include/hw/i386/pc.h |  2 ++
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e8757b4..a9e64a8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -744,7 +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_add_i16(fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
 
     /* FW_CFG_MAX_CPUS is a bit confusing/problematic on x86:
      *
@@ -1087,17 +1087,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
     }
 }
 
-static int pc_present_cpus_count(PCMachineState *pcms)
-{
-    int i, boot_cpus = 0;
-    for (i = 0; i < pcms->possible_cpus->len; i++) {
-        if (pcms->possible_cpus->cpus[i].cpu) {
-            boot_cpus++;
-        }
-    }
-    return boot_cpus;
-}
-
 static X86CPU *pc_new_cpu(const char *typename, int64_t apic_id,
                           Error **errp)
 {
@@ -1234,6 +1223,19 @@ static void pc_build_feature_control_file(PCMachineState *pcms)
     fw_cfg_add_file(pcms->fw_cfg, "etc/msr_feature_control", val, sizeof(*val));
 }
 
+static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
+{
+    if (cpus_count > 0xff) {
+        /* If the number of CPUs can't be represented in 8 bits, the
+         * BIOS must use "FW_CFG_NB_CPUS". Set RTC field to 0 just
+         * to make old BIOSes fail more predictably.
+         */
+        rtc_set_memory(rtc, 0x5f, 0);
+    } else {
+        rtc_set_memory(rtc, 0x5f, cpus_count - 1);
+    }
+}
+
 static
 void pc_machine_done(Notifier *notifier, void *data)
 {
@@ -1242,7 +1244,7 @@ void pc_machine_done(Notifier *notifier, void *data)
     PCIBus *bus = pcms->bus;
 
     /* set the number of CPUs */
-    rtc_set_memory(pcms->rtc, 0x5f, pc_present_cpus_count(pcms) - 1);
+    rtc_set_cpus_count(pcms->rtc, pcms->boot_cpus);
 
     if (bus) {
         int extra_hosts = 0;
@@ -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);
@@ -1812,9 +1816,11 @@ 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);
+        fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
     }
 
     found_cpu = pc_find_cpu_slot(pcms, CPU(dev), NULL);
@@ -1868,7 +1874,11 @@ 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);
+    fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
  out:
     error_propagate(errp, local_err);
 }
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index e32e957..67a1a9e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -36,6 +36,7 @@
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
+ * @boot_cpus: number of present VCPUs
  */
 struct PCMachineState {
     /*< private >*/
@@ -70,6 +71,7 @@ struct PCMachineState {
     bool apic_xrupt_override;
     unsigned apic_id_limit;
     CPUArchIdList *possible_cpus;
+    uint16_t boot_cpus;
 
     /* NUMA information: */
     uint64_t numa_nodes;
-- 
2.7.4

Re: [Qemu-devel] [PULL 0/3] pc: remove redundant fw_cfg file "etc/boot-cpus"

[Stefan Hajnoczi]

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

On Wed, Nov 16, 2016 at 12:17:33PM -0200, Eduardo Habkost wrote:
> Unfortunately not in time for -rc0, but we still want to remove
> "etc/boot-cpus" before 2.8.0 is released.
> 
> The following changes since commit b0bcc86d2a87456f5a276f941dc775b265b309cf:
> 
>   Update version for v2.8.0-rc0 release (2016-11-15 20:55:12 +0000)
> 
> are available in the git repository at:
> 
>   git://github.com/ehabkost/qemu.git tags/machine-pull-request
> 
> for you to fetch changes up to e3cadac073a99489df1627be56c3f487f5cb9e31:
> 
>   pc: fix FW_CFG_NB_CPUS to account for -device added CPUs (2016-11-16 12:10:00 -0200)
> 
> ----------------------------------------------------------------
> pc: remove redundant fw_cfg file "etc/boot-cpus"
> 
> ----------------------------------------------------------------
> 
> Igor Mammedov (3):
>   Revert "pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than
>     255 CPUs"
>   fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
>   pc: fix FW_CFG_NB_CPUS to account for -device added CPUs
> 
>  hw/arm/virt.c         |  4 +++-
>  hw/i386/pc.c          | 26 ++++++++++++--------------
>  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 +
>  include/hw/i386/pc.h  |  4 ++--
>  8 files changed, 21 insertions(+), 18 deletions(-)
> 
> -- 
> 2.7.4
> 
> 

Thanks, applied to my staging tree:
https://github.com/stefanha/qemu/commits/staging

Stefan

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