[Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Like Xu]

This patch series make existing cores/threads/sockets into machine
properties and get rid of global variables they use currently.

Like Xu (9):
  cpu/topology: add struct CpuTopology to MachineState
  cpu/topology: add general support for machine properties
  cpu/topology: add uncommon arch support for smp machine properties
  cpu/topology: add ARM support for smp machine properties
  cpu/topology: add i386 support for smp machine properties
  cpu/topology: add PPC support for smp machine properties
  cpu/topology: add riscv support for smp machine properties
  cpu/topology: add s390x support for smp machine properties
  cpu/topology: replace smp global variables with machine propertie

 accel/kvm/kvm-all.c          |  3 +++
 backends/hostmem.c           |  4 ++++
 cpus.c                       |  4 ++++
 exec.c                       |  2 ++
 gdbstub.c                    |  7 ++++++-
 hw/alpha/dp264.c             |  1 +
 hw/arm/fsl-imx6.c            |  5 +++++
 hw/arm/fsl-imx6ul.c          |  5 +++++
 hw/arm/fsl-imx7.c            |  5 +++++
 hw/arm/highbank.c            |  1 +
 hw/arm/mcimx6ul-evk.c        |  1 +
 hw/arm/mcimx7d-sabre.c       |  3 +++
 hw/arm/raspi.c               |  2 ++
 hw/arm/realview.c            |  1 +
 hw/arm/sabrelite.c           |  1 +
 hw/arm/vexpress.c            |  3 +++
 hw/arm/virt.c                |  7 +++++++
 hw/arm/xlnx-zynqmp.c         |  7 +++++++
 hw/cpu/core.c                |  3 +++
 hw/hppa/machine.c            |  4 ++++
 hw/i386/acpi-build.c         |  3 +++
 hw/i386/kvmvapic.c           |  5 +++++
 hw/i386/pc.c                 | 12 +++++++++++
 hw/mips/boston.c             |  1 +
 hw/mips/mips_malta.c         |  9 +++++++++
 hw/openrisc/openrisc_sim.c   |  1 +
 hw/ppc/e500.c                |  3 +++
 hw/ppc/mac_newworld.c        |  2 ++
 hw/ppc/mac_oldworld.c        |  2 ++
 hw/ppc/pnv.c                 |  3 +++
 hw/ppc/prep.c                |  2 ++
 hw/ppc/spapr.c               | 29 ++++++++++++++++++++++++++
 hw/ppc/spapr_rtas.c          |  3 +++
 hw/riscv/sifive_e.c          |  4 ++++
 hw/riscv/sifive_plic.c       |  3 +++
 hw/riscv/sifive_u.c          |  4 ++++
 hw/riscv/spike.c             |  2 ++
 hw/riscv/virt.c              |  1 +
 hw/s390x/s390-virtio-ccw.c   |  2 ++
 hw/s390x/sclp.c              |  1 +
 hw/smbios/smbios.c           | 11 ++++++++++
 hw/sparc/sun4m.c             |  2 ++
 hw/sparc64/sun4u.c           |  2 ++
 hw/xtensa/sim.c              |  1 +
 hw/xtensa/xtfpga.c           |  1 +
 include/hw/arm/virt.h        |  2 +-
 include/hw/boards.h          |  8 ++++++++
 include/sysemu/sysemu.h      |  2 +-
 migration/postcopy-ram.c     |  7 +++++++
 numa.c                       |  1 +
 target/arm/cpu.c             |  7 +++++++
 target/i386/cpu.c            |  4 ++++
 target/openrisc/sys_helper.c |  5 +++++
 target/s390x/cpu.c           |  3 +++
 target/s390x/excp_helper.c   |  6 ++++++
 tcg/tcg.c                    | 15 ++++++++++++++
 vl.c                         | 48 ++++++++++++++++++++++++--------------------
 57 files changed, 261 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 1/9] cpu/topology: add struct CpuTopology to MachineState

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 include/hw/arm/virt.h   | 2 +-
 include/hw/boards.h     | 8 ++++++++
 include/sysemu/sysemu.h | 2 +-
 vl.c                    | 7 ++++++-
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 507517c..724da0c 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -123,7 +123,7 @@ typedef struct {
     struct arm_boot_info bootinfo;
     MemMapEntry *memmap;
     const int *irqmap;
-    int smp_cpus;
+    unsigned int smp_cpus;
     void *fdt;
     int fdt_size;
     uint32_t clock_phandle;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index e231860..cbde276 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -231,6 +231,13 @@ typedef struct DeviceMemoryState {
     MemoryRegion mr;
 } DeviceMemoryState;
 
+typedef struct CpuTopology {
+    unsigned int smp_cpus;
+    unsigned int smp_cores;
+    unsigned int smp_threads;
+    unsigned int max_cpus;
+} CpuTopology;
+
 /**
  * MachineState:
  */
@@ -273,6 +280,7 @@ struct MachineState {
     const char *cpu_type;
     AccelState *accelerator;
     CPUArchIdList *possible_cpus;
+    CpuTopology topo;
     struct NVDIMMState *nvdimms_state;
 };
 
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 6065d9e..c0d7d7c 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -100,7 +100,7 @@ extern const char *keyboard_layout;
 extern int win2k_install_hack;
 extern int alt_grab;
 extern int ctrl_grab;
-extern int smp_cpus;
+extern unsigned int smp_cpus;
 extern unsigned int max_cpus;
 extern int cursor_hide;
 extern int graphic_rotate;
diff --git a/vl.c b/vl.c
index d61d560..9089253 100644
--- a/vl.c
+++ b/vl.c
@@ -162,7 +162,7 @@ static Chardev **serial_hds;
 Chardev *parallel_hds[MAX_PARALLEL_PORTS];
 int win2k_install_hack = 0;
 int singlestep = 0;
-int smp_cpus;
+unsigned int smp_cpus;
 unsigned int max_cpus;
 int smp_cores = 1;
 int smp_threads = 1;
@@ -4116,6 +4116,11 @@ int main(int argc, char **argv, char **envp)
 
     smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
+    current_machine->topo.smp_cpus = smp_cpus;
+    current_machine->topo.max_cpus = max_cpus;
+    current_machine->topo.smp_cores = smp_cores;
+    current_machine->topo.smp_threads = smp_threads;
+
     /* sanity-check smp_cpus and max_cpus against machine_class */
     if (smp_cpus < machine_class->min_cpus) {
         error_report("Invalid SMP CPUs %d. The min CPUs "
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 accel/kvm/kvm-all.c      |  3 +++
 backends/hostmem.c       |  4 ++++
 cpus.c                   |  4 ++++
 exec.c                   |  2 ++
 gdbstub.c                |  7 ++++++-
 hw/cpu/core.c            |  3 +++
 hw/smbios/smbios.c       | 11 +++++++++++
 migration/postcopy-ram.c |  7 +++++++
 numa.c                   |  1 +
 tcg/tcg.c                | 15 +++++++++++++++
 10 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 241db49..5385218 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1526,6 +1526,9 @@ bool kvm_vcpu_id_is_valid(int vcpu_id)
 static int kvm_init(MachineState *ms)
 {
     MachineClass *mc = MACHINE_GET_CLASS(ms);
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+    unsigned int max_cpus = ms->topo.max_cpus;
+
     static const char upgrade_note[] =
         "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
         "(see http://sourceforge.net/projects/kvm).\n";
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 04baf47..cecdfd5 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -222,6 +222,8 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
 {
     Error *local_err = NULL;
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     if (backend->force_prealloc) {
         if (value) {
@@ -311,6 +313,8 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 {
     HostMemoryBackend *backend = MEMORY_BACKEND(uc);
     HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     Error *local_err = NULL;
     void *ptr;
     uint64_t sz;
diff --git a/cpus.c b/cpus.c
index e83f72b..834a697 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2067,6 +2067,10 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
 
 void qemu_init_vcpu(CPUState *cpu)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cores = ms->topo.smp_cores;
+    unsigned int smp_threads = ms->topo.smp_threads;
+
     cpu->nr_cores = smp_cores;
     cpu->nr_threads = smp_threads;
     cpu->stopped = true;
diff --git a/exec.c b/exec.c
index 86a38d3..a3c3db7 100644
--- a/exec.c
+++ b/exec.c
@@ -1829,6 +1829,8 @@ static void *file_ram_alloc(RAMBlock *block,
                             bool truncate,
                             Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     void *area;
 
     block->page_size = qemu_fd_getpagesize(fd);
diff --git a/gdbstub.c b/gdbstub.c
index d54abd1..35f6bbc 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -30,6 +30,7 @@
 #include "sysemu/sysemu.h"
 #include "exec/gdbstub.h"
 #include "hw/cpu/cluster.h"
+#include "hw/boards.h"
 #endif
 
 #define MAX_PACKET_LENGTH 4096
@@ -1154,11 +1155,15 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
     CPUState *cpu;
     GDBThreadIdKind kind;
 #ifdef CONFIG_USER_ONLY
-    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
+    /* global variable max_cpus exists only in system mode */
+    unsigned int max_cpus = 1;
 
     CPU_FOREACH(cpu) {
         max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
     }
+#else
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
 #endif
     /* uninitialised CPUs stay 0 */
     newstates = g_new0(char, max_cpus);
diff --git a/hw/cpu/core.c b/hw/cpu/core.c
index 7e42e2c..b75ffbb 100644
--- a/hw/cpu/core.c
+++ b/hw/cpu/core.c
@@ -11,6 +11,7 @@
 #include "qapi/visitor.h"
 #include "qapi/error.h"
 #include "sysemu/cpus.h"
+#include "hw/boards.h"
 
 static void core_prop_get_core_id(Object *obj, Visitor *v, const char *name,
                                   void *opaque, Error **errp)
@@ -69,6 +70,8 @@ static void core_prop_set_nr_threads(Object *obj, Visitor *v, const char *name,
 
 static void cpu_core_instance_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_threads = ms->topo.smp_threads;
     CPUCore *core = CPU_CORE(obj);
 
     object_property_add(obj, "core-id", "int", core_prop_get_core_id,
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 47be907..a5eabe7 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -28,6 +28,7 @@
 #include "hw/loader.h"
 #include "exec/cpu-common.h"
 #include "smbios_build.h"
+#include "hw/boards.h"
 
 /* legacy structures and constants for <= 2.0 machines */
 struct smbios_header {
@@ -342,6 +343,9 @@ opts_init(smbios_register_config);
 
 static void smbios_validate_table(void)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+
     uint32_t expect_t4_count = smbios_legacy ? smp_cpus : smbios_smp_sockets;
 
     if (smbios_type4_count && smbios_type4_count != expect_t4_count) {
@@ -571,6 +575,9 @@ static void smbios_build_type_3_table(void)
 
 static void smbios_build_type_4_table(unsigned instance)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_threads = ms->topo.smp_threads;
+    unsigned int smp_cores = ms->topo.smp_cores;
     char sock_str[128];
 
     SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
@@ -843,7 +850,11 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
                        uint8_t **tables, size_t *tables_len,
                        uint8_t **anchor, size_t *anchor_len)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     unsigned i, dimm_cnt;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+    unsigned int smp_cores = ms->topo.smp_cores;
+    unsigned int smp_threads = ms->topo.smp_threads;
 
     if (smbios_legacy) {
         *tables = *anchor = NULL;
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index e2aa57a..ae92f6e 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -29,6 +29,7 @@
 #include "sysemu/balloon.h"
 #include "qemu/error-report.h"
 #include "trace.h"
+#include "hw/boards.h"
 
 /* Arbitrary limit on size of each discard command,
  * keeps them around ~200 bytes
@@ -128,6 +129,8 @@ static void migration_exit_cb(Notifier *n, void *data)
 
 static struct PostcopyBlocktimeContext *blocktime_context_new(void)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
     ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
     ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
@@ -141,6 +144,8 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
 
 static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     uint32List *list = NULL, *entry = NULL;
     int i;
 
@@ -806,8 +811,10 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
 static void mark_postcopy_blocktime_end(uintptr_t addr)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
+    MachineState *ms = MACHINE(qdev_get_machine());
     PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
     int i, affected_cpu = 0;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     bool vcpu_total_blocktime = false;
     uint32_t read_vcpu_time, low_time_offset;
 
diff --git a/numa.c b/numa.c
index 3875e1e..127ddbe 100644
--- a/numa.c
+++ b/numa.c
@@ -64,6 +64,7 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
     uint16_t nodenr;
     uint16List *cpus = NULL;
     MachineClass *mc = MACHINE_GET_CLASS(ms);
+    unsigned int max_cpus = ms->topo.max_cpus;
 
     if (node->has_nodeid) {
         nodenr = node->nodeid;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 9b2bf7f..d1501eb 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -44,6 +44,10 @@
 #include "exec/cpu-common.h"
 #include "exec/exec-all.h"
 
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/boards.h"
+#endif
+
 #include "tcg-op.h"
 
 #if UINTPTR_MAX == UINT32_MAX
@@ -602,6 +606,10 @@ static size_t tcg_n_regions(void)
     size_t i;
 
     /* Use a single region if all we have is one vCPU thread */
+#if !defined(CONFIG_USER_ONLY)
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
+#endif
     if (max_cpus == 1 || !qemu_tcg_mttcg_enabled()) {
         return 1;
     }
@@ -751,7 +759,12 @@ void tcg_register_thread(void)
 
     /* Claim an entry in tcg_ctxs */
     n = atomic_fetch_inc(&n_tcg_ctxs);
+#if !defined(CONFIG_USER_ONLY)
+    MachineState *ms = MACHINE(qdev_get_machine());
+    g_assert(n < ms->topo.max_cpus);
+#elif
     g_assert(n < max_cpus);
+#endif
     atomic_set(&tcg_ctxs[n], s);
 
     tcg_ctx = s;
@@ -961,6 +974,8 @@ void tcg_context_init(TCGContext *s)
     tcg_ctxs = &tcg_ctx;
     n_tcg_ctxs = 1;
 #else
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
     tcg_ctxs = g_new(TCGContext *, max_cpus);
 #endif
 
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 3/9] cpu/topology: add uncommon arch support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/alpha/dp264.c     | 1 +
 hw/hppa/machine.c    | 4 ++++
 hw/mips/boston.c     | 1 +
 hw/mips/mips_malta.c | 9 +++++++++
 hw/sparc/sun4m.c     | 2 ++
 hw/sparc64/sun4u.c   | 2 ++
 hw/xtensa/sim.c      | 1 +
 hw/xtensa/xtfpga.c   | 1 +
 8 files changed, 21 insertions(+)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 0347eb8..ee5d432 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -63,6 +63,7 @@ static void clipper_init(MachineState *machine)
     char *palcode_filename;
     uint64_t palcode_entry, palcode_low, palcode_high;
     uint64_t kernel_entry, kernel_low, kernel_high;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     /* Create up to 4 cpus.  */
     memset(cpus, 0, sizeof(cpus));
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index d1b1d3c..f652891 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -16,6 +16,7 @@
 #include "hw/ide.h"
 #include "hw/timer/i8254.h"
 #include "hw/char/serial.h"
+#include "hw/boards.h"
 #include "hppa_sys.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
@@ -72,6 +73,7 @@ static void machine_hppa_init(MachineState *machine)
     MemoryRegion *ram_region;
     MemoryRegion *cpu_region;
     long i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     ram_size = machine->ram_size;
 
@@ -242,7 +244,9 @@ static void machine_hppa_init(MachineState *machine)
 
 static void hppa_machine_reset(void)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     qemu_devices_reset();
 
diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index e5bab3c..7752c10 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -434,6 +434,7 @@ static void boston_mach_init(MachineState *machine)
     DriveInfo *hd[6];
     Chardev *chr;
     int fw_size, fit_err;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     bool is_64b;
 
     if ((machine->ram_size % GiB) ||
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 439665a..d595375 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1095,6 +1095,8 @@ static int64_t load_kernel (void)
 
 static void malta_mips_config(MIPSCPU *cpu)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     CPUMIPSState *env = &cpu->env;
     CPUState *cs = CPU(cpu);
 
@@ -1127,9 +1129,11 @@ static void main_cpu_reset(void *opaque)
 static void create_cpu_without_cps(const char *cpu_type,
                                    qemu_irq *cbus_irq, qemu_irq *i8259_irq)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     CPUMIPSState *env;
     MIPSCPU *cpu;
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     for (i = 0; i < smp_cpus; i++) {
         cpu = MIPS_CPU(cpu_create(cpu_type));
@@ -1149,7 +1153,9 @@ static void create_cpu_without_cps(const char *cpu_type,
 static void create_cps(MaltaState *s, const char *cpu_type,
                        qemu_irq *cbus_irq, qemu_irq *i8259_irq)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     Error *err = NULL;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
     qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
@@ -1171,6 +1177,9 @@ static void create_cps(MaltaState *s, const char *cpu_type,
 static void mips_create_cpu(MaltaState *s, const char *cpu_type,
                             qemu_irq *cbus_irq, qemu_irq *i8259_irq)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+
     if ((smp_cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
         create_cps(s, cpu_type, cbus_irq, i8259_irq);
     } else {
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index ca1e382..2321cfb 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -853,6 +853,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
     unsigned int num_vsimms;
     DeviceState *dev;
     SysBusDevice *s;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
+    unsigned int max_cpus = machine->topo.max_cpus;
 
     /* init CPUs */
     for(i = 0; i < smp_cpus; i++) {
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 399f2d7..d089c4d 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -546,6 +546,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
     NICInfo *nd;
     MACAddr macaddr;
     bool onboard_nic;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
+    unsigned int max_cpus = machine->topo.max_cpus;
 
     /* init CPUs */
     cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
index 12c7437..cc0396b 100644
--- a/hw/xtensa/sim.c
+++ b/hw/xtensa/sim.c
@@ -59,6 +59,7 @@ static void xtensa_sim_init(MachineState *machine)
     ram_addr_t ram_size = machine->ram_size;
     const char *kernel_filename = machine->kernel_filename;
     int n;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     for (n = 0; n < smp_cpus; n++) {
         cpu = XTENSA_CPU(cpu_create(machine->cpu_type));
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index e05ef75..f71a15e 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -238,6 +238,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
     const unsigned system_io_size = 224 * MiB;
     uint32_t freq = 10000000;
     int n;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     if (smp_cpus > 1) {
         mx_pic = xtensa_mx_pic_init(31);
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/arm/fsl-imx6.c      | 5 +++++
 hw/arm/fsl-imx6ul.c    | 5 +++++
 hw/arm/fsl-imx7.c      | 5 +++++
 hw/arm/highbank.c      | 1 +
 hw/arm/mcimx6ul-evk.c  | 1 +
 hw/arm/mcimx7d-sabre.c | 3 +++
 hw/arm/raspi.c         | 2 ++
 hw/arm/realview.c      | 1 +
 hw/arm/sabrelite.c     | 1 +
 hw/arm/vexpress.c      | 3 +++
 hw/arm/virt.c          | 7 +++++++
 hw/arm/xlnx-zynqmp.c   | 7 +++++++
 target/arm/cpu.c       | 7 +++++++
 13 files changed, 48 insertions(+)

diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 7b7b97f..efed3a4 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -23,6 +23,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "hw/arm/fsl-imx6.h"
+#include "hw/boards.h"
 #include "sysemu/sysemu.h"
 #include "chardev/char.h"
 #include "qemu/error-report.h"
@@ -33,9 +34,11 @@
 
 static void fsl_imx6_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     FslIMX6State *s = FSL_IMX6(obj);
     char name[NAME_SIZE];
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
         snprintf(name, NAME_SIZE, "cpu%d", i);
@@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
 
 static void fsl_imx6_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     FslIMX6State *s = FSL_IMX6(dev);
     uint16_t i;
     Error *err = NULL;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     if (smp_cpus > FSL_IMX6_NUM_CPUS) {
         error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index 4b56bfa..426bf8e 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -23,14 +23,17 @@
 #include "hw/misc/unimp.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
+#include "hw/boards.h"
 
 #define NAME_SIZE 20
 
 static void fsl_imx6ul_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     FslIMX6ULState *s = FSL_IMX6UL(obj);
     char name[NAME_SIZE];
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
         snprintf(name, NAME_SIZE, "cpu%d", i);
@@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
 
 static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     FslIMX6ULState *s = FSL_IMX6UL(dev);
     int i;
     qemu_irq irq;
     char name[NAME_SIZE];
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
         error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 7663ad6..b759f7b 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -25,11 +25,14 @@
 #include "hw/misc/unimp.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
+#include "hw/boards.h"
 
 #define NAME_SIZE 20
 
 static void fsl_imx7_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     FslIMX7State *s = FSL_IMX7(obj);
     char name[NAME_SIZE];
     int i;
@@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
 
 static void fsl_imx7_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     FslIMX7State *s = FSL_IMX7(dev);
     Object *o;
     int i;
     qemu_irq irq;
     char name[NAME_SIZE];
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     if (smp_cpus > FSL_IMX7_NUM_CPUS) {
         error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index 96ccf18..58c77f6 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
     SysBusDevice *busdev;
     qemu_irq pic[128];
     int n;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     qemu_irq cpu_irq[4];
     qemu_irq cpu_fiq[4];
     qemu_irq cpu_virq[4];
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index fb2b015..29fb706 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
     static struct arm_boot_info boot_info;
     MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
         error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 9c5f0e7..c2cdaf9 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -20,6 +20,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
 #include "sysemu/qtest.h"
+#include "hw/boards.h"
 
 typedef struct {
     FslIMX7State soc;
@@ -28,10 +29,12 @@ typedef struct {
 
 static void mcimx7d_sabre_init(MachineState *machine)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     static struct arm_boot_info boot_info;
     MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
     Object *soc;
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
         error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 66899c2..8ab8e10 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
 {
     static struct arm_boot_info binfo;
     int r;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     binfo.board_id = raspi_boardid[version];
     binfo.ram_size = ram_size;
@@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int version)
     BlockBackend *blk;
     BusState *bus;
     DeviceState *carddev;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     object_initialize(&s->soc, sizeof(s->soc),
                       version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 242f5a8..517c275 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
     NICInfo *nd;
     I2CBus *i2c;
     int n;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     int done_nic = 0;
     qemu_irq cpu_irq[4];
     int is_mpcore = 0;
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index ee140e5..108faab 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
 {
     IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
     Error *err = NULL;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     /* Check the amount of memory is compatible with the SOC */
     if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index f07134c..1927d12 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -206,9 +206,11 @@ struct VEDBoardInfo {
 static void init_cpus(const char *cpu_type, const char *privdev,
                       hwaddr periphbase, qemu_irq *pic, bool secure, bool virt)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     DeviceState *dev;
     SysBusDevice *busdev;
     int n;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     /* Create the actual CPUs */
     for (n = 0; n < smp_cpus; n++) {
@@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState *machine)
     MemoryRegion *flash0mem;
     const hwaddr *map = daughterboard->motherboard_map;
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
 
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ce2664a..ff1dff3 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
 
 static void create_gic(VirtMachineState *vms, qemu_irq *pic)
 {
+    MachineState *ms = MACHINE(vms);
     /* We create a standalone GIC */
     DeviceState *gicdev;
     SysBusDevice *gicbusdev;
     const char *gictype;
     int type = vms->gic_version, i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     uint32_t nb_redist_regions = 0;
 
     gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
@@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState *vms,
 
 static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
 {
+    MachineState *ms = MACHINE(vms);
     hwaddr base = vms->memmap[VIRT_FW_CFG].base;
     hwaddr size = vms->memmap[VIRT_FW_CFG].size;
     FWCfgState *fw_cfg;
     char *nodename;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
 
     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);
@@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
     bool aarch64 = true;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
+    unsigned int max_cpus = machine->topo.max_cpus;
 
     /*
      * In accelerated mode, the memory map is computed earlier in kvm_type()
@@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
 static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
 {
     int n;
+    unsigned int max_cpus = ms->topo.max_cpus;
     VirtMachineState *vms = VIRT_MACHINE(ms);
 
     if (ms->possible_cpus) {
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 4f8bc41..29b9427 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -19,6 +19,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "hw/boards.h"
 #include "hw/arm/xlnx-zynqmp.h"
 #include "hw/intc/arm_gic_common.h"
 #include "exec/address-spaces.h"
@@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
 static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
                                    Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     Error *err = NULL;
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
 
     if (num_rpus <= 0) {
@@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
 
 static void xlnx_zynqmp_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
     int i;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
 
     object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
@@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
     XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
     MemoryRegion *system_memory = get_system_memory();
     uint8_t i;
     uint64_t ram_size;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
     const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
     ram_addr_t ddr_low_size, ddr_high_size;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 4155782..e5d7e3b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -30,6 +30,7 @@
 #include "hw/qdev-properties.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
+#include "hw/boards.h"
 #endif
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
@@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     init_cpreg_list(cpu);
 
 #ifndef CONFIG_USER_ONLY
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+
     if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
         cs->num_ases = 2;
 
@@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
 #ifndef CONFIG_USER_ONLY
 static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+
     /* Linux wants the number of processors from here.
      * Might as well set the interrupt-controller bit too.
      */
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 5/9] cpu/topology: add i386 support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/i386/acpi-build.c |  3 +++
 hw/i386/kvmvapic.c   |  5 +++++
 hw/i386/pc.c         | 12 ++++++++++++
 target/i386/cpu.c    |  4 ++++
 4 files changed, 24 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 416da31..3813c28 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -44,6 +44,7 @@
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
 #include "hw/acpi/vmgenid.h"
+#include "hw/boards.h"
 #include "sysemu/tpm_backend.h"
 #include "hw/timer/mc146818rtc_regs.h"
 #include "hw/mem/memory-device.h"
@@ -127,6 +128,8 @@ typedef struct FwCfgTPMConfig {
 
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
     uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
     AmlAddressSpace as = AML_AS_SYSTEM_IO;
     AcpiFadtData fadt = {
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 70f6f26..2463f8a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -17,6 +17,7 @@
 #include "sysemu/kvm.h"
 #include "hw/i386/apic_internal.h"
 #include "hw/sysbus.h"
+#include "hw/boards.h"
 #include "tcg/tcg.h"
 
 #define VAPIC_IO_PORT           0x7e
@@ -441,6 +442,8 @@ static void do_patch_instruction(CPUState *cs, run_on_cpu_data data)
 
 static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     CPUState *cs = CPU(cpu);
     VAPICHandlers *handlers;
     PatchInfo *info;
@@ -746,6 +749,8 @@ static void do_vapic_enable(CPUState *cs, run_on_cpu_data data)
 static void kvmvapic_vm_state_change(void *opaque, int running,
                                      RunState state)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     VAPICROMState *s = opaque;
     uint8_t *zero;
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6077d27..2816bfd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -931,6 +931,9 @@ void enable_compat_apic_id_mode(void)
  */
 static uint32_t x86_cpu_apic_id_from_index(unsigned int cpu_index)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cores = ms->topo.smp_cores;
+    unsigned int smp_threads = ms->topo.smp_threads;
     uint32_t correct_id;
     static bool warned;
 
@@ -1562,6 +1565,7 @@ void pc_cpus_init(PCMachineState *pcms)
     const CPUArchIdList *possible_cpus;
     MachineState *ms = MACHINE(pcms);
     MachineClass *mc = MACHINE_GET_CLASS(pcms);
+    unsigned int smp_cpus = ms->topo.smp_cpus, max_cpus = ms->topo.max_cpus;
 
     /* Calculates the limit to CPU APIC ID values
      *
@@ -2291,6 +2295,9 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
     X86CPU *cpu = X86_CPU(dev);
     MachineState *ms = MACHINE(hotplug_dev);
     PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+    unsigned int smp_cores = ms->topo.smp_cores;
+    unsigned int smp_threads = ms->topo.smp_threads;
+    unsigned int max_cpus = ms->topo.max_cpus;
 
     if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
         error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
@@ -2646,6 +2653,8 @@ pc_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
 static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
 {
    X86CPUTopoInfo topo;
+   unsigned int smp_cores = ms->topo.smp_cores;
+   unsigned int smp_threads = ms->topo.smp_threads;
 
    assert(idx < ms->possible_cpus->len);
    x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id,
@@ -2656,6 +2665,9 @@ static int64_t pc_get_default_cpu_node_id(const MachineState *ms, int idx)
 static const CPUArchIdList *pc_possible_cpu_arch_ids(MachineState *ms)
 {
     int i;
+    unsigned int max_cpus = ms->topo.max_cpus;
+    unsigned int smp_cores = ms->topo.smp_cores;
+    unsigned int smp_threads = ms->topo.smp_threads;
 
     if (ms->possible_cpus) {
         /*
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d6bb57d..6dcdad4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -53,6 +53,7 @@
 #include "hw/hw.h"
 #include "hw/xen/xen.h"
 #include "hw/i386/apic_internal.h"
+#include "hw/boards.h"
 #endif
 
 #include "disas/capstone.h"
@@ -5291,6 +5292,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
 #ifndef CONFIG_USER_ONLY
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
+
     qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
 
     if (cpu->env.features[FEAT_1_EDX] & CPUID_APIC || smp_cpus > 1) {
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 6/9] cpu/topology: add PPC support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/ppc/e500.c         |  3 +++
 hw/ppc/mac_newworld.c |  2 ++
 hw/ppc/mac_oldworld.c |  2 ++
 hw/ppc/pnv.c          |  3 +++
 hw/ppc/prep.c         |  2 ++
 hw/ppc/spapr.c        | 29 +++++++++++++++++++++++++++++
 hw/ppc/spapr_rtas.c   |  3 +++
 7 files changed, 44 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index beb2efd..c854cb5 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -307,6 +307,7 @@ static int ppce500_load_device_tree(PPCE500MachineState *pms,
                                     bool dry_run)
 {
     MachineState *machine = MACHINE(pms);
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     const PPCE500MachineClass *pmc = PPCE500_MACHINE_GET_CLASS(pms);
     CPUPPCState *env = first_cpu->env_ptr;
     int ret = -1;
@@ -734,6 +735,7 @@ static DeviceState *ppce500_init_mpic_qemu(PPCE500MachineState *pms,
     SysBusDevice *s;
     int i, j, k;
     MachineState *machine = MACHINE(pms);
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     const PPCE500MachineClass *pmc = PPCE500_MACHINE_GET_CLASS(pms);
 
     dev = qdev_create(NULL, TYPE_OPENPIC);
@@ -846,6 +848,7 @@ void ppce500_init(MachineState *machine)
     struct boot_info *boot_info;
     int dt_size;
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     /* irq num for pin INTA, INTB, INTC and INTD is 1, 2, 3 and
      * 4 respectively */
     unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4};
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 02d8559..ce236ee 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -131,10 +131,12 @@ static void ppc_core99_init(MachineState *machine)
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
     int machine_arch;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     SysBusDevice *s;
     DeviceState *dev, *pic_dev;
     hwaddr nvram_addr = 0xFFF04000;
     uint64_t tbfreq;
+    unsigned int max_cpus = machine->topo.max_cpus;
 
     linux_boot = (kernel_filename != NULL);
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 460cbc7..cde20c9 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -99,6 +99,8 @@ static void ppc_heathrow_init(MachineState *machine)
     DeviceState *dev, *pic_dev;
     BusState *adb_bus;
     int bios_size;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
+    unsigned int max_cpus = machine->topo.max_cpus;
     uint16_t ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
     void *fw_cfg;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index dfb4ea5..c99c0b2 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -596,6 +596,7 @@ static void pnv_init(MachineState *machine)
     long fw_size;
     int i;
     char *chip_typename;
+    unsigned int smp_cores = machine->topo.smp_cores;
 
     /* allocate RAM */
     if (machine->ram_size < (1 * GiB)) {
@@ -1135,6 +1136,8 @@ static void pnv_chip_instance_init(Object *obj)
 
 static void pnv_chip_core_realize(PnvChip *chip, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_threads = ms->topo.smp_threads;
     Error *error = NULL;
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(chip);
     const char *typename = pnv_chip_core_typename(chip);
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 847d320..86aa021 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -423,6 +423,7 @@ static void ppc_prep_init(MachineState *machine)
     ISADevice *isa;
     int ppc_boot_device;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     sysctrl = g_malloc0(sizeof(sysctrl_t));
 
@@ -630,6 +631,7 @@ static void ibm_40p_init(MachineState *machine)
     uint32_t kernel_base = 0, initrd_base = 0;
     long kernel_size = 0, initrd_size = 0;
     char boot_device;
+    unsigned int max_cpus = machine->topo.max_cpus;
 
     /* init CPU */
     cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6c16d6c..bf70121 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -105,6 +105,9 @@
  */
 static int spapr_vcpu_id(SpaprMachineState *spapr, int cpu_index)
 {
+    MachineState *ms = MACHINE(spapr);
+    unsigned int smp_threads = ms->topo.smp_threads;
+
     assert(spapr->vsmt);
     return
         (cpu_index / smp_threads) * spapr->vsmt + cpu_index % smp_threads;
@@ -152,6 +155,10 @@ static void pre_2_10_vmstate_unregister_dummy_icp(int i)
 
 int spapr_max_server_number(SpaprMachineState *spapr)
 {
+    MachineState *ms = MACHINE(spapr);
+    unsigned int smp_threads = ms->topo.smp_threads;
+    unsigned int max_cpus = ms->topo.max_cpus;
+
     assert(spapr->vsmt);
     return DIV_ROUND_UP(max_cpus * spapr->vsmt, smp_threads);
 }
@@ -286,10 +293,12 @@ static void spapr_populate_pa_features(SpaprMachineState *spapr,
 
 static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
 {
+    MachineState *ms = MACHINE(spapr);
     int ret = 0, offset, cpus_offset;
     CPUState *cs;
     char cpu_model[32];
     uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
+    unsigned int smp_threads = ms->topo.smp_threads;
 
     CPU_FOREACH(cs) {
         PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -441,6 +450,7 @@ static int spapr_populate_memory(SpaprMachineState *spapr, void *fdt)
 static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
                                   SpaprMachineState *spapr)
 {
+    MachineState *ms = MACHINE(spapr);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
@@ -452,6 +462,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
     uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
     uint32_t page_sizes_prop[64];
     size_t page_sizes_prop_size;
+    unsigned int smp_threads = ms->topo.smp_threads;
+    unsigned int smp_cores = ms->topo.smp_cores;
     uint32_t vcpus_per_socket = smp_threads * smp_cores;
     uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
     int compat_smt = MIN(smp_threads, ppc_compat_max_vthreads(cpu));
@@ -1022,6 +1034,9 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr,
 
 static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
 {
+    MachineState *ms = MACHINE(spapr);
+    unsigned int max_cpus = ms->topo.max_cpus;
+    unsigned int smp_threads = ms->topo.smp_threads;
     int rtas;
     GString *hypertas = g_string_sized_new(256);
     GString *qemu_hypertas = g_string_sized_new(256);
@@ -2513,6 +2528,7 @@ static void spapr_validate_node_memory(MachineState *machine, Error **errp)
 /* find cpu slot in machine->possible_cpus by core_id */
 static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
 {
+    unsigned int smp_threads = ms->topo.smp_threads;
     int index = id / smp_threads;
 
     if (index >= ms->possible_cpus->len) {
@@ -2526,10 +2542,12 @@ static CPUArchId *spapr_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
 
 static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
 {
+    MachineState *ms = MACHINE(spapr);
     Error *local_err = NULL;
     bool vsmt_user = !!spapr->vsmt;
     int kvm_smt = kvmppc_smt_threads();
     int ret;
+    unsigned int smp_threads = ms->topo.smp_threads;
 
     if (!kvm_enabled() && (smp_threads > 1)) {
         error_setg(&local_err, "TCG cannot support more than 1 thread/core "
@@ -2603,6 +2621,9 @@ static void spapr_init_cpus(SpaprMachineState *spapr)
     SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
     const char *type = spapr_get_cpu_core_type(machine->cpu_type);
     const CPUArchIdList *possible_cpus;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
+    unsigned int smp_threads = machine->topo.smp_threads;
+    unsigned int max_cpus = machine->topo.max_cpus;
     int boot_cores_nr = smp_cpus / smp_threads;
     int i;
 
@@ -3835,6 +3856,7 @@ static void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     const char *type = object_get_typename(OBJECT(dev));
     CPUArchId *core_slot;
     int index;
+    unsigned int smp_threads = machine->topo.smp_threads;
 
     if (dev->hotplugged && !mc->has_hotpluggable_cpus) {
         error_setg(&local_err, "CPU hotplug not supported for this machine");
@@ -4087,12 +4109,17 @@ spapr_cpu_index_to_props(MachineState *machine, unsigned cpu_index)
 
 static int64_t spapr_get_default_cpu_node_id(const MachineState *ms, int idx)
 {
+    unsigned int smp_cores = ms->topo.smp_cores;
+
     return idx / smp_cores % nb_numa_nodes;
 }
 
 static const CPUArchIdList *spapr_possible_cpu_arch_ids(MachineState *machine)
 {
     int i;
+    unsigned int max_cpus = machine->topo.max_cpus;
+    unsigned int smp_threads = machine->topo.smp_threads;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     const char *core_type;
     int spapr_max_cores = max_cpus / smp_threads;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -4213,6 +4240,8 @@ int spapr_get_vcpu_id(PowerPCCPU *cpu)
 void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    MachineState *ms = MACHINE(spapr);
+    unsigned int smp_threads = ms->topo.smp_threads;
     int vcpu_id;
 
     vcpu_id = spapr_vcpu_id(spapr, cpu_index);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 24c45b1..658f3f9 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -231,6 +231,9 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
                                           target_ulong args,
                                           uint32_t nret, target_ulong rets)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     target_ulong parameter = rtas_ld(args, 0);
     target_ulong buffer = rtas_ld(args, 1);
     target_ulong length = rtas_ld(args, 2);
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 7/9] cpu/topology: add riscv support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/openrisc/openrisc_sim.c | 1 +
 hw/riscv/sifive_e.c        | 4 ++++
 hw/riscv/sifive_plic.c     | 3 +++
 hw/riscv/sifive_u.c        | 4 ++++
 hw/riscv/spike.c           | 2 ++
 hw/riscv/virt.c            | 1 +
 6 files changed, 15 insertions(+)

diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 7d3b734..ecfc973 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -131,6 +131,7 @@ static void openrisc_sim_init(MachineState *machine)
     qemu_irq *cpu_irqs[2];
     qemu_irq serial_irq;
     int n;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     for (n = 0; n < smp_cpus; n++) {
         cpu = OPENRISC_CPU(cpu_create(machine->cpu_type));
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index b1cd113..c65c7b5 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -137,6 +137,8 @@ static void riscv_sifive_e_init(MachineState *machine)
 
 static void riscv_sifive_e_soc_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     SiFiveESoCState *s = RISCV_E_SOC(obj);
 
     object_initialize_child(obj, "cpus", &s->cpus,
@@ -150,6 +152,8 @@ static void riscv_sifive_e_soc_init(Object *obj)
 
 static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     const struct MemmapEntry *memmap = sifive_e_memmap;
 
     SiFiveESoCState *s = RISCV_E_SOC(dev);
diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index ac768e6..161fbd9 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -23,6 +23,7 @@
 #include "qemu/error-report.h"
 #include "hw/sysbus.h"
 #include "hw/pci/msi.h"
+#include "hw/boards.h"
 #include "target/riscv/cpu.h"
 #include "sysemu/sysemu.h"
 #include "hw/riscv/sifive_plic.h"
@@ -432,6 +433,8 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level)
 
 static void sifive_plic_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     SiFivePLICState *plic = SIFIVE_PLIC(dev);
     int i;
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 5ecc47c..b4a8d66 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -321,6 +321,8 @@ static void riscv_sifive_u_init(MachineState *machine)
 
 static void riscv_sifive_u_soc_init(Object *obj)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     SiFiveUSoCState *s = RISCV_U_SOC(obj);
 
     object_initialize_child(obj, "cpus", &s->cpus, sizeof(s->cpus),
@@ -336,6 +338,8 @@ static void riscv_sifive_u_soc_init(Object *obj)
 
 static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
 {
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int smp_cpus = ms->topo.smp_cpus;
     SiFiveUSoCState *s = RISCV_U_SOC(dev);
     const struct MemmapEntry *memmap = sifive_u_memmap;
     MemoryRegion *system_memory = get_system_memory();
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 2a000a5..5fe441c 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -171,6 +171,7 @@ static void spike_v1_10_0_board_init(MachineState *machine)
     MemoryRegion *main_mem = g_new(MemoryRegion, 1);
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     /* Initialize SOC */
     object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
@@ -253,6 +254,7 @@ static void spike_v1_09_1_board_init(MachineState *machine)
     MemoryRegion *main_mem = g_new(MemoryRegion, 1);
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
 
     /* Initialize SOC */
     object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index fc4c6b3..9d4d305 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -395,6 +395,7 @@ static void riscv_virt_board_init(MachineState *machine)
     char *plic_hart_config;
     size_t plic_hart_config_len;
     int i;
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     void *fdt;
 
     /* Initialize SOC */
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 8/9] cpu/topology: add s390x support for smp machine properties

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 hw/s390x/s390-virtio-ccw.c   | 2 ++
 hw/s390x/sclp.c              | 1 +
 target/openrisc/sys_helper.c | 5 +++++
 target/s390x/cpu.c           | 3 +++
 target/s390x/excp_helper.c   | 6 ++++++
 5 files changed, 17 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index d11069b..8cb71e0 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -77,6 +77,7 @@ out:
 static void s390_init_cpus(MachineState *machine)
 {
     MachineClass *mc = MACHINE_GET_CLASS(machine);
+    unsigned int smp_cpus = machine->topo.smp_cpus;
     int i;
 
     /* initialize possible_cpus */
@@ -398,6 +399,7 @@ static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
 static const CPUArchIdList *s390_possible_cpu_arch_ids(MachineState *ms)
 {
     int i;
+    unsigned int max_cpus = ms->topo.max_cpus;
 
     if (ms->possible_cpus) {
         g_assert(ms->possible_cpus && ms->possible_cpus->len == max_cpus);
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 4510a80..1de3fb6 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -56,6 +56,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 {
     ReadInfo *read_info = (ReadInfo *) sccb;
     MachineState *machine = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = machine->topo.max_cpus;
     int cpu_count;
     int rnsize, rnmax;
     IplParameterBlock *ipib = s390_ipl_get_iplb();
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 05f66c4..a8b03c6 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -24,6 +24,9 @@
 #include "exec/helper-proto.h"
 #include "exception.h"
 #include "sysemu/sysemu.h"
+#ifndef CONFIG_USER_ONLY
+#include "hw/boards.h"
+#endif
 
 #define TO_SPR(group, number) (((group) << 11) + (number))
 
@@ -194,6 +197,8 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env, target_ulong rd,
                            target_ulong spr)
 {
 #ifndef CONFIG_USER_ONLY
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
     OpenRISCCPU *cpu = openrisc_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
     int idx;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 698dd9c..4b614c5 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -37,6 +37,7 @@
 #include "hw/qdev-properties.h"
 #ifndef CONFIG_USER_ONLY
 #include "hw/hw.h"
+#include "hw/boards.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/sysemu.h"
 #endif
@@ -193,6 +194,8 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
 #if !defined(CONFIG_USER_ONLY)
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->topo.max_cpus;
     if (cpu->env.core_id >= max_cpus) {
         error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
                    ", maximum core-id: %d", cpu->env.core_id,
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index f84bfb1..d3eec86 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -31,6 +31,7 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "hw/s390x/s390_flic.h"
+#include "hw/boards.h"
 #endif
 
 void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
@@ -279,7 +280,12 @@ static void do_ext_interrupt(CPUS390XState *env)
         g_assert(cpu_addr < S390_MAX_CPUS);
         lowcore->cpu_addr = cpu_to_be16(cpu_addr);
         clear_bit(cpu_addr, env->emergency_signals);
+#ifndef CONFIG_USER_ONLY
+        MachineState *ms = MACHINE(qdev_get_machine());
+        if (bitmap_empty(env->emergency_signals, ms->topo.max_cpus)) {
+#elif
         if (bitmap_empty(env->emergency_signals, max_cpus)) {
+#endif
             env->pending_int &= ~INTERRUPT_EMERGENCY_SIGNAL;
         }
     } else if ((env->pending_int & INTERRUPT_EXTERNAL_CALL) &&
-- 
1.8.3.1

[Qemu-trivial] [Qemu-devel] [PATCH 9/9] cpu/topology: replace smp global variables with machine propertie

[Like Xu]

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 vl.c | 53 ++++++++++++++++++++++++++---------------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/vl.c b/vl.c
index 9089253..0c5a384 100644
--- a/vl.c
+++ b/vl.c
@@ -162,10 +162,6 @@ static Chardev **serial_hds;
 Chardev *parallel_hds[MAX_PARALLEL_PORTS];
 int win2k_install_hack = 0;
 int singlestep = 0;
-unsigned int smp_cpus;
-unsigned int max_cpus;
-int smp_cores = 1;
-int smp_threads = 1;
 int acpi_enabled = 1;
 int no_hpet = 0;
 int fd_bootchk = 1;
@@ -1281,8 +1277,9 @@ static void smp_parse(QemuOpts *opts)
                 sockets = sockets > 0 ? sockets : 1;
                 cpus = cores * threads * sockets;
             } else {
-                max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
-                sockets = max_cpus / (cores * threads);
+                current_machine->topo.max_cpus =
+                                qemu_opt_get_number(opts, "maxcpus", cpus);
+                sockets = current_machine->topo.max_cpus / (cores * threads);
             }
         } else if (cores == 0) {
             threads = threads > 0 ? threads : 1;
@@ -1299,34 +1296,37 @@ static void smp_parse(QemuOpts *opts)
             exit(1);
         }
 
-        max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
+        current_machine->topo.max_cpus =
+                                qemu_opt_get_number(opts, "maxcpus", cpus);
 
-        if (max_cpus < cpus) {
+        if (current_machine->topo.max_cpus < cpus) {
             error_report("maxcpus must be equal to or greater than smp");
             exit(1);
         }
 
-        if (sockets * cores * threads > max_cpus) {
+        if (sockets * cores * threads > current_machine->topo.max_cpus) {
             error_report("cpu topology: "
                          "sockets (%u) * cores (%u) * threads (%u) > "
                          "maxcpus (%u)",
-                         sockets, cores, threads, max_cpus);
+                         sockets, cores, threads,
+                         current_machine->topo.max_cpus);
             exit(1);
         }
 
-        if (sockets * cores * threads != max_cpus) {
+        if (sockets * cores * threads != current_machine->topo.max_cpus) {
             warn_report("Invalid CPU topology deprecated: "
                         "sockets (%u) * cores (%u) * threads (%u) "
                         "!= maxcpus (%u)",
-                        sockets, cores, threads, max_cpus);
+                        sockets, cores, threads,
+                        current_machine->topo.max_cpus);
         }
 
-        smp_cpus = cpus;
-        smp_cores = cores;
-        smp_threads = threads;
+        current_machine->topo.smp_cpus = cpus;
+        current_machine->topo.smp_cores = cores;
+        current_machine->topo.smp_threads = threads;
     }
 
-    if (smp_cpus > 1) {
+    if (current_machine->topo.smp_cpus > 1) {
         Error *blocker = NULL;
         error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
         replay_add_blocker(blocker);
@@ -4111,26 +4111,25 @@ int main(int argc, char **argv, char **envp)
     machine_class->default_cpus = machine_class->default_cpus ?: 1;
 
     /* default to machine_class->default_cpus */
-    smp_cpus = machine_class->default_cpus;
-    max_cpus = machine_class->default_cpus;
+    current_machine->topo.smp_cpus = machine_class->default_cpus;
+    current_machine->topo.max_cpus = machine_class->max_cpus;
+    current_machine->topo.smp_cores = 1;
+    current_machine->topo.smp_threads = 1;
 
     smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
-    current_machine->topo.smp_cpus = smp_cpus;
-    current_machine->topo.max_cpus = max_cpus;
-    current_machine->topo.smp_cores = smp_cores;
-    current_machine->topo.smp_threads = smp_threads;
-
     /* sanity-check smp_cpus and max_cpus against machine_class */
-    if (smp_cpus < machine_class->min_cpus) {
+    if (current_machine->topo.smp_cpus < machine_class->min_cpus) {
         error_report("Invalid SMP CPUs %d. The min CPUs "
-                     "supported by machine '%s' is %d", smp_cpus,
+                     "supported by machine '%s' is %d",
+                     current_machine->topo.smp_cpus,
                      machine_class->name, machine_class->min_cpus);
         exit(1);
     }
-    if (max_cpus > machine_class->max_cpus) {
+    if (current_machine->topo.max_cpus > machine_class->max_cpus) {
         error_report("Invalid SMP CPUs %d. The max CPUs "
-                     "supported by machine '%s' is %d", max_cpus,
+                     "supported by machine '%s' is %d",
+                     current_machine->topo.max_cpus,
                      machine_class->name, machine_class->max_cpus);
         exit(1);
     }
-- 
1.8.3.1

Re: [Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[no-reply]

Patchew URL: https://patchew.org/QEMU/1553849325-44201-1-git-send-email-like.xu@linux.intel.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

/usr/bin/ld: /tmp/qemu-test/src/hw/i386/xen/xen-hvm.c:(.text+0x17f7): undefined reference to `max_cpus'
/usr/bin/ld: /tmp/qemu-test/src/hw/i386/xen/xen-hvm.c:(.text+0x1b1e): undefined reference to `max_cpus'
/usr/bin/ld: hw/i386/xen/xen-hvm.o:/tmp/qemu-test/src/hw/i386/xen/xen-hvm.c:(.text+0x1b3e): more undefined references to `max_cpus' follow
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:204: qemu-system-x86_64] Error 1
make: *** [Makefile:450: subdir-x86_64-softmmu] Error 2
Traceback (most recent call last):


The full log is available at
http://patchew.org/logs/1553849325-44201-1-git-send-email-like.xu@linux.intel.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/9] cpu/topology: add struct CpuTopology to MachineState

[Alex Bennée]

Like Xu <like.xu@linux.intel.com> writes:

> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  include/hw/arm/virt.h   | 2 +-
>  include/hw/boards.h     | 8 ++++++++
>  include/sysemu/sysemu.h | 2 +-
>  vl.c                    | 7 ++++++-
>  4 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 507517c..724da0c 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -123,7 +123,7 @@ typedef struct {
>      struct arm_boot_info bootinfo;
>      MemMapEntry *memmap;
>      const int *irqmap;
> -    int smp_cpus;
> +    unsigned int smp_cpus;

This seem unrelated to the rest of the change.

>      void *fdt;
>      int fdt_size;
<snip>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6065d9e..c0d7d7c 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -100,7 +100,7 @@ extern const char *keyboard_layout;
>  extern int win2k_install_hack;
>  extern int alt_grab;
>  extern int ctrl_grab;
> -extern int smp_cpus;
> +extern unsigned int smp_cpus;

Along with this. Is this meant to align with what is currently defined
as smp_cpus in vl.c? Maybe these changes should be rolled together with
that before the patch to introduce the new topology structure?

--
Alex Bennée

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Alex Bennée]

Like Xu <like.xu@linux.intel.com> writes:

> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hw/arm/fsl-imx6.c      | 5 +++++
>  hw/arm/fsl-imx6ul.c    | 5 +++++
>  hw/arm/fsl-imx7.c      | 5 +++++
>  hw/arm/highbank.c      | 1 +
>  hw/arm/mcimx6ul-evk.c  | 1 +
>  hw/arm/mcimx7d-sabre.c | 3 +++
>  hw/arm/raspi.c         | 2 ++
>  hw/arm/realview.c      | 1 +
>  hw/arm/sabrelite.c     | 1 +
>  hw/arm/vexpress.c      | 3 +++
>  hw/arm/virt.c          | 7 +++++++
>  hw/arm/xlnx-zynqmp.c   | 7 +++++++
>  target/arm/cpu.c       | 7 +++++++
>  13 files changed, 48 insertions(+)
>
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index 7b7b97f..efed3a4 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -23,6 +23,7 @@
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "hw/arm/fsl-imx6.h"
> +#include "hw/boards.h"
>  #include "sysemu/sysemu.h"
>  #include "chardev/char.h"
>  #include "qemu/error-report.h"
> @@ -33,9 +34,11 @@
>
>  static void fsl_imx6_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6State *s = FSL_IMX6(obj);
>      char name[NAME_SIZE];
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
> @@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
>
>  static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6State *s = FSL_IMX6(dev);
>      uint16_t i;
>      Error *err = NULL;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      if (smp_cpus > FSL_IMX6_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 4b56bfa..426bf8e 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -23,14 +23,17 @@
>  #include "hw/misc/unimp.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "hw/boards.h"
>
>  #define NAME_SIZE 20
>
>  static void fsl_imx6ul_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6ULState *s = FSL_IMX6UL(obj);
>      char name[NAME_SIZE];
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
> @@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
>
>  static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6ULState *s = FSL_IMX6UL(dev);
>      int i;
>      qemu_irq irq;
>      char name[NAME_SIZE];
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 7663ad6..b759f7b 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -25,11 +25,14 @@
>  #include "hw/misc/unimp.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "hw/boards.h"
>
>  #define NAME_SIZE 20
>
>  static void fsl_imx7_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      FslIMX7State *s = FSL_IMX7(obj);
>      char name[NAME_SIZE];
>      int i;
> @@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
>
>  static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX7State *s = FSL_IMX7(dev);
>      Object *o;
>      int i;
>      qemu_irq irq;
>      char name[NAME_SIZE];
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      if (smp_cpus > FSL_IMX7_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 96ccf18..58c77f6 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>      SysBusDevice *busdev;
>      qemu_irq pic[128];
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>      qemu_irq cpu_irq[4];
>      qemu_irq cpu_fiq[4];
>      qemu_irq cpu_virq[4];
> diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
> index fb2b015..29fb706 100644
> --- a/hw/arm/mcimx6ul-evk.c
> +++ b/hw/arm/mcimx6ul-evk.c
> @@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
>      static struct arm_boot_info boot_info;
>      MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
>      int i;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>
>      if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
>          error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
> diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
> index 9c5f0e7..c2cdaf9 100644
> --- a/hw/arm/mcimx7d-sabre.c
> +++ b/hw/arm/mcimx7d-sabre.c
> @@ -20,6 +20,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/qtest.h"
> +#include "hw/boards.h"
>
>  typedef struct {
>      FslIMX7State soc;
> @@ -28,10 +29,12 @@ typedef struct {
>
>  static void mcimx7d_sabre_init(MachineState *machine)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());

Isn't this redundant? Doesn't ms == machine?

>      static struct arm_boot_info boot_info;
>      MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
>      Object *soc;
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
>          error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 66899c2..8ab8e10 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>  {
>      static struct arm_boot_info binfo;
>      int r;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>
>      binfo.board_id = raspi_boardid[version];
>      binfo.ram_size = ram_size;
> @@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int version)
>      BlockBackend *blk;
>      BusState *bus;
>      DeviceState *carddev;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>
>      object_initialize(&s->soc, sizeof(s->soc),
>                        version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 242f5a8..517c275 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
>      NICInfo *nd;
>      I2CBus *i2c;
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>      int done_nic = 0;
>      qemu_irq cpu_irq[4];
>      int is_mpcore = 0;
> diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
> index ee140e5..108faab 100644
> --- a/hw/arm/sabrelite.c
> +++ b/hw/arm/sabrelite.c
> @@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
>  {
>      IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
>      Error *err = NULL;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>
>      /* Check the amount of memory is compatible with the SOC */
>      if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index f07134c..1927d12 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -206,9 +206,11 @@ struct VEDBoardInfo {
>  static void init_cpus(const char *cpu_type, const char *privdev,
>                        hwaddr periphbase, qemu_irq *pic, bool secure, bool virt)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      DeviceState *dev;
>      SysBusDevice *busdev;
>      int n;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      /* Create the actual CPUs */
>      for (n = 0; n < smp_cpus; n++) {
> @@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState *machine)
>      MemoryRegion *flash0mem;
>      const hwaddr *map = daughterboard->motherboard_map;
>      int i;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>
>      daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ce2664a..ff1dff3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
>
>  static void create_gic(VirtMachineState *vms, qemu_irq *pic)
>  {
> +    MachineState *ms = MACHINE(vms);
>      /* We create a standalone GIC */
>      DeviceState *gicdev;
>      SysBusDevice *gicbusdev;
>      const char *gictype;
>      int type = vms->gic_version, i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      uint32_t nb_redist_regions = 0;
>
>      gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
> @@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState *vms,
>
>  static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
>  {
> +    MachineState *ms = MACHINE(vms);
>      hwaddr base = vms->memmap[VIRT_FW_CFG].base;
>      hwaddr size = vms->memmap[VIRT_FW_CFG].size;
>      FWCfgState *fw_cfg;
>      char *nodename;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>
>      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);
> @@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>      bool aarch64 = true;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> +    unsigned int max_cpus = machine->topo.max_cpus;
>
>      /*
>       * In accelerated mode, the memory map is computed earlier in kvm_type()
> @@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>  static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>  {
>      int n;
> +    unsigned int max_cpus = ms->topo.max_cpus;
>      VirtMachineState *vms = VIRT_MACHINE(ms);
>
>      if (ms->possible_cpus) {
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 4f8bc41..29b9427 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -19,6 +19,7 @@
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> +#include "hw/boards.h"
>  #include "hw/arm/xlnx-zynqmp.h"
>  #include "hw/intc/arm_gic_common.h"
>  #include "exec/address-spaces.h"
> @@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
>  static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>                                     Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      Error *err = NULL;
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
>
>      if (num_rpus <= 0) {
> @@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>
>  static void xlnx_zynqmp_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>
>      object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
> @@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
>
>  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
>      MemoryRegion *system_memory = get_system_memory();
>      uint8_t i;
>      uint64_t ram_size;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>      ram_addr_t ddr_low_size, ddr_high_size;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 4155782..e5d7e3b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -30,6 +30,7 @@
>  #include "hw/qdev-properties.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
> +#include "hw/boards.h"
>  #endif
>  #include "hw/arm/arm.h"
>  #include "sysemu/sysemu.h"
> @@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      init_cpreg_list(cpu);
>
>  #ifndef CONFIG_USER_ONLY
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +
>      if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>          cs->num_ases = 2;
>
> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>  #ifndef CONFIG_USER_ONLY
>  static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());

How expensive is qdev_get_machine? This could potentially be a
performance issue if this register is read a lot.

> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +
>      /* Linux wants the number of processors from here.
>       * Might as well set the interrupt-controller bit too.
>       */


--
Alex Bennée

Re: [Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Igor Mammedov]

On Fri, 29 Mar 2019 16:48:36 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> This patch series make existing cores/threads/sockets into machine
> properties and get rid of global variables they use currently.
Thanks for looking into it!
Its long overdue and rather desired conversion (albeit naive one,
but this series is a good starting point). I'll go over your patches
next week with comments and concrete suggestions how to implement
particular things.

> Like Xu (9):
>   cpu/topology: add struct CpuTopology to MachineState
>   cpu/topology: add general support for machine properties
>   cpu/topology: add uncommon arch support for smp machine properties
>   cpu/topology: add ARM support for smp machine properties
>   cpu/topology: add i386 support for smp machine properties
>   cpu/topology: add PPC support for smp machine properties
>   cpu/topology: add riscv support for smp machine properties
>   cpu/topology: add s390x support for smp machine properties
>   cpu/topology: replace smp global variables with machine propertie
> 
>  accel/kvm/kvm-all.c          |  3 +++
>  backends/hostmem.c           |  4 ++++
>  cpus.c                       |  4 ++++
>  exec.c                       |  2 ++
>  gdbstub.c                    |  7 ++++++-
>  hw/alpha/dp264.c             |  1 +
>  hw/arm/fsl-imx6.c            |  5 +++++
>  hw/arm/fsl-imx6ul.c          |  5 +++++
>  hw/arm/fsl-imx7.c            |  5 +++++
>  hw/arm/highbank.c            |  1 +
>  hw/arm/mcimx6ul-evk.c        |  1 +
>  hw/arm/mcimx7d-sabre.c       |  3 +++
>  hw/arm/raspi.c               |  2 ++
>  hw/arm/realview.c            |  1 +
>  hw/arm/sabrelite.c           |  1 +
>  hw/arm/vexpress.c            |  3 +++
>  hw/arm/virt.c                |  7 +++++++
>  hw/arm/xlnx-zynqmp.c         |  7 +++++++
>  hw/cpu/core.c                |  3 +++
>  hw/hppa/machine.c            |  4 ++++
>  hw/i386/acpi-build.c         |  3 +++
>  hw/i386/kvmvapic.c           |  5 +++++
>  hw/i386/pc.c                 | 12 +++++++++++
>  hw/mips/boston.c             |  1 +
>  hw/mips/mips_malta.c         |  9 +++++++++
>  hw/openrisc/openrisc_sim.c   |  1 +
>  hw/ppc/e500.c                |  3 +++
>  hw/ppc/mac_newworld.c        |  2 ++
>  hw/ppc/mac_oldworld.c        |  2 ++
>  hw/ppc/pnv.c                 |  3 +++
>  hw/ppc/prep.c                |  2 ++
>  hw/ppc/spapr.c               | 29 ++++++++++++++++++++++++++
>  hw/ppc/spapr_rtas.c          |  3 +++
>  hw/riscv/sifive_e.c          |  4 ++++
>  hw/riscv/sifive_plic.c       |  3 +++
>  hw/riscv/sifive_u.c          |  4 ++++
>  hw/riscv/spike.c             |  2 ++
>  hw/riscv/virt.c              |  1 +
>  hw/s390x/s390-virtio-ccw.c   |  2 ++
>  hw/s390x/sclp.c              |  1 +
>  hw/smbios/smbios.c           | 11 ++++++++++
>  hw/sparc/sun4m.c             |  2 ++
>  hw/sparc64/sun4u.c           |  2 ++
>  hw/xtensa/sim.c              |  1 +
>  hw/xtensa/xtfpga.c           |  1 +
>  include/hw/arm/virt.h        |  2 +-
>  include/hw/boards.h          |  8 ++++++++
>  include/sysemu/sysemu.h      |  2 +-
>  migration/postcopy-ram.c     |  7 +++++++
>  numa.c                       |  1 +
>  target/arm/cpu.c             |  7 +++++++
>  target/i386/cpu.c            |  4 ++++
>  target/openrisc/sys_helper.c |  5 +++++
>  target/s390x/cpu.c           |  3 +++
>  target/s390x/excp_helper.c   |  6 ++++++
>  tcg/tcg.c                    | 15 ++++++++++++++
>  vl.c                         | 48 ++++++++++++++++++++++++--------------------
>  57 files changed, 261 insertions(+), 25 deletions(-)
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Philippe Mathieu-Daudé]

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

Le ven. 29 mars 2019 10:27, Alex Bennée <alex.bennee@linaro.org> a écrit :

>
> Like Xu <like.xu@linux.intel.com> writes:
>
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > ---
> >  hw/arm/fsl-imx6.c      | 5 +++++
> >  hw/arm/fsl-imx6ul.c    | 5 +++++
> >  hw/arm/fsl-imx7.c      | 5 +++++
> >  hw/arm/highbank.c      | 1 +
> >  hw/arm/mcimx6ul-evk.c  | 1 +
> >  hw/arm/mcimx7d-sabre.c | 3 +++
> >  hw/arm/raspi.c         | 2 ++
> >  hw/arm/realview.c      | 1 +
> >  hw/arm/sabrelite.c     | 1 +
> >  hw/arm/vexpress.c      | 3 +++
> >  hw/arm/virt.c          | 7 +++++++
> >  hw/arm/xlnx-zynqmp.c   | 7 +++++++
> >  target/arm/cpu.c       | 7 +++++++
> >  13 files changed, 48 insertions(+)
> >
> > diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> > index 7b7b97f..efed3a4 100644
> > --- a/hw/arm/fsl-imx6.c
> > +++ b/hw/arm/fsl-imx6.c
> > @@ -23,6 +23,7 @@
> >  #include "qapi/error.h"
> >  #include "qemu-common.h"
> >  #include "hw/arm/fsl-imx6.h"
> > +#include "hw/boards.h"
> >  #include "sysemu/sysemu.h"
> >  #include "chardev/char.h"
> >  #include "qemu/error-report.h"
> > @@ -33,9 +34,11 @@
> >
> >  static void fsl_imx6_init(Object *obj)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      FslIMX6State *s = FSL_IMX6(obj);
> >      char name[NAME_SIZE];
> >      int i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
> >          snprintf(name, NAME_SIZE, "cpu%d", i);
> > @@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
> >
> >  static void fsl_imx6_realize(DeviceState *dev, Error **errp)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      FslIMX6State *s = FSL_IMX6(dev);
> >      uint16_t i;
> >      Error *err = NULL;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      if (smp_cpus > FSL_IMX6_NUM_CPUS) {
> >          error_setg(errp, "%s: Only %d CPUs are supported (%d
> requested)",
> > diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> > index 4b56bfa..426bf8e 100644
> > --- a/hw/arm/fsl-imx6ul.c
> > +++ b/hw/arm/fsl-imx6ul.c
> > @@ -23,14 +23,17 @@
> >  #include "hw/misc/unimp.h"
> >  #include "sysemu/sysemu.h"
> >  #include "qemu/error-report.h"
> > +#include "hw/boards.h"
> >
> >  #define NAME_SIZE 20
> >
> >  static void fsl_imx6ul_init(Object *obj)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      FslIMX6ULState *s = FSL_IMX6UL(obj);
> >      char name[NAME_SIZE];
> >      int i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
> >          snprintf(name, NAME_SIZE, "cpu%d", i);
> > @@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
> >
> >  static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      FslIMX6ULState *s = FSL_IMX6UL(dev);
> >      int i;
> >      qemu_irq irq;
> >      char name[NAME_SIZE];
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
> >          error_setg(errp, "%s: Only %d CPUs are supported (%d
> requested)",
> > diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> > index 7663ad6..b759f7b 100644
> > --- a/hw/arm/fsl-imx7.c
> > +++ b/hw/arm/fsl-imx7.c
> > @@ -25,11 +25,14 @@
> >  #include "hw/misc/unimp.h"
> >  #include "sysemu/sysemu.h"
> >  #include "qemu/error-report.h"
> > +#include "hw/boards.h"
> >
> >  #define NAME_SIZE 20
> >
> >  static void fsl_imx7_init(Object *obj)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      FslIMX7State *s = FSL_IMX7(obj);
> >      char name[NAME_SIZE];
> >      int i;
> > @@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
> >
> >  static void fsl_imx7_realize(DeviceState *dev, Error **errp)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      FslIMX7State *s = FSL_IMX7(dev);
> >      Object *o;
> >      int i;
> >      qemu_irq irq;
> >      char name[NAME_SIZE];
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      if (smp_cpus > FSL_IMX7_NUM_CPUS) {
> >          error_setg(errp, "%s: Only %d CPUs are supported (%d
> requested)",
> > diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> > index 96ccf18..58c77f6 100644
> > --- a/hw/arm/highbank.c
> > +++ b/hw/arm/highbank.c
> > @@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum
> cxmachines machine_id)
> >      SysBusDevice *busdev;
> >      qemu_irq pic[128];
> >      int n;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >      qemu_irq cpu_irq[4];
> >      qemu_irq cpu_fiq[4];
> >      qemu_irq cpu_virq[4];
> > diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
> > index fb2b015..29fb706 100644
> > --- a/hw/arm/mcimx6ul-evk.c
> > +++ b/hw/arm/mcimx6ul-evk.c
> > @@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
> >      static struct arm_boot_info boot_info;
> >      MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
> >      int i;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >
> >      if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
> >          error_report("RAM size " RAM_ADDR_FMT " above max supported
> (%08x)",
> > diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
> > index 9c5f0e7..c2cdaf9 100644
> > --- a/hw/arm/mcimx7d-sabre.c
> > +++ b/hw/arm/mcimx7d-sabre.c
> > @@ -20,6 +20,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "qemu/error-report.h"
> >  #include "sysemu/qtest.h"
> > +#include "hw/boards.h"
> >
> >  typedef struct {
> >      FslIMX7State soc;
> > @@ -28,10 +29,12 @@ typedef struct {
> >
> >  static void mcimx7d_sabre_init(MachineState *machine)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
>
> Isn't this redundant? Doesn't ms == machine?
>
> >      static struct arm_boot_info boot_info;
> >      MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
> >      Object *soc;
> >      int i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
> >          error_report("RAM size " RAM_ADDR_FMT " above max supported
> (%08x)",
> > diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> > index 66899c2..8ab8e10 100644
> > --- a/hw/arm/raspi.c
> > +++ b/hw/arm/raspi.c
> > @@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int
> version, size_t ram_size)
> >  {
> >      static struct arm_boot_info binfo;
> >      int r;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >
> >      binfo.board_id = raspi_boardid[version];
> >      binfo.ram_size = ram_size;
> > @@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int
> version)
> >      BlockBackend *blk;
> >      BusState *bus;
> >      DeviceState *carddev;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >
> >      object_initialize(&s->soc, sizeof(s->soc),
> >                        version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
> > diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> > index 242f5a8..517c275 100644
> > --- a/hw/arm/realview.c
> > +++ b/hw/arm/realview.c
> > @@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
> >      NICInfo *nd;
> >      I2CBus *i2c;
> >      int n;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >      int done_nic = 0;
> >      qemu_irq cpu_irq[4];
> >      int is_mpcore = 0;
> > diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
> > index ee140e5..108faab 100644
> > --- a/hw/arm/sabrelite.c
> > +++ b/hw/arm/sabrelite.c
> > @@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
> >  {
> >      IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
> >      Error *err = NULL;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >
> >      /* Check the amount of memory is compatible with the SOC */
> >      if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
> > diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> > index f07134c..1927d12 100644
> > --- a/hw/arm/vexpress.c
> > +++ b/hw/arm/vexpress.c
> > @@ -206,9 +206,11 @@ struct VEDBoardInfo {
> >  static void init_cpus(const char *cpu_type, const char *privdev,
> >                        hwaddr periphbase, qemu_irq *pic, bool secure,
> bool virt)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      DeviceState *dev;
> >      SysBusDevice *busdev;
> >      int n;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      /* Create the actual CPUs */
> >      for (n = 0; n < smp_cpus; n++) {
> > @@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState
> *machine)
> >      MemoryRegion *flash0mem;
> >      const hwaddr *map = daughterboard->motherboard_map;
> >      int i;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >
> >      daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ce2664a..ff1dff3 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms,
> qemu_irq *pic)
> >
> >  static void create_gic(VirtMachineState *vms, qemu_irq *pic)
> >  {
> > +    MachineState *ms = MACHINE(vms);
> >      /* We create a standalone GIC */
> >      DeviceState *gicdev;
> >      SysBusDevice *gicbusdev;
> >      const char *gictype;
> >      int type = vms->gic_version, i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      uint32_t nb_redist_regions = 0;
> >
> >      gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
> > @@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState
> *vms,
> >
> >  static FWCfgState *create_fw_cfg(const VirtMachineState *vms,
> AddressSpace *as)
> >  {
> > +    MachineState *ms = MACHINE(vms);
> >      hwaddr base = vms->memmap[VIRT_FW_CFG].base;
> >      hwaddr size = vms->memmap[VIRT_FW_CFG].size;
> >      FWCfgState *fw_cfg;
> >      char *nodename;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >
> >      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);
> > @@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
> >      MemoryRegion *ram = g_new(MemoryRegion, 1);
> >      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> >      bool aarch64 = true;
> > +    unsigned int smp_cpus = machine->topo.smp_cpus;
> > +    unsigned int max_cpus = machine->topo.max_cpus;
> >
> >      /*
> >       * In accelerated mode, the memory map is computed earlier in
> kvm_type()
> > @@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const
> MachineState *ms, int idx)
> >  static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
> >  {
> >      int n;
> > +    unsigned int max_cpus = ms->topo.max_cpus;
> >      VirtMachineState *vms = VIRT_MACHINE(ms);
> >
> >      if (ms->possible_cpus) {
> > diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> > index 4f8bc41..29b9427 100644
> > --- a/hw/arm/xlnx-zynqmp.c
> > +++ b/hw/arm/xlnx-zynqmp.c
> > @@ -19,6 +19,7 @@
> >  #include "qapi/error.h"
> >  #include "qemu-common.h"
> >  #include "cpu.h"
> > +#include "hw/boards.h"
> >  #include "hw/arm/xlnx-zynqmp.h"
> >  #include "hw/intc/arm_gic_common.h"
> >  #include "exec/address-spaces.h"
> > @@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int
> ppi_index)
> >  static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char
> *boot_cpu,
> >                                     Error **errp)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      Error *err = NULL;
> >      int i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
> XLNX_ZYNQMP_NUM_RPU_CPUS);
> >
> >      if (num_rpus <= 0) {
> > @@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState
> *s, const char *boot_cpu,
> >
> >  static void xlnx_zynqmp_init(Object *obj)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
> >      int i;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
> >
> >      object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
> > @@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
> >
> >  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
> >      MemoryRegion *system_memory = get_system_memory();
> >      uint8_t i;
> >      uint64_t ram_size;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
> >      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
> >      ram_addr_t ddr_low_size, ddr_high_size;
> > diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> > index 4155782..e5d7e3b 100644
> > --- a/target/arm/cpu.c
> > +++ b/target/arm/cpu.c
> > @@ -30,6 +30,7 @@
> >  #include "hw/qdev-properties.h"
> >  #if !defined(CONFIG_USER_ONLY)
> >  #include "hw/loader.h"
> > +#include "hw/boards.h"
> >  #endif
> >  #include "hw/arm/arm.h"
> >  #include "sysemu/sysemu.h"
> > @@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev,
> Error **errp)
> >      init_cpreg_list(cpu);
> >
> >  #ifndef CONFIG_USER_ONLY
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> > +
> >      if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> >          cs->num_ases = 2;
> >
> > @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >  static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo
> *ri)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
>
> How expensive is qdev_get_machine? This could potentially be a
> performance issue if this register is read a lot.
>

At this point this number is constant and accessible somehow via CPUARMState,
right?


> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> > +
> >      /* Linux wants the number of processors from here.
> >       * Might as well set the interrupt-controller bit too.
> >       */
>
>
> --
> Alex Bennée
>
>

[-- Attachment #2: Type: text/html, Size: 18606 bytes --]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/9] cpu/topology: add struct CpuTopology to MachineState

[Like Xu]

On 2019/3/29 17:22, Alex Bennée wrote:
> 
> Like Xu <like.xu@linux.intel.com> writes:
> 
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>   include/hw/arm/virt.h   | 2 +-
>>   include/hw/boards.h     | 8 ++++++++
>>   include/sysemu/sysemu.h | 2 +-
>>   vl.c                    | 7 ++++++-
>>   4 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
>> index 507517c..724da0c 100644
>> --- a/include/hw/arm/virt.h
>> +++ b/include/hw/arm/virt.h
>> @@ -123,7 +123,7 @@ typedef struct {
>>       struct arm_boot_info bootinfo;
>>       MemMapEntry *memmap;
>>       const int *irqmap;
>> -    int smp_cpus;
>> +    unsigned int smp_cpus;
> 
> This seem unrelated to the rest of the change.
Just by the way to keep structural consistency.
> 
>>       void *fdt;
>>       int fdt_size;
> <snip>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 6065d9e..c0d7d7c 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -100,7 +100,7 @@ extern const char *keyboard_layout;
>>   extern int win2k_install_hack;
>>   extern int alt_grab;
>>   extern int ctrl_grab;
>> -extern int smp_cpus;
>> +extern unsigned int smp_cpus;
> 
> Along with this. Is this meant to align with what is currently defined
> as smp_cpus in vl.c? Maybe these changes should be rolled together with
> that before the patch to introduce the new topology structure?
At that time (just patch 1/9 is applied), it's align with what is
defined as smp_cpus in vl.c and these exposed smp variables would be
removed in an appropriate way after machine properties is fully enabled.

> 
> --
> Alex Bennée
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 7/9] cpu/topology: add riscv support for smp machine properties

[Like Xu]

On 2019/3/30 7:01, Alistair Francis wrote:
> On Fri, Mar 29, 2019 at 1:59 AM Like Xu <like.xu@linux.intel.com> wrote:
>>
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>   hw/openrisc/openrisc_sim.c | 1 +
>>   hw/riscv/sifive_e.c        | 4 ++++
>>   hw/riscv/sifive_plic.c     | 3 +++
>>   hw/riscv/sifive_u.c        | 4 ++++
>>   hw/riscv/spike.c           | 2 ++
>>   hw/riscv/virt.c            | 1 +
>>   6 files changed, 15 insertions(+)
>>
>> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
>> index 7d3b734..ecfc973 100644
>> --- a/hw/openrisc/openrisc_sim.c
>> +++ b/hw/openrisc/openrisc_sim.c
>> @@ -131,6 +131,7 @@ static void openrisc_sim_init(MachineState *machine)
>>       qemu_irq *cpu_irqs[2];
>>       qemu_irq serial_irq;
>>       int n;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> 
> OpenRISC and RISC-V are not the same thing, it's probably worth
> splitting this out into a separate patch.
> 
> Alistair
You're right and thanks.
I may fix it in next version.

> 
>>
>>       for (n = 0; n < smp_cpus; n++) {
>>           cpu = OPENRISC_CPU(cpu_create(machine->cpu_type));
>> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
>> index b1cd113..c65c7b5 100644
>> --- a/hw/riscv/sifive_e.c
>> +++ b/hw/riscv/sifive_e.c
>> @@ -137,6 +137,8 @@ static void riscv_sifive_e_init(MachineState *machine)
>>
>>   static void riscv_sifive_e_soc_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       SiFiveESoCState *s = RISCV_E_SOC(obj);
>>
>>       object_initialize_child(obj, "cpus", &s->cpus,
>> @@ -150,6 +152,8 @@ static void riscv_sifive_e_soc_init(Object *obj)
>>
>>   static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       const struct MemmapEntry *memmap = sifive_e_memmap;
>>
>>       SiFiveESoCState *s = RISCV_E_SOC(dev);
>> diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
>> index ac768e6..161fbd9 100644
>> --- a/hw/riscv/sifive_plic.c
>> +++ b/hw/riscv/sifive_plic.c
>> @@ -23,6 +23,7 @@
>>   #include "qemu/error-report.h"
>>   #include "hw/sysbus.h"
>>   #include "hw/pci/msi.h"
>> +#include "hw/boards.h"
>>   #include "target/riscv/cpu.h"
>>   #include "sysemu/sysemu.h"
>>   #include "hw/riscv/sifive_plic.h"
>> @@ -432,6 +433,8 @@ static void sifive_plic_irq_request(void *opaque, int irq, int level)
>>
>>   static void sifive_plic_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       SiFivePLICState *plic = SIFIVE_PLIC(dev);
>>       int i;
>>
>> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
>> index 5ecc47c..b4a8d66 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -321,6 +321,8 @@ static void riscv_sifive_u_init(MachineState *machine)
>>
>>   static void riscv_sifive_u_soc_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       SiFiveUSoCState *s = RISCV_U_SOC(obj);
>>
>>       object_initialize_child(obj, "cpus", &s->cpus, sizeof(s->cpus),
>> @@ -336,6 +338,8 @@ static void riscv_sifive_u_soc_init(Object *obj)
>>
>>   static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       SiFiveUSoCState *s = RISCV_U_SOC(dev);
>>       const struct MemmapEntry *memmap = sifive_u_memmap;
>>       MemoryRegion *system_memory = get_system_memory();
>> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
>> index 2a000a5..5fe441c 100644
>> --- a/hw/riscv/spike.c
>> +++ b/hw/riscv/spike.c
>> @@ -171,6 +171,7 @@ static void spike_v1_10_0_board_init(MachineState *machine)
>>       MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>>       MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>       int i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       /* Initialize SOC */
>>       object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
>> @@ -253,6 +254,7 @@ static void spike_v1_09_1_board_init(MachineState *machine)
>>       MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>>       MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>>       int i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       /* Initialize SOC */
>>       object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index fc4c6b3..9d4d305 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -395,6 +395,7 @@ static void riscv_virt_board_init(MachineState *machine)
>>       char *plic_hart_config;
>>       size_t plic_hart_config_len;
>>       int i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>       void *fdt;
>>
>>       /* Initialize SOC */
>> --
>> 1.8.3.1
>>
>>
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Like Xu]

On 2019/3/29 19:20, Philippe Mathieu-Daudé wrote:
> Le ven. 29 mars 2019 10:27, Alex Bennée <alex.bennee@linaro.org> a écrit :
> 
>>
>> Like Xu <like.xu@linux.intel.com> writes:
>>
>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>> ---
>>>   hw/arm/fsl-imx6.c      | 5 +++++
>>>   hw/arm/fsl-imx6ul.c    | 5 +++++
>>>   hw/arm/fsl-imx7.c      | 5 +++++
>>>   hw/arm/highbank.c      | 1 +
>>>   hw/arm/mcimx6ul-evk.c  | 1 +
>>>   hw/arm/mcimx7d-sabre.c | 3 +++
>>>   hw/arm/raspi.c         | 2 ++
>>>   hw/arm/realview.c      | 1 +
>>>   hw/arm/sabrelite.c     | 1 +
>>>   hw/arm/vexpress.c      | 3 +++
>>>   hw/arm/virt.c          | 7 +++++++
>>>   hw/arm/xlnx-zynqmp.c   | 7 +++++++
>>>   target/arm/cpu.c       | 7 +++++++
>>>   13 files changed, 48 insertions(+)
>>>
>>> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
>>> index 7b7b97f..efed3a4 100644
>>> --- a/hw/arm/fsl-imx6.c
>>> +++ b/hw/arm/fsl-imx6.c
>>> @@ -23,6 +23,7 @@
>>>   #include "qapi/error.h"
>>>   #include "qemu-common.h"
>>>   #include "hw/arm/fsl-imx6.h"
>>> +#include "hw/boards.h"
>>>   #include "sysemu/sysemu.h"
>>>   #include "chardev/char.h"
>>>   #include "qemu/error-report.h"
>>> @@ -33,9 +34,11 @@
>>>
>>>   static void fsl_imx6_init(Object *obj)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       FslIMX6State *s = FSL_IMX6(obj);
>>>       char name[NAME_SIZE];
>>>       int i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
>>>           snprintf(name, NAME_SIZE, "cpu%d", i);
>>> @@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
>>>
>>>   static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       FslIMX6State *s = FSL_IMX6(dev);
>>>       uint16_t i;
>>>       Error *err = NULL;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       if (smp_cpus > FSL_IMX6_NUM_CPUS) {
>>>           error_setg(errp, "%s: Only %d CPUs are supported (%d
>> requested)",
>>> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
>>> index 4b56bfa..426bf8e 100644
>>> --- a/hw/arm/fsl-imx6ul.c
>>> +++ b/hw/arm/fsl-imx6ul.c
>>> @@ -23,14 +23,17 @@
>>>   #include "hw/misc/unimp.h"
>>>   #include "sysemu/sysemu.h"
>>>   #include "qemu/error-report.h"
>>> +#include "hw/boards.h"
>>>
>>>   #define NAME_SIZE 20
>>>
>>>   static void fsl_imx6ul_init(Object *obj)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       FslIMX6ULState *s = FSL_IMX6UL(obj);
>>>       char name[NAME_SIZE];
>>>       int i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>>>           snprintf(name, NAME_SIZE, "cpu%d", i);
>>> @@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
>>>
>>>   static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       FslIMX6ULState *s = FSL_IMX6UL(dev);
>>>       int i;
>>>       qemu_irq irq;
>>>       char name[NAME_SIZE];
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
>>>           error_setg(errp, "%s: Only %d CPUs are supported (%d
>> requested)",
>>> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
>>> index 7663ad6..b759f7b 100644
>>> --- a/hw/arm/fsl-imx7.c
>>> +++ b/hw/arm/fsl-imx7.c
>>> @@ -25,11 +25,14 @@
>>>   #include "hw/misc/unimp.h"
>>>   #include "sysemu/sysemu.h"
>>>   #include "qemu/error-report.h"
>>> +#include "hw/boards.h"
>>>
>>>   #define NAME_SIZE 20
>>>
>>>   static void fsl_imx7_init(Object *obj)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>       FslIMX7State *s = FSL_IMX7(obj);
>>>       char name[NAME_SIZE];
>>>       int i;
>>> @@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
>>>
>>>   static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       FslIMX7State *s = FSL_IMX7(dev);
>>>       Object *o;
>>>       int i;
>>>       qemu_irq irq;
>>>       char name[NAME_SIZE];
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       if (smp_cpus > FSL_IMX7_NUM_CPUS) {
>>>           error_setg(errp, "%s: Only %d CPUs are supported (%d
>> requested)",
>>> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
>>> index 96ccf18..58c77f6 100644
>>> --- a/hw/arm/highbank.c
>>> +++ b/hw/arm/highbank.c
>>> @@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum
>> cxmachines machine_id)
>>>       SysBusDevice *busdev;
>>>       qemu_irq pic[128];
>>>       int n;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>       qemu_irq cpu_irq[4];
>>>       qemu_irq cpu_fiq[4];
>>>       qemu_irq cpu_virq[4];
>>> diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
>>> index fb2b015..29fb706 100644
>>> --- a/hw/arm/mcimx6ul-evk.c
>>> +++ b/hw/arm/mcimx6ul-evk.c
>>> @@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
>>>       static struct arm_boot_info boot_info;
>>>       MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
>>>       int i;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>
>>>       if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
>>>           error_report("RAM size " RAM_ADDR_FMT " above max supported
>> (%08x)",
>>> diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
>>> index 9c5f0e7..c2cdaf9 100644
>>> --- a/hw/arm/mcimx7d-sabre.c
>>> +++ b/hw/arm/mcimx7d-sabre.c
>>> @@ -20,6 +20,7 @@
>>>   #include "sysemu/sysemu.h"
>>>   #include "qemu/error-report.h"
>>>   #include "sysemu/qtest.h"
>>> +#include "hw/boards.h"
>>>
>>>   typedef struct {
>>>       FslIMX7State soc;
>>> @@ -28,10 +29,12 @@ typedef struct {
>>>
>>>   static void mcimx7d_sabre_init(MachineState *machine)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>
>> Isn't this redundant? Doesn't ms == machine?
>>
>>>       static struct arm_boot_info boot_info;
>>>       MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
>>>       Object *soc;
>>>       int i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
>>>           error_report("RAM size " RAM_ADDR_FMT " above max supported
>> (%08x)",
>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>> index 66899c2..8ab8e10 100644
>>> --- a/hw/arm/raspi.c
>>> +++ b/hw/arm/raspi.c
>>> @@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int
>> version, size_t ram_size)
>>>   {
>>>       static struct arm_boot_info binfo;
>>>       int r;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>
>>>       binfo.board_id = raspi_boardid[version];
>>>       binfo.ram_size = ram_size;
>>> @@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int
>> version)
>>>       BlockBackend *blk;
>>>       BusState *bus;
>>>       DeviceState *carddev;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>
>>>       object_initialize(&s->soc, sizeof(s->soc),
>>>                         version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
>>> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
>>> index 242f5a8..517c275 100644
>>> --- a/hw/arm/realview.c
>>> +++ b/hw/arm/realview.c
>>> @@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
>>>       NICInfo *nd;
>>>       I2CBus *i2c;
>>>       int n;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>       int done_nic = 0;
>>>       qemu_irq cpu_irq[4];
>>>       int is_mpcore = 0;
>>> diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
>>> index ee140e5..108faab 100644
>>> --- a/hw/arm/sabrelite.c
>>> +++ b/hw/arm/sabrelite.c
>>> @@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
>>>   {
>>>       IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
>>>       Error *err = NULL;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>
>>>       /* Check the amount of memory is compatible with the SOC */
>>>       if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
>>> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
>>> index f07134c..1927d12 100644
>>> --- a/hw/arm/vexpress.c
>>> +++ b/hw/arm/vexpress.c
>>> @@ -206,9 +206,11 @@ struct VEDBoardInfo {
>>>   static void init_cpus(const char *cpu_type, const char *privdev,
>>>                         hwaddr periphbase, qemu_irq *pic, bool secure,
>> bool virt)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       DeviceState *dev;
>>>       SysBusDevice *busdev;
>>>       int n;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       /* Create the actual CPUs */
>>>       for (n = 0; n < smp_cpus; n++) {
>>> @@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState
>> *machine)
>>>       MemoryRegion *flash0mem;
>>>       const hwaddr *map = daughterboard->motherboard_map;
>>>       int i;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>>
>>>       daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index ce2664a..ff1dff3 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms,
>> qemu_irq *pic)
>>>
>>>   static void create_gic(VirtMachineState *vms, qemu_irq *pic)
>>>   {
>>> +    MachineState *ms = MACHINE(vms);
>>>       /* We create a standalone GIC */
>>>       DeviceState *gicdev;
>>>       SysBusDevice *gicbusdev;
>>>       const char *gictype;
>>>       int type = vms->gic_version, i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>       uint32_t nb_redist_regions = 0;
>>>
>>>       gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
>>> @@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState
>> *vms,
>>>
>>>   static FWCfgState *create_fw_cfg(const VirtMachineState *vms,
>> AddressSpace *as)
>>>   {
>>> +    MachineState *ms = MACHINE(vms);
>>>       hwaddr base = vms->memmap[VIRT_FW_CFG].base;
>>>       hwaddr size = vms->memmap[VIRT_FW_CFG].size;
>>>       FWCfgState *fw_cfg;
>>>       char *nodename;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>
>>>       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);
>>> @@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
>>>       MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>       bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>>>       bool aarch64 = true;
>>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>> +    unsigned int max_cpus = machine->topo.max_cpus;
>>>
>>>       /*
>>>        * In accelerated mode, the memory map is computed earlier in
>> kvm_type()
>>> @@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const
>> MachineState *ms, int idx)
>>>   static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>>>   {
>>>       int n;
>>> +    unsigned int max_cpus = ms->topo.max_cpus;
>>>       VirtMachineState *vms = VIRT_MACHINE(ms);
>>>
>>>       if (ms->possible_cpus) {
>>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>>> index 4f8bc41..29b9427 100644
>>> --- a/hw/arm/xlnx-zynqmp.c
>>> +++ b/hw/arm/xlnx-zynqmp.c
>>> @@ -19,6 +19,7 @@
>>>   #include "qapi/error.h"
>>>   #include "qemu-common.h"
>>>   #include "cpu.h"
>>> +#include "hw/boards.h"
>>>   #include "hw/arm/xlnx-zynqmp.h"
>>>   #include "hw/intc/arm_gic_common.h"
>>>   #include "exec/address-spaces.h"
>>> @@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int
>> ppi_index)
>>>   static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char
>> *boot_cpu,
>>>                                      Error **errp)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       Error *err = NULL;
>>>       int i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>       int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS,
>> XLNX_ZYNQMP_NUM_RPU_CPUS);
>>>
>>>       if (num_rpus <= 0) {
>>> @@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState
>> *s, const char *boot_cpu,
>>>
>>>   static void xlnx_zynqmp_init(Object *obj)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>>>       int i;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>       int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>>
>>>       object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
>>> @@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
>>>
>>>   static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>       XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
>>>       MemoryRegion *system_memory = get_system_memory();
>>>       uint8_t i;
>>>       uint64_t ram_size;
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>>       int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>>       const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>>>       ram_addr_t ddr_low_size, ddr_high_size;
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> index 4155782..e5d7e3b 100644
>>> --- a/target/arm/cpu.c
>>> +++ b/target/arm/cpu.c
>>> @@ -30,6 +30,7 @@
>>>   #include "hw/qdev-properties.h"
>>>   #if !defined(CONFIG_USER_ONLY)
>>>   #include "hw/loader.h"
>>> +#include "hw/boards.h"
>>>   #endif
>>>   #include "hw/arm/arm.h"
>>>   #include "sysemu/sysemu.h"
>>> @@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev,
>> Error **errp)
>>>       init_cpreg_list(cpu);
>>>
>>>   #ifndef CONFIG_USER_ONLY
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>> +
>>>       if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>>>           cs->num_ases = 2;
>>>
>>> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>>>   #ifndef CONFIG_USER_ONLY
>>>   static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo
>> *ri)
>>>   {
>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>
>> How expensive is qdev_get_machine? This could potentially be a
>> performance issue if this register is read a lot.
>>
> 
> At this point this number is constant and accessible somehow via CPUARMState,
> right?

I'm not sure if all of smp variables are constant for arm.

I haven't see the offset relation between MachineState and CPUARMState
and it's recommended to access smp properties via MachineState.

> 
> 
>>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>> +
>>>       /* Linux wants the number of processors from here.
>>>        * Might as well set the interrupt-controller bit too.
>>>        */
>>
>>
>> --
>> Alex Bennée
>>
>>
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Like Xu]

On 2019/3/29 17:27, Alex Bennée wrote:
> 
> Like Xu <like.xu@linux.intel.com> writes:
> 
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>> ---
>>   hw/arm/fsl-imx6.c      | 5 +++++
>>   hw/arm/fsl-imx6ul.c    | 5 +++++
>>   hw/arm/fsl-imx7.c      | 5 +++++
>>   hw/arm/highbank.c      | 1 +
>>   hw/arm/mcimx6ul-evk.c  | 1 +
>>   hw/arm/mcimx7d-sabre.c | 3 +++
>>   hw/arm/raspi.c         | 2 ++
>>   hw/arm/realview.c      | 1 +
>>   hw/arm/sabrelite.c     | 1 +
>>   hw/arm/vexpress.c      | 3 +++
>>   hw/arm/virt.c          | 7 +++++++
>>   hw/arm/xlnx-zynqmp.c   | 7 +++++++
>>   target/arm/cpu.c       | 7 +++++++
>>   13 files changed, 48 insertions(+)
>>
>> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
>> index 7b7b97f..efed3a4 100644
>> --- a/hw/arm/fsl-imx6.c
>> +++ b/hw/arm/fsl-imx6.c
>> @@ -23,6 +23,7 @@
>>   #include "qapi/error.h"
>>   #include "qemu-common.h"
>>   #include "hw/arm/fsl-imx6.h"
>> +#include "hw/boards.h"
>>   #include "sysemu/sysemu.h"
>>   #include "chardev/char.h"
>>   #include "qemu/error-report.h"
>> @@ -33,9 +34,11 @@
>>
>>   static void fsl_imx6_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       FslIMX6State *s = FSL_IMX6(obj);
>>       char name[NAME_SIZE];
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
>>           snprintf(name, NAME_SIZE, "cpu%d", i);
>> @@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
>>
>>   static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       FslIMX6State *s = FSL_IMX6(dev);
>>       uint16_t i;
>>       Error *err = NULL;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       if (smp_cpus > FSL_IMX6_NUM_CPUS) {
>>           error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
>> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
>> index 4b56bfa..426bf8e 100644
>> --- a/hw/arm/fsl-imx6ul.c
>> +++ b/hw/arm/fsl-imx6ul.c
>> @@ -23,14 +23,17 @@
>>   #include "hw/misc/unimp.h"
>>   #include "sysemu/sysemu.h"
>>   #include "qemu/error-report.h"
>> +#include "hw/boards.h"
>>
>>   #define NAME_SIZE 20
>>
>>   static void fsl_imx6ul_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       FslIMX6ULState *s = FSL_IMX6UL(obj);
>>       char name[NAME_SIZE];
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>>           snprintf(name, NAME_SIZE, "cpu%d", i);
>> @@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
>>
>>   static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       FslIMX6ULState *s = FSL_IMX6UL(dev);
>>       int i;
>>       qemu_irq irq;
>>       char name[NAME_SIZE];
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
>>           error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
>> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
>> index 7663ad6..b759f7b 100644
>> --- a/hw/arm/fsl-imx7.c
>> +++ b/hw/arm/fsl-imx7.c
>> @@ -25,11 +25,14 @@
>>   #include "hw/misc/unimp.h"
>>   #include "sysemu/sysemu.h"
>>   #include "qemu/error-report.h"
>> +#include "hw/boards.h"
>>
>>   #define NAME_SIZE 20
>>
>>   static void fsl_imx7_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       FslIMX7State *s = FSL_IMX7(obj);
>>       char name[NAME_SIZE];
>>       int i;
>> @@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
>>
>>   static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       FslIMX7State *s = FSL_IMX7(dev);
>>       Object *o;
>>       int i;
>>       qemu_irq irq;
>>       char name[NAME_SIZE];
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       if (smp_cpus > FSL_IMX7_NUM_CPUS) {
>>           error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
>> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
>> index 96ccf18..58c77f6 100644
>> --- a/hw/arm/highbank.c
>> +++ b/hw/arm/highbank.c
>> @@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>>       SysBusDevice *busdev;
>>       qemu_irq pic[128];
>>       int n;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>       qemu_irq cpu_irq[4];
>>       qemu_irq cpu_fiq[4];
>>       qemu_irq cpu_virq[4];
>> diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
>> index fb2b015..29fb706 100644
>> --- a/hw/arm/mcimx6ul-evk.c
>> +++ b/hw/arm/mcimx6ul-evk.c
>> @@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
>>       static struct arm_boot_info boot_info;
>>       MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
>>       int i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
>>           error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
>> diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
>> index 9c5f0e7..c2cdaf9 100644
>> --- a/hw/arm/mcimx7d-sabre.c
>> +++ b/hw/arm/mcimx7d-sabre.c
>> @@ -20,6 +20,7 @@
>>   #include "sysemu/sysemu.h"
>>   #include "qemu/error-report.h"
>>   #include "sysemu/qtest.h"
>> +#include "hw/boards.h"
>>
>>   typedef struct {
>>       FslIMX7State soc;
>> @@ -28,10 +29,12 @@ typedef struct {
>>
>>   static void mcimx7d_sabre_init(MachineState *machine)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
> 
> Isn't this redundant? Doesn't ms == machine?

You're right and thanks.

> 
>>       static struct arm_boot_info boot_info;
>>       MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
>>       Object *soc;
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
>>           error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index 66899c2..8ab8e10 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>>   {
>>       static struct arm_boot_info binfo;
>>       int r;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       binfo.board_id = raspi_boardid[version];
>>       binfo.ram_size = ram_size;
>> @@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int version)
>>       BlockBackend *blk;
>>       BusState *bus;
>>       DeviceState *carddev;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       object_initialize(&s->soc, sizeof(s->soc),
>>                         version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
>> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
>> index 242f5a8..517c275 100644
>> --- a/hw/arm/realview.c
>> +++ b/hw/arm/realview.c
>> @@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
>>       NICInfo *nd;
>>       I2CBus *i2c;
>>       int n;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>       int done_nic = 0;
>>       qemu_irq cpu_irq[4];
>>       int is_mpcore = 0;
>> diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
>> index ee140e5..108faab 100644
>> --- a/hw/arm/sabrelite.c
>> +++ b/hw/arm/sabrelite.c
>> @@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
>>   {
>>       IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
>>       Error *err = NULL;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       /* Check the amount of memory is compatible with the SOC */
>>       if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
>> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
>> index f07134c..1927d12 100644
>> --- a/hw/arm/vexpress.c
>> +++ b/hw/arm/vexpress.c
>> @@ -206,9 +206,11 @@ struct VEDBoardInfo {
>>   static void init_cpus(const char *cpu_type, const char *privdev,
>>                         hwaddr periphbase, qemu_irq *pic, bool secure, bool virt)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       DeviceState *dev;
>>       SysBusDevice *busdev;
>>       int n;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       /* Create the actual CPUs */
>>       for (n = 0; n < smp_cpus; n++) {
>> @@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState *machine)
>>       MemoryRegion *flash0mem;
>>       const hwaddr *map = daughterboard->motherboard_map;
>>       int i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>
>>       daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index ce2664a..ff1dff3 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
>>
>>   static void create_gic(VirtMachineState *vms, qemu_irq *pic)
>>   {
>> +    MachineState *ms = MACHINE(vms);
>>       /* We create a standalone GIC */
>>       DeviceState *gicdev;
>>       SysBusDevice *gicbusdev;
>>       const char *gictype;
>>       int type = vms->gic_version, i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       uint32_t nb_redist_regions = 0;
>>
>>       gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
>> @@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState *vms,
>>
>>   static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
>>   {
>> +    MachineState *ms = MACHINE(vms);
>>       hwaddr base = vms->memmap[VIRT_FW_CFG].base;
>>       hwaddr size = vms->memmap[VIRT_FW_CFG].size;
>>       FWCfgState *fw_cfg;
>>       char *nodename;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>
>>       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);
>> @@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
>>       MemoryRegion *ram = g_new(MemoryRegion, 1);
>>       bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>>       bool aarch64 = true;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>> +    unsigned int max_cpus = machine->topo.max_cpus;
>>
>>       /*
>>        * In accelerated mode, the memory map is computed earlier in kvm_type()
>> @@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>>   static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>>   {
>>       int n;
>> +    unsigned int max_cpus = ms->topo.max_cpus;
>>       VirtMachineState *vms = VIRT_MACHINE(ms);
>>
>>       if (ms->possible_cpus) {
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index 4f8bc41..29b9427 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -19,6 +19,7 @@
>>   #include "qapi/error.h"
>>   #include "qemu-common.h"
>>   #include "cpu.h"
>> +#include "hw/boards.h"
>>   #include "hw/arm/xlnx-zynqmp.h"
>>   #include "hw/intc/arm_gic_common.h"
>>   #include "exec/address-spaces.h"
>> @@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
>>   static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>>                                      Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       Error *err = NULL;
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
>>
>>       if (num_rpus <= 0) {
>> @@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>>
>>   static void xlnx_zynqmp_init(Object *obj)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>
>>       object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
>> @@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
>>
>>   static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
>>       MemoryRegion *system_memory = get_system_memory();
>>       uint8_t i;
>>       uint64_t ram_size;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>>       const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>>       ram_addr_t ddr_low_size, ddr_high_size;
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index 4155782..e5d7e3b 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -30,6 +30,7 @@
>>   #include "hw/qdev-properties.h"
>>   #if !defined(CONFIG_USER_ONLY)
>>   #include "hw/loader.h"
>> +#include "hw/boards.h"
>>   #endif
>>   #include "hw/arm/arm.h"
>>   #include "sysemu/sysemu.h"
>> @@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>>       init_cpreg_list(cpu);
>>
>>   #ifndef CONFIG_USER_ONLY
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>> +
>>       if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>>           cs->num_ases = 2;
>>
>> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>>   #ifndef CONFIG_USER_ONLY
>>   static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
> 
> How expensive is qdev_get_machine? This could potentially be a
> performance issue if this register is read a lot.

It may not bother us cause the MachineState object would be feteched 
from qemu QOM at its first requested time and save it as static variable 
for incoming requests.

> 
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>> +
>>       /* Linux wants the number of processors from here.
>>        * Might as well set the interrupt-controller bit too.
>>        */
> 
> 
> --
> Alex Bennée
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Eduardo Habkost]

On Mon, Apr 01, 2019 at 10:56:30AM +0800, Like Xu wrote:
> On 2019/3/29 17:27, Alex Bennée wrote:
[...]
> > > @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
> > >   #ifndef CONFIG_USER_ONLY
> > >   static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> > >   {
> > > +    MachineState *ms = MACHINE(qdev_get_machine());
> > 
> > How expensive is qdev_get_machine? This could potentially be a
> > performance issue if this register is read a lot.
> 
> It may not bother us cause the MachineState object would be feteched from
> qemu QOM at its first requested time and save it as static variable for
> incoming requests.

We already have a current_machine variable declared in
include/hw/boards.h.  We normally avoid using it and use
MACHINE(qdev_get_machine()) instead, but I never understood why.

-- 
Eduardo

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Like Xu]

On 2019/4/2 7:38, Eduardo Habkost wrote:
> On Mon, Apr 01, 2019 at 10:56:30AM +0800, Like Xu wrote:
>> On 2019/3/29 17:27, Alex Bennée wrote:
> [...]
>>>> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>>>>    #ifndef CONFIG_USER_ONLY
>>>>    static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>>>>    {
>>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>
>>> How expensive is qdev_get_machine? This could potentially be a
>>> performance issue if this register is read a lot.
>>
>> It may not bother us cause the MachineState object would be feteched from
>> qemu QOM at its first requested time and save it as static variable for
>> incoming requests.
> 
> We already have a current_machine variable declared in
> include/hw/boards.h.  We normally avoid using it and use
> MACHINE(qdev_get_machine()) instead, but I never understood why.
>
Which one do you prefer to access smp machine properties?

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Peter Maydell]

On Tue, 2 Apr 2019 at 09:46, Like Xu <like.xu@linux.intel.com> wrote:
>
> On 2019/4/2 7:38, Eduardo Habkost wrote:
> > On Mon, Apr 01, 2019 at 10:56:30AM +0800, Like Xu wrote:
> >> On 2019/3/29 17:27, Alex Bennée wrote:
> > [...]
> >>>> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
> >>>>    #ifndef CONFIG_USER_ONLY
> >>>>    static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> >>>>    {
> >>>> +    MachineState *ms = MACHINE(qdev_get_machine());
> >>>
> >>> How expensive is qdev_get_machine? This could potentially be a
> >>> performance issue if this register is read a lot.
> >>
> >> It may not bother us cause the MachineState object would be feteched from
> >> qemu QOM at its first requested time and save it as static variable for
> >> incoming requests.
> >
> > We already have a current_machine variable declared in
> > include/hw/boards.h.  We normally avoid using it and use
> > MACHINE(qdev_get_machine()) instead, but I never understood why.
> >
> Which one do you prefer to access smp machine properties?

My suggestion would be that we use qdev_get_machine(). I think
it would be nice to make the remaining dozen or so uses of
the global current_machine outside vl.c use qdev_get_machine()
instead, and then make current_machine local to vl.c instead
of global.

thanks
-- PMM

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Like Xu]

On 2019/4/2 12:45, Peter Maydell wrote:
> On Tue, 2 Apr 2019 at 09:46, Like Xu <like.xu@linux.intel.com> wrote:
>>
>> On 2019/4/2 7:38, Eduardo Habkost wrote:
>>> On Mon, Apr 01, 2019 at 10:56:30AM +0800, Like Xu wrote:
>>>> On 2019/3/29 17:27, Alex Bennée wrote:
>>> [...]
>>>>>> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>>>>>>     #ifndef CONFIG_USER_ONLY
>>>>>>     static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>>>>>>     {
>>>>>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>>>>
>>>>> How expensive is qdev_get_machine? This could potentially be a
>>>>> performance issue if this register is read a lot.
>>>>
>>>> It may not bother us cause the MachineState object would be feteched from
>>>> qemu QOM at its first requested time and save it as static variable for
>>>> incoming requests.
>>>
>>> We already have a current_machine variable declared in
>>> include/hw/boards.h.  We normally avoid using it and use
>>> MACHINE(qdev_get_machine()) instead, but I never understood why.
>>>
>> Which one do you prefer to access smp machine properties?
> 
> My suggestion would be that we use qdev_get_machine(). I think
> it would be nice to make the remaining dozen or so uses of
> the global current_machine outside vl.c use qdev_get_machine()
> instead, and then make current_machine local to vl.c instead
> of global.
> 
> thanks
> -- PMM
> 
> 
It looks good to me and I am not sure if this refactoring should be 
added in next patch series or as a separate patch.

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Peter Maydell]

On Tue, 2 Apr 2019 at 12:20, Like Xu <like.xu@linux.intel.com> wrote:
>
> On 2019/4/2 12:45, Peter Maydell wrote:
> > My suggestion would be that we use qdev_get_machine(). I think
> > it would be nice to make the remaining dozen or so uses of
> > the global current_machine outside vl.c use qdev_get_machine()
> > instead, and then make current_machine local to vl.c instead
> > of global.

> It looks good to me and I am not sure if this refactoring should be
> added in next patch series or as a separate patch.

Changing the other uses of current_machine is definitely a
refactoring that should be a separate patch series.

thanks
-- PMM

Re: [Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Like Xu]

On 2019/3/29 18:21, Igor Mammedov wrote:
> On Fri, 29 Mar 2019 16:48:36 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 
>> This patch series make existing cores/threads/sockets into machine
>> properties and get rid of global variables they use currently.
> Thanks for looking into it!
> Its long overdue and rather desired conversion (albeit naive one,
> but this series is a good starting point). I'll go over your patches
> next week with comments and concrete suggestions how to implement
> particular things.

Hi Igor, any comments and suggestions on smp machine properties
in this patch considering we may add die topology for PCMachine as an 
extension?

> 
>> Like Xu (9):
>>    cpu/topology: add struct CpuTopology to MachineState
>>    cpu/topology: add general support for machine properties
>>    cpu/topology: add uncommon arch support for smp machine properties
>>    cpu/topology: add ARM support for smp machine properties
>>    cpu/topology: add i386 support for smp machine properties
>>    cpu/topology: add PPC support for smp machine properties
>>    cpu/topology: add riscv support for smp machine properties
>>    cpu/topology: add s390x support for smp machine properties
>>    cpu/topology: replace smp global variables with machine propertie
>>
>>   accel/kvm/kvm-all.c          |  3 +++
>>   backends/hostmem.c           |  4 ++++
>>   cpus.c                       |  4 ++++
>>   exec.c                       |  2 ++
>>   gdbstub.c                    |  7 ++++++-
>>   hw/alpha/dp264.c             |  1 +
>>   hw/arm/fsl-imx6.c            |  5 +++++
>>   hw/arm/fsl-imx6ul.c          |  5 +++++
>>   hw/arm/fsl-imx7.c            |  5 +++++
>>   hw/arm/highbank.c            |  1 +
>>   hw/arm/mcimx6ul-evk.c        |  1 +
>>   hw/arm/mcimx7d-sabre.c       |  3 +++
>>   hw/arm/raspi.c               |  2 ++
>>   hw/arm/realview.c            |  1 +
>>   hw/arm/sabrelite.c           |  1 +
>>   hw/arm/vexpress.c            |  3 +++
>>   hw/arm/virt.c                |  7 +++++++
>>   hw/arm/xlnx-zynqmp.c         |  7 +++++++
>>   hw/cpu/core.c                |  3 +++
>>   hw/hppa/machine.c            |  4 ++++
>>   hw/i386/acpi-build.c         |  3 +++
>>   hw/i386/kvmvapic.c           |  5 +++++
>>   hw/i386/pc.c                 | 12 +++++++++++
>>   hw/mips/boston.c             |  1 +
>>   hw/mips/mips_malta.c         |  9 +++++++++
>>   hw/openrisc/openrisc_sim.c   |  1 +
>>   hw/ppc/e500.c                |  3 +++
>>   hw/ppc/mac_newworld.c        |  2 ++
>>   hw/ppc/mac_oldworld.c        |  2 ++
>>   hw/ppc/pnv.c                 |  3 +++
>>   hw/ppc/prep.c                |  2 ++
>>   hw/ppc/spapr.c               | 29 ++++++++++++++++++++++++++
>>   hw/ppc/spapr_rtas.c          |  3 +++
>>   hw/riscv/sifive_e.c          |  4 ++++
>>   hw/riscv/sifive_plic.c       |  3 +++
>>   hw/riscv/sifive_u.c          |  4 ++++
>>   hw/riscv/spike.c             |  2 ++
>>   hw/riscv/virt.c              |  1 +
>>   hw/s390x/s390-virtio-ccw.c   |  2 ++
>>   hw/s390x/sclp.c              |  1 +
>>   hw/smbios/smbios.c           | 11 ++++++++++
>>   hw/sparc/sun4m.c             |  2 ++
>>   hw/sparc64/sun4u.c           |  2 ++
>>   hw/xtensa/sim.c              |  1 +
>>   hw/xtensa/xtfpga.c           |  1 +
>>   include/hw/arm/virt.h        |  2 +-
>>   include/hw/boards.h          |  8 ++++++++
>>   include/sysemu/sysemu.h      |  2 +-
>>   migration/postcopy-ram.c     |  7 +++++++
>>   numa.c                       |  1 +
>>   target/arm/cpu.c             |  7 +++++++
>>   target/i386/cpu.c            |  4 ++++
>>   target/openrisc/sys_helper.c |  5 +++++
>>   target/s390x/cpu.c           |  3 +++
>>   target/s390x/excp_helper.c   |  6 ++++++
>>   tcg/tcg.c                    | 15 ++++++++++++++
>>   vl.c                         | 48 ++++++++++++++++++++++++--------------------
>>   57 files changed, 261 insertions(+), 25 deletions(-)
>>
> 
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/9] cpu/topology: add struct CpuTopology to MachineState

[Igor Mammedov]

On Fri, 29 Mar 2019 16:48:37 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  include/hw/arm/virt.h   | 2 +-
>  include/hw/boards.h     | 8 ++++++++
>  include/sysemu/sysemu.h | 2 +-
>  vl.c                    | 7 ++++++-
>  4 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 507517c..724da0c 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -123,7 +123,7 @@ typedef struct {
>      struct arm_boot_info bootinfo;
>      MemMapEntry *memmap;
>      const int *irqmap;
> -    int smp_cpus;
> +    unsigned int smp_cpus;
As Alex pointed out it's unrelated cleanup
(with other similar hunks in this patch),
it's better to split it out into separate patch

>      void *fdt;
>      int fdt_size;
>      uint32_t clock_phandle;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index e231860..cbde276 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -231,6 +231,13 @@ typedef struct DeviceMemoryState {
>      MemoryRegion mr;
>  } DeviceMemoryState;
>  
> +typedef struct CpuTopology {
> +    unsigned int smp_cpus;
> +    unsigned int smp_cores;
> +    unsigned int smp_threads;
> +    unsigned int max_cpus;
> +} CpuTopology;
> +
>  /**
>   * MachineState:
>   */
> @@ -273,6 +280,7 @@ struct MachineState {
>      const char *cpu_type;
>      AccelState *accelerator;
>      CPUArchIdList *possible_cpus;
> +    CpuTopology topo;
maybe s/topo/smp/ and drop smp_ prefix inside CpuTopology structure

>      struct NVDIMMState *nvdimms_state;
>  };
>  
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 6065d9e..c0d7d7c 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -100,7 +100,7 @@ extern const char *keyboard_layout;
>  extern int win2k_install_hack;
>  extern int alt_grab;
>  extern int ctrl_grab;
> -extern int smp_cpus;
> +extern unsigned int smp_cpus;
>  extern unsigned int max_cpus;
>  extern int cursor_hide;
>  extern int graphic_rotate;
> diff --git a/vl.c b/vl.c
> index d61d560..9089253 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -162,7 +162,7 @@ static Chardev **serial_hds;
>  Chardev *parallel_hds[MAX_PARALLEL_PORTS];
>  int win2k_install_hack = 0;
>  int singlestep = 0;
> -int smp_cpus;
> +unsigned int smp_cpus;
>  unsigned int max_cpus;
>  int smp_cores = 1;
>  int smp_threads = 1;
> @@ -4116,6 +4116,11 @@ int main(int argc, char **argv, char **envp)
>  
>      smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
>  
> +    current_machine->topo.smp_cpus = smp_cpus;
> +    current_machine->topo.max_cpus = max_cpus;
> +    current_machine->topo.smp_cores = smp_cores;
> +    current_machine->topo.smp_threads = smp_threads;
> +
>      /* sanity-check smp_cpus and max_cpus against machine_class */
>      if (smp_cpus < machine_class->min_cpus) {
>          error_report("Invalid SMP CPUs %d. The min CPUs "

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Igor Mammedov]

On Fri, 29 Mar 2019 16:48:38 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  accel/kvm/kvm-all.c      |  3 +++
>  backends/hostmem.c       |  4 ++++
>  cpus.c                   |  4 ++++
>  exec.c                   |  2 ++
>  gdbstub.c                |  7 ++++++-
>  hw/cpu/core.c            |  3 +++
>  hw/smbios/smbios.c       | 11 +++++++++++
>  migration/postcopy-ram.c |  7 +++++++
>  numa.c                   |  1 +
>  tcg/tcg.c                | 15 +++++++++++++++
>  10 files changed, 56 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 241db49..5385218 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1526,6 +1526,9 @@ bool kvm_vcpu_id_is_valid(int vcpu_id)
>  static int kvm_init(MachineState *ms)
>  {
>      MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +    unsigned int max_cpus = ms->topo.max_cpus;
> +
>      static const char upgrade_note[] =
>          "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
>          "(see http://sourceforge.net/projects/kvm).\n";

> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 04baf47..cecdfd5 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -222,6 +222,8 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>  {
>      Error *local_err = NULL;
>      HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      if (backend->force_prealloc) {
>          if (value) {
> @@ -311,6 +313,8 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>  {
>      HostMemoryBackend *backend = MEMORY_BACKEND(uc);
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      Error *local_err = NULL;
>      void *ptr;
>      uint64_t sz;
I don't like us abusing smp_cpus for os_mem_prealloc in here as
it doesn't really have to do anything with machine and just adds
non necessary machine dependency to hostmem. And probably should
be a separate option for backend to specify a number of threads
to use for allocation.

But that's out of scope of this series, so it's fine is for now
and later we should get rid of it (which I suspect would be easier
once initial memoy also allocated by backends).

> diff --git a/cpus.c b/cpus.c
> index e83f72b..834a697 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2067,6 +2067,10 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
>  
>  void qemu_init_vcpu(CPUState *cpu)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cores = ms->topo.smp_cores;
> +    unsigned int smp_threads = ms->topo.smp_threads;

(***)
for once it probably will crash *-user builds
and secondly the purpose of getting rid of smp_foo globals
is disentangle layer violations and not replace it with another global
(qdev_get_machine()).

What should be done is to make a properties of nr_cores/nr_threads and set
them from the parent object that creates CPUs. The point is CPUs shouldn't
reach out outside itself to fish out data bits it needs, it's responsibility
of creator to feed to being create CPU needed properties.

This kind of refactoring probably deserves its own series and should precede
-smp refactoring as it doesn't depend on CpuTopology at all.

> +
>      cpu->nr_cores = smp_cores;
>      cpu->nr_threads = smp_threads;
>      cpu->stopped = true;
> diff --git a/exec.c b/exec.c
> index 86a38d3..a3c3db7 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1829,6 +1829,8 @@ static void *file_ram_alloc(RAMBlock *block,
>                              bool truncate,
>                              Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      void *area;
>  
>      block->page_size = qemu_fd_getpagesize(fd);
> diff --git a/gdbstub.c b/gdbstub.c
> index d54abd1..35f6bbc 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -30,6 +30,7 @@
>  #include "sysemu/sysemu.h"
>  #include "exec/gdbstub.h"
>  #include "hw/cpu/cluster.h"
> +#include "hw/boards.h"
>  #endif
>  
>  #define MAX_PACKET_LENGTH 4096
> @@ -1154,11 +1155,15 @@ static int gdb_handle_vcont(GDBState *s, const char *p)
>      CPUState *cpu;
>      GDBThreadIdKind kind;
>  #ifdef CONFIG_USER_ONLY
> -    int max_cpus = 1; /* global variable max_cpus exists only in system mode */
> +    /* global variable max_cpus exists only in system mode */
> +    unsigned int max_cpus = 1;
not related change

>  
>      CPU_FOREACH(cpu) {
>          max_cpus = max_cpus <= cpu->cpu_index ? cpu->cpu_index + 1 : max_cpus;
>      }
> +#else
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int max_cpus = ms->topo.max_cpus;
>  #endif
>      /* uninitialised CPUs stay 0 */
>      newstates = g_new0(char, max_cpus);
> diff --git a/hw/cpu/core.c b/hw/cpu/core.c
> index 7e42e2c..b75ffbb 100644
> --- a/hw/cpu/core.c
> +++ b/hw/cpu/core.c
> @@ -11,6 +11,7 @@
>  #include "qapi/visitor.h"
>  #include "qapi/error.h"
>  #include "sysemu/cpus.h"
> +#include "hw/boards.h"
>  
>  static void core_prop_get_core_id(Object *obj, Visitor *v, const char *name,
>                                    void *opaque, Error **errp)
> @@ -69,6 +70,8 @@ static void core_prop_set_nr_threads(Object *obj, Visitor *v, const char *name,
>  
>  static void cpu_core_instance_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_threads = ms->topo.smp_threads;
the same as [***]

>      CPUCore *core = CPU_CORE(obj);
>  
>      object_property_add(obj, "core-id", "int", core_prop_get_core_id,
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 47be907..a5eabe7 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -28,6 +28,7 @@
>  #include "hw/loader.h"
>  #include "exec/cpu-common.h"
>  #include "smbios_build.h"
> +#include "hw/boards.h"
>  
>  /* legacy structures and constants for <= 2.0 machines */
>  struct smbios_header {
> @@ -342,6 +343,9 @@ opts_init(smbios_register_config);
>  
>  static void smbios_validate_table(void)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +
>      uint32_t expect_t4_count = smbios_legacy ? smp_cpus : smbios_smp_sockets;
>  
>      if (smbios_type4_count && smbios_type4_count != expect_t4_count) {
> @@ -571,6 +575,9 @@ static void smbios_build_type_3_table(void)
>  
>  static void smbios_build_type_4_table(unsigned instance)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_threads = ms->topo.smp_threads;
> +    unsigned int smp_cores = ms->topo.smp_cores;
>      char sock_str[128];
>  
>      SMBIOS_BUILD_TABLE_PRE(4, 0x400 + instance, true); /* required */
> @@ -843,7 +850,11 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
>                         uint8_t **tables, size_t *tables_len,
>                         uint8_t **anchor, size_t *anchor_len)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      unsigned i, dimm_cnt;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +    unsigned int smp_cores = ms->topo.smp_cores;
> +    unsigned int smp_threads = ms->topo.smp_threads;
>  
wrt whole smbios changes:
instead of accessing machine directly, I suggest to pass necessary values
as arguments and modify callers to provide them

>      if (smbios_legacy) {
>          *tables = *anchor = NULL;
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index e2aa57a..ae92f6e 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/balloon.h"
>  #include "qemu/error-report.h"
>  #include "trace.h"
> +#include "hw/boards.h"
>  
>  /* Arbitrary limit on size of each discard command,
>   * keeps them around ~200 bytes
> @@ -128,6 +129,8 @@ static void migration_exit_cb(Notifier *n, void *data)
>  
>  static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
>      ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
>      ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
> @@ -141,6 +144,8 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
>  
>  static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      uint32List *list = NULL, *entry = NULL;
>      int i;
>  
> @@ -806,8 +811,10 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
>  static void mark_postcopy_blocktime_end(uintptr_t addr)
>  {
>      MigrationIncomingState *mis = migration_incoming_get_current();
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
>      int i, affected_cpu = 0;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      bool vcpu_total_blocktime = false;
>      uint32_t read_vcpu_time, low_time_offset;

I don't know enough about migration to say some thing useful here,
CCin David for comments.

>  
> diff --git a/numa.c b/numa.c
> index 3875e1e..127ddbe 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -64,6 +64,7 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>      uint16_t nodenr;
>      uint16List *cpus = NULL;
>      MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    unsigned int max_cpus = ms->topo.max_cpus;
>  
>      if (node->has_nodeid) {
>          nodenr = node->nodeid;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 9b2bf7f..d1501eb 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -44,6 +44,10 @@
>  #include "exec/cpu-common.h"
>  #include "exec/exec-all.h"
>  
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/boards.h"
> +#endif
> +
>  #include "tcg-op.h"
>  
>  #if UINTPTR_MAX == UINT32_MAX
> @@ -602,6 +606,10 @@ static size_t tcg_n_regions(void)
>      size_t i;
>  
>      /* Use a single region if all we have is one vCPU thread */
> +#if !defined(CONFIG_USER_ONLY)
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int max_cpus = ms->topo.max_cpus;
> +#endif
>      if (max_cpus == 1 || !qemu_tcg_mttcg_enabled()) {
>          return 1;
>      }
> @@ -751,7 +759,12 @@ void tcg_register_thread(void)
>  
>      /* Claim an entry in tcg_ctxs */
>      n = atomic_fetch_inc(&n_tcg_ctxs);
> +#if !defined(CONFIG_USER_ONLY)
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    g_assert(n < ms->topo.max_cpus);
> +#elif
>      g_assert(n < max_cpus);
> +#endif
>      atomic_set(&tcg_ctxs[n], s);
>  
>      tcg_ctx = s;
> @@ -961,6 +974,8 @@ void tcg_context_init(TCGContext *s)
>      tcg_ctxs = &tcg_ctx;
>      n_tcg_ctxs = 1;
>  #else
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int max_cpus = ms->topo.max_cpus;
>      tcg_ctxs = g_new(TCGContext *, max_cpus);
>  #endif

not sure what to in case of accelerators (TCG/KVM/...), as their code
eventually endups being used by cpus. Passing smp information down
from CPU would be a pain and might not work in every case. Maybe
we should pass it as part of configure_accelerator()

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Dr. David Alan Gilbert]

* Igor Mammedov (imammedo@redhat.com) wrote:
> On Fri, 29 Mar 2019 16:48:38 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > ---

<snipp>

> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index e2aa57a..ae92f6e 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -29,6 +29,7 @@
> >  #include "sysemu/balloon.h"
> >  #include "qemu/error-report.h"
> >  #include "trace.h"
> > +#include "hw/boards.h"
> >  
> >  /* Arbitrary limit on size of each discard command,
> >   * keeps them around ~200 bytes
> > @@ -128,6 +129,8 @@ static void migration_exit_cb(Notifier *n, void *data)
> >  
> >  static struct PostcopyBlocktimeContext *blocktime_context_new(void)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1);
> >      ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus);
> >      ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus);
> > @@ -141,6 +144,8 @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void)
> >  
> >  static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx)
> >  {
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      uint32List *list = NULL, *entry = NULL;
> >      int i;
> >  
> > @@ -806,8 +811,10 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid,
> >  static void mark_postcopy_blocktime_end(uintptr_t addr)
> >  {
> >      MigrationIncomingState *mis = migration_incoming_get_current();
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> >      PostcopyBlocktimeContext *dc = mis->blocktime_ctx;
> >      int i, affected_cpu = 0;
> > +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >      bool vcpu_total_blocktime = false;
> >      uint32_t read_vcpu_time, low_time_offset;
> 
> I don't know enough about migration to say some thing useful here,
> CCin David for comments.

I think that's OK; we just need to know the total number of vcpus; this
thing calculates some stats based on the number of the vCPUs that are
blocked by postcopy page waits, and in particular if everyone is
blocked.

(I'd slightly prefer macs there rather than ms, just because we tend to
use ms in migration for MigrationState sometimes, but not always).

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-trivial] [Qemu-devel] [PATCH 3/9] cpu/topology: add uncommon arch support for smp machine properties

[Igor Mammedov]

On Fri, 29 Mar 2019 16:48:39 +0800
Like Xu <like.xu@linux.intel.com> wrote:

here should be a commit message explaining what patch does
in more detail.


> Signed-off-by: Like Xu <like.xu@linux.intel.com>

Generic note, try not call qdev_get_machine() every time
you replace smp_cpus or other variables. It's often possible
to pass MachineState via call chain with trivial fixups.

> ---
>  hw/alpha/dp264.c     | 1 +
>  hw/hppa/machine.c    | 4 ++++
>  hw/mips/boston.c     | 1 +
>  hw/mips/mips_malta.c | 9 +++++++++
>  hw/sparc/sun4m.c     | 2 ++
>  hw/sparc64/sun4u.c   | 2 ++
>  hw/xtensa/sim.c      | 1 +
>  hw/xtensa/xtfpga.c   | 1 +
>  8 files changed, 21 insertions(+)
> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 0347eb8..ee5d432 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -63,6 +63,7 @@ static void clipper_init(MachineState *machine)
>      char *palcode_filename;
>      uint64_t palcode_entry, palcode_low, palcode_high;
>      uint64_t kernel_entry, kernel_low, kernel_high;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      /* Create up to 4 cpus.  */
>      memset(cpus, 0, sizeof(cpus));
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index d1b1d3c..f652891 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -16,6 +16,7 @@
>  #include "hw/ide.h"
>  #include "hw/timer/i8254.h"
>  #include "hw/char/serial.h"
> +#include "hw/boards.h"
>  #include "hppa_sys.h"
>  #include "qemu/units.h"
>  #include "qapi/error.h"
> @@ -72,6 +73,7 @@ static void machine_hppa_init(MachineState *machine)
>      MemoryRegion *ram_region;
>      MemoryRegion *cpu_region;
>      long i;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
I'd prefer to replace smp_cpus with machine->topo.smp_cpus
directly at places it's used, as it makes affected sites
more visible in the patch.
And use local smp_cpus only in places where using machine->topo.smp_cpus
makes core less readable.
(but it's just personal preference so I don't insist on it)

>  
>      ram_size = machine->ram_size;
>  
> @@ -242,7 +244,9 @@ static void machine_hppa_init(MachineState *machine)
>  
>  static void hppa_machine_reset(void)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;

***)
It would be better to pass MachineState as argument to
hppa_machine_reset(), a patch to so should go before this one.

Quick look shows only 3 overrides (hppa, pc, pnv) and one caller,
so I'd rather fix it than calling qdev_get_machine() unnecessarily 

>  
>      qemu_devices_reset();
>  
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index e5bab3c..7752c10 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -434,6 +434,7 @@ static void boston_mach_init(MachineState *machine)
>      DriveInfo *hd[6];
>      Chardev *chr;
>      int fw_size, fit_err;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>      bool is_64b;
>  
>      if ((machine->ram_size % GiB) ||
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 439665a..d595375 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1095,6 +1095,8 @@ static int64_t load_kernel (void)
>  
>  static void malta_mips_config(MIPSCPU *cpu)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      CPUMIPSState *env = &cpu->env;
>      CPUState *cs = CPU(cpu);
this one also called from reset, so the same [***] applies here too.

>  
> @@ -1127,9 +1129,11 @@ static void main_cpu_reset(void *opaque)
>  static void create_cpu_without_cps(const char *cpu_type,
>                                     qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
caller has an access to MachineState so pass it down call chain all the way

>      CPUMIPSState *env;
>      MIPSCPU *cpu;
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      for (i = 0; i < smp_cpus; i++) {
>          cpu = MIPS_CPU(cpu_create(cpu_type));
> @@ -1149,7 +1153,9 @@ static void create_cpu_without_cps(const char *cpu_type,
>  static void create_cps(MaltaState *s, const char *cpu_type,
>                         qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
ditto

>      Error *err = NULL;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
>      qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
> @@ -1171,6 +1177,9 @@ static void create_cps(MaltaState *s, const char *cpu_type,
>  static void mips_create_cpu(MaltaState *s, const char *cpu_type,
>                              qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
ditto

> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +
>      if ((smp_cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
>          create_cps(s, cpu_type, cbus_irq, i8259_irq);
>      } else {
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index ca1e382..2321cfb 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -853,6 +853,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>      unsigned int num_vsimms;
>      DeviceState *dev;
>      SysBusDevice *s;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> +    unsigned int max_cpus = machine->topo.max_cpus;
>  
>      /* init CPUs */
>      for(i = 0; i < smp_cpus; i++) {
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index 399f2d7..d089c4d 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -546,6 +546,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>      NICInfo *nd;
>      MACAddr macaddr;
>      bool onboard_nic;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> +    unsigned int max_cpus = machine->topo.max_cpus;
>  
>      /* init CPUs */
>      cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> index 12c7437..cc0396b 100644
> --- a/hw/xtensa/sim.c
> +++ b/hw/xtensa/sim.c
> @@ -59,6 +59,7 @@ static void xtensa_sim_init(MachineState *machine)
>      ram_addr_t ram_size = machine->ram_size;
>      const char *kernel_filename = machine->kernel_filename;
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      for (n = 0; n < smp_cpus; n++) {
>          cpu = XTENSA_CPU(cpu_create(machine->cpu_type));
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index e05ef75..f71a15e 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -238,6 +238,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>      const unsigned system_io_size = 224 * MiB;
>      uint32_t freq = 10000000;
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      if (smp_cpus > 1) {
>          mx_pic = xtensa_mx_pic_init(31);

Re: [Qemu-trivial] [Qemu-devel] [PATCH 4/9] cpu/topology: add ARM support for smp machine properties

[Igor Mammedov]

On Fri, 29 Mar 2019 16:48:40 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  hw/arm/fsl-imx6.c      | 5 +++++
>  hw/arm/fsl-imx6ul.c    | 5 +++++
>  hw/arm/fsl-imx7.c      | 5 +++++
>  hw/arm/highbank.c      | 1 +
>  hw/arm/mcimx6ul-evk.c  | 1 +
>  hw/arm/mcimx7d-sabre.c | 3 +++
>  hw/arm/raspi.c         | 2 ++
>  hw/arm/realview.c      | 1 +
>  hw/arm/sabrelite.c     | 1 +
>  hw/arm/vexpress.c      | 3 +++
>  hw/arm/virt.c          | 7 +++++++
>  hw/arm/xlnx-zynqmp.c   | 7 +++++++
>  target/arm/cpu.c       | 7 +++++++
>  13 files changed, 48 insertions(+)
> 
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index 7b7b97f..efed3a4 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -23,6 +23,7 @@
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "hw/arm/fsl-imx6.h"
> +#include "hw/boards.h"
>  #include "sysemu/sysemu.h"
>  #include "chardev/char.h"
>  #include "qemu/error-report.h"
> @@ -33,9 +34,11 @@
>  
>  static void fsl_imx6_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6State *s = FSL_IMX6(obj);
>      char name[NAME_SIZE];
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) {
this is so broken, one creates all cpus here and ...

>          snprintf(name, NAME_SIZE, "cpu%d", i);
> @@ -93,9 +96,11 @@ static void fsl_imx6_init(Object *obj)
>  
>  static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6State *s = FSL_IMX6(dev);
>      uint16_t i;
>      Error *err = NULL;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      if (smp_cpus > FSL_IMX6_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
than checks for validity later.

(***)
Replacing smp_cpus with another global var in the form qdev_get_machine()
ain't much of improvement.
I suggest adding smp_cpus property here/set it from caller
and then move initializing cpus to realize time after this check.

The same same applies to other similar cases below.

> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index 4b56bfa..426bf8e 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -23,14 +23,17 @@
>  #include "hw/misc/unimp.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "hw/boards.h"
>  
>  #define NAME_SIZE 20
>  
>  static void fsl_imx6ul_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6ULState *s = FSL_IMX6UL(obj);
>      char name[NAME_SIZE];
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
>          snprintf(name, NAME_SIZE, "cpu%d", i);
> @@ -156,10 +159,12 @@ static void fsl_imx6ul_init(Object *obj)
>  
>  static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX6ULState *s = FSL_IMX6UL(dev);
>      int i;
>      qemu_irq irq;
>      char name[NAME_SIZE];
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
> index 7663ad6..b759f7b 100644
> --- a/hw/arm/fsl-imx7.c
> +++ b/hw/arm/fsl-imx7.c
> @@ -25,11 +25,14 @@
>  #include "hw/misc/unimp.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
> +#include "hw/boards.h"
>  
>  #define NAME_SIZE 20
>  
>  static void fsl_imx7_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      FslIMX7State *s = FSL_IMX7(obj);
>      char name[NAME_SIZE];
>      int i;
> @@ -155,11 +158,13 @@ static void fsl_imx7_init(Object *obj)
>  
>  static void fsl_imx7_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      FslIMX7State *s = FSL_IMX7(dev);
>      Object *o;
>      int i;
>      qemu_irq irq;
>      char name[NAME_SIZE];
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      if (smp_cpus > FSL_IMX7_NUM_CPUS) {
>          error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 96ccf18..58c77f6 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -240,6 +240,7 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
>      SysBusDevice *busdev;
>      qemu_irq pic[128];
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>      qemu_irq cpu_irq[4];
>      qemu_irq cpu_fiq[4];
>      qemu_irq cpu_virq[4];
> diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
> index fb2b015..29fb706 100644
> --- a/hw/arm/mcimx6ul-evk.c
> +++ b/hw/arm/mcimx6ul-evk.c
> @@ -29,6 +29,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
>      static struct arm_boot_info boot_info;
>      MCIMX6ULEVK *s = g_new0(MCIMX6ULEVK, 1);
>      int i;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      if (machine->ram_size > FSL_IMX6UL_MMDC_SIZE) {
>          error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
> diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
> index 9c5f0e7..c2cdaf9 100644
> --- a/hw/arm/mcimx7d-sabre.c
> +++ b/hw/arm/mcimx7d-sabre.c
> @@ -20,6 +20,7 @@
>  #include "sysemu/sysemu.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/qtest.h"
> +#include "hw/boards.h"
>  
>  typedef struct {
>      FslIMX7State soc;
> @@ -28,10 +29,12 @@ typedef struct {
>  
>  static void mcimx7d_sabre_init(MachineState *machine)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      static struct arm_boot_info boot_info;
>      MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1);
>      Object *soc;
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      if (machine->ram_size > FSL_IMX7_MMDC_SIZE) {
>          error_report("RAM size " RAM_ADDR_FMT " above max supported (%08x)",
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 66899c2..8ab8e10 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -113,6 +113,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>  {
>      static struct arm_boot_info binfo;
>      int r;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      binfo.board_id = raspi_boardid[version];
>      binfo.ram_size = ram_size;
> @@ -174,6 +175,7 @@ static void raspi_init(MachineState *machine, int version)
>      BlockBackend *blk;
>      BusState *bus;
>      DeviceState *carddev;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      object_initialize(&s->soc, sizeof(s->soc),
>                        version == 3 ? TYPE_BCM2837 : TYPE_BCM2836);
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 242f5a8..517c275 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -69,6 +69,7 @@ static void realview_init(MachineState *machine,
>      NICInfo *nd;
>      I2CBus *i2c;
>      int n;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>      int done_nic = 0;
>      qemu_irq cpu_irq[4];
>      int is_mpcore = 0;
> diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
> index ee140e5..108faab 100644
> --- a/hw/arm/sabrelite.c
> +++ b/hw/arm/sabrelite.c
> @@ -47,6 +47,7 @@ static void sabrelite_init(MachineState *machine)
>  {
>      IMX6Sabrelite *s = g_new0(IMX6Sabrelite, 1);
>      Error *err = NULL;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      /* Check the amount of memory is compatible with the SOC */
>      if (machine->ram_size > FSL_IMX6_MMDC_SIZE) {
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index f07134c..1927d12 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -206,9 +206,11 @@ struct VEDBoardInfo {
>  static void init_cpus(const char *cpu_type, const char *privdev,
>                        hwaddr periphbase, qemu_irq *pic, bool secure, bool virt)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
pass machine as argument instead.

>      DeviceState *dev;
>      SysBusDevice *busdev;
>      int n;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      /* Create the actual CPUs */
>      for (n = 0; n < smp_cpus; n++) {
> @@ -558,6 +560,7 @@ static void vexpress_common_init(MachineState *machine)
>      MemoryRegion *flash0mem;
>      const hwaddr *map = daughterboard->motherboard_map;
>      int i;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>  
>      daughterboard->init(vms, machine->ram_size, machine->cpu_type, pic);
>  
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ce2664a..ff1dff3 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -555,11 +555,13 @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic)
>  
>  static void create_gic(VirtMachineState *vms, qemu_irq *pic)
>  {
> +    MachineState *ms = MACHINE(vms);
>      /* We create a standalone GIC */
>      DeviceState *gicdev;
>      SysBusDevice *gicbusdev;
>      const char *gictype;
>      int type = vms->gic_version, i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      uint32_t nb_redist_regions = 0;
>  
>      gictype = (type == 3) ? gicv3_class_name() : gic_class_name();
> @@ -984,10 +986,12 @@ static void create_flash(const VirtMachineState *vms,
>  
>  static FWCfgState *create_fw_cfg(const VirtMachineState *vms, AddressSpace *as)
>  {
> +    MachineState *ms = MACHINE(vms);
>      hwaddr base = vms->memmap[VIRT_FW_CFG].base;
>      hwaddr size = vms->memmap[VIRT_FW_CFG].size;
>      FWCfgState *fw_cfg;
>      char *nodename;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>  
>      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);
> @@ -1423,6 +1427,8 @@ static void machvirt_init(MachineState *machine)
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
>      bool aarch64 = true;
> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> +    unsigned int max_cpus = machine->topo.max_cpus;
>  
>      /*
>       * In accelerated mode, the memory map is computed earlier in kvm_type()
> @@ -1786,6 +1792,7 @@ static int64_t virt_get_default_cpu_node_id(const MachineState *ms, int idx)
>  static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms)
>  {
>      int n;
> +    unsigned int max_cpus = ms->topo.max_cpus;
>      VirtMachineState *vms = VIRT_MACHINE(ms);
>  
>      if (ms->possible_cpus) {
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 4f8bc41..29b9427 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -19,6 +19,7 @@
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> +#include "hw/boards.h"
>  #include "hw/arm/xlnx-zynqmp.h"
>  #include "hw/intc/arm_gic_common.h"
>  #include "exec/address-spaces.h"
> @@ -174,8 +175,10 @@ static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
>  static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>                                     Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      Error *err = NULL;
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_rpus = MIN(smp_cpus - XLNX_ZYNQMP_NUM_APU_CPUS, XLNX_ZYNQMP_NUM_RPU_CPUS);
>  
>      if (num_rpus <= 0) {
> @@ -221,8 +224,10 @@ static void xlnx_zynqmp_create_rpu(XlnxZynqMPState *s, const char *boot_cpu,
>  
>  static void xlnx_zynqmp_init(Object *obj)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
>      int i;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>  
>      object_initialize_child(obj, "apu-cluster", &s->apu_cluster,
> @@ -290,10 +295,12 @@ static void xlnx_zynqmp_init(Object *obj)
>  
>  static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
>      XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
>      MemoryRegion *system_memory = get_system_memory();
>      uint8_t i;
>      uint64_t ram_size;
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>      int num_apus = MIN(smp_cpus, XLNX_ZYNQMP_NUM_APU_CPUS);
>      const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
>      ram_addr_t ddr_low_size, ddr_high_size;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 4155782..e5d7e3b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -30,6 +30,7 @@
>  #include "hw/qdev-properties.h"
>  #if !defined(CONFIG_USER_ONLY)
>  #include "hw/loader.h"
> +#include "hw/boards.h"
>  #endif
>  #include "hw/arm/arm.h"
>  #include "sysemu/sysemu.h"
> @@ -1183,6 +1184,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>      init_cpreg_list(cpu);
>  
>  #ifndef CONFIG_USER_ONLY
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> +
>      if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>          cs->num_ases = 2;
>  
> @@ -1713,6 +1717,9 @@ static void cortex_a9_initfn(Object *obj)
>  #ifndef CONFIG_USER_ONLY
>  static uint64_t a15_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    unsigned int smp_cpus = ms->topo.smp_cpus;
for both hunks the same as [***]

> +
>      /* Linux wants the number of processors from here.
>       * Might as well set the interrupt-controller bit too.
>       */

Re: [Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Igor Mammedov]

On Thu, 4 Apr 2019 11:26:09 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> On 2019/3/29 18:21, Igor Mammedov wrote:
> > On Fri, 29 Mar 2019 16:48:36 +0800
> > Like Xu <like.xu@linux.intel.com> wrote:
> >   
> >> This patch series make existing cores/threads/sockets into machine
> >> properties and get rid of global variables they use currently.  
> > Thanks for looking into it!
> > Its long overdue and rather desired conversion (albeit naive one,
> > but this series is a good starting point). I'll go over your patches
> > next week with comments and concrete suggestions how to implement
> > particular things.  
> 
> Hi Igor, any comments and suggestions on smp machine properties
> in this patch considering we may add die topology for PCMachine as an 
> extension?

I've looked at several patches and that it for this series.
The most comments apply to the patches I've not reviewed as well.

> >   
> >> Like Xu (9):
> >>    cpu/topology: add struct CpuTopology to MachineState
> >>    cpu/topology: add general support for machine properties
> >>    cpu/topology: add uncommon arch support for smp machine properties
> >>    cpu/topology: add ARM support for smp machine properties
> >>    cpu/topology: add i386 support for smp machine properties
> >>    cpu/topology: add PPC support for smp machine properties
> >>    cpu/topology: add riscv support for smp machine properties
> >>    cpu/topology: add s390x support for smp machine properties
> >>    cpu/topology: replace smp global variables with machine propertie
> >>
> >>   accel/kvm/kvm-all.c          |  3 +++
> >>   backends/hostmem.c           |  4 ++++
> >>   cpus.c                       |  4 ++++
> >>   exec.c                       |  2 ++
> >>   gdbstub.c                    |  7 ++++++-
> >>   hw/alpha/dp264.c             |  1 +
> >>   hw/arm/fsl-imx6.c            |  5 +++++
> >>   hw/arm/fsl-imx6ul.c          |  5 +++++
> >>   hw/arm/fsl-imx7.c            |  5 +++++
> >>   hw/arm/highbank.c            |  1 +
> >>   hw/arm/mcimx6ul-evk.c        |  1 +
> >>   hw/arm/mcimx7d-sabre.c       |  3 +++
> >>   hw/arm/raspi.c               |  2 ++
> >>   hw/arm/realview.c            |  1 +
> >>   hw/arm/sabrelite.c           |  1 +
> >>   hw/arm/vexpress.c            |  3 +++
> >>   hw/arm/virt.c                |  7 +++++++
> >>   hw/arm/xlnx-zynqmp.c         |  7 +++++++
> >>   hw/cpu/core.c                |  3 +++
> >>   hw/hppa/machine.c            |  4 ++++
> >>   hw/i386/acpi-build.c         |  3 +++
> >>   hw/i386/kvmvapic.c           |  5 +++++
> >>   hw/i386/pc.c                 | 12 +++++++++++
> >>   hw/mips/boston.c             |  1 +
> >>   hw/mips/mips_malta.c         |  9 +++++++++
> >>   hw/openrisc/openrisc_sim.c   |  1 +
> >>   hw/ppc/e500.c                |  3 +++
> >>   hw/ppc/mac_newworld.c        |  2 ++
> >>   hw/ppc/mac_oldworld.c        |  2 ++
> >>   hw/ppc/pnv.c                 |  3 +++
> >>   hw/ppc/prep.c                |  2 ++
> >>   hw/ppc/spapr.c               | 29 ++++++++++++++++++++++++++
> >>   hw/ppc/spapr_rtas.c          |  3 +++
> >>   hw/riscv/sifive_e.c          |  4 ++++
> >>   hw/riscv/sifive_plic.c       |  3 +++
> >>   hw/riscv/sifive_u.c          |  4 ++++
> >>   hw/riscv/spike.c             |  2 ++
> >>   hw/riscv/virt.c              |  1 +
> >>   hw/s390x/s390-virtio-ccw.c   |  2 ++
> >>   hw/s390x/sclp.c              |  1 +
> >>   hw/smbios/smbios.c           | 11 ++++++++++
> >>   hw/sparc/sun4m.c             |  2 ++
> >>   hw/sparc64/sun4u.c           |  2 ++
> >>   hw/xtensa/sim.c              |  1 +
> >>   hw/xtensa/xtfpga.c           |  1 +
> >>   include/hw/arm/virt.h        |  2 +-
> >>   include/hw/boards.h          |  8 ++++++++
> >>   include/sysemu/sysemu.h      |  2 +-
> >>   migration/postcopy-ram.c     |  7 +++++++
> >>   numa.c                       |  1 +
> >>   target/arm/cpu.c             |  7 +++++++
> >>   target/i386/cpu.c            |  4 ++++
> >>   target/openrisc/sys_helper.c |  5 +++++
> >>   target/s390x/cpu.c           |  3 +++
> >>   target/s390x/excp_helper.c   |  6 ++++++
> >>   tcg/tcg.c                    | 15 ++++++++++++++
> >>   vl.c                         | 48 ++++++++++++++++++++++++--------------------
> >>   57 files changed, 261 insertions(+), 25 deletions(-)
> >>  
> > 
> > 
> >   
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 0/9] refactor cpu topo into machine properties

[Like Xu]

On 2019/4/8 21:26, Igor Mammedov wrote:
> On Thu, 4 Apr 2019 11:26:09 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 
>> On 2019/3/29 18:21, Igor Mammedov wrote:
>>> On Fri, 29 Mar 2019 16:48:36 +0800
>>> Like Xu <like.xu@linux.intel.com> wrote:
>>>    
>>>> This patch series make existing cores/threads/sockets into machine
>>>> properties and get rid of global variables they use currently.
>>> Thanks for looking into it!
>>> Its long overdue and rather desired conversion (albeit naive one,
>>> but this series is a good starting point). I'll go over your patches
>>> next week with comments and concrete suggestions how to implement
>>> particular things.
>>
>> Hi Igor, any comments and suggestions on smp machine properties
>> in this patch considering we may add die topology for PCMachine as an
>> extension?
> 
> I've looked at several patches and that it for this series.
> The most comments apply to the patches I've not reviewed as well.

Hi Igor, thanks for your comments, time and patience.

I'll try to fix them in next version ASAP.

> 
>>>    
>>>> Like Xu (9):
>>>>     cpu/topology: add struct CpuTopology to MachineState
>>>>     cpu/topology: add general support for machine properties
>>>>     cpu/topology: add uncommon arch support for smp machine properties
>>>>     cpu/topology: add ARM support for smp machine properties
>>>>     cpu/topology: add i386 support for smp machine properties
>>>>     cpu/topology: add PPC support for smp machine properties
>>>>     cpu/topology: add riscv support for smp machine properties
>>>>     cpu/topology: add s390x support for smp machine properties
>>>>     cpu/topology: replace smp global variables with machine propertie
>>>>
>>>>    accel/kvm/kvm-all.c          |  3 +++
>>>>    backends/hostmem.c           |  4 ++++
>>>>    cpus.c                       |  4 ++++
>>>>    exec.c                       |  2 ++
>>>>    gdbstub.c                    |  7 ++++++-
>>>>    hw/alpha/dp264.c             |  1 +
>>>>    hw/arm/fsl-imx6.c            |  5 +++++
>>>>    hw/arm/fsl-imx6ul.c          |  5 +++++
>>>>    hw/arm/fsl-imx7.c            |  5 +++++
>>>>    hw/arm/highbank.c            |  1 +
>>>>    hw/arm/mcimx6ul-evk.c        |  1 +
>>>>    hw/arm/mcimx7d-sabre.c       |  3 +++
>>>>    hw/arm/raspi.c               |  2 ++
>>>>    hw/arm/realview.c            |  1 +
>>>>    hw/arm/sabrelite.c           |  1 +
>>>>    hw/arm/vexpress.c            |  3 +++
>>>>    hw/arm/virt.c                |  7 +++++++
>>>>    hw/arm/xlnx-zynqmp.c         |  7 +++++++
>>>>    hw/cpu/core.c                |  3 +++
>>>>    hw/hppa/machine.c            |  4 ++++
>>>>    hw/i386/acpi-build.c         |  3 +++
>>>>    hw/i386/kvmvapic.c           |  5 +++++
>>>>    hw/i386/pc.c                 | 12 +++++++++++
>>>>    hw/mips/boston.c             |  1 +
>>>>    hw/mips/mips_malta.c         |  9 +++++++++
>>>>    hw/openrisc/openrisc_sim.c   |  1 +
>>>>    hw/ppc/e500.c                |  3 +++
>>>>    hw/ppc/mac_newworld.c        |  2 ++
>>>>    hw/ppc/mac_oldworld.c        |  2 ++
>>>>    hw/ppc/pnv.c                 |  3 +++
>>>>    hw/ppc/prep.c                |  2 ++
>>>>    hw/ppc/spapr.c               | 29 ++++++++++++++++++++++++++
>>>>    hw/ppc/spapr_rtas.c          |  3 +++
>>>>    hw/riscv/sifive_e.c          |  4 ++++
>>>>    hw/riscv/sifive_plic.c       |  3 +++
>>>>    hw/riscv/sifive_u.c          |  4 ++++
>>>>    hw/riscv/spike.c             |  2 ++
>>>>    hw/riscv/virt.c              |  1 +
>>>>    hw/s390x/s390-virtio-ccw.c   |  2 ++
>>>>    hw/s390x/sclp.c              |  1 +
>>>>    hw/smbios/smbios.c           | 11 ++++++++++
>>>>    hw/sparc/sun4m.c             |  2 ++
>>>>    hw/sparc64/sun4u.c           |  2 ++
>>>>    hw/xtensa/sim.c              |  1 +
>>>>    hw/xtensa/xtfpga.c           |  1 +
>>>>    include/hw/arm/virt.h        |  2 +-
>>>>    include/hw/boards.h          |  8 ++++++++
>>>>    include/sysemu/sysemu.h      |  2 +-
>>>>    migration/postcopy-ram.c     |  7 +++++++
>>>>    numa.c                       |  1 +
>>>>    target/arm/cpu.c             |  7 +++++++
>>>>    target/i386/cpu.c            |  4 ++++
>>>>    target/openrisc/sys_helper.c |  5 +++++
>>>>    target/s390x/cpu.c           |  3 +++
>>>>    target/s390x/excp_helper.c   |  6 ++++++
>>>>    tcg/tcg.c                    | 15 ++++++++++++++
>>>>    vl.c                         | 48 ++++++++++++++++++++++++--------------------
>>>>    57 files changed, 261 insertions(+), 25 deletions(-)
>>>>   
>>>
>>>
>>>    
>>
>>
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 3/9] cpu/topology: add uncommon arch support for smp machine properties

[Like Xu]

On 2019/4/8 20:54, Igor Mammedov wrote:
> On Fri, 29 Mar 2019 16:48:39 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 
> here should be a commit message explaining what patch does
> in more detail.
> 
> 
>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> 
> Generic note, try not call qdev_get_machine() every time
> you replace smp_cpus or other variables. It's often possible
> to pass MachineState via call chain with trivial fixups.

Hi Igor,

I have some doubts on this comments after some attempts.

I'm not sure if this idea could apply to all qdev_get_machine()
usages in tree or just for this smp-touch-only patch.

It takes a lot of efforts on hooks overrides when we
undo calls to qdev_get_machine() with modification of incoming parameters.

The implementation of qdev_get_machine() couldn't be simpler more
and it doesn't seem to bring much overhead compared with parameter stack.

> 
>> ---
>>   hw/alpha/dp264.c     | 1 +
>>   hw/hppa/machine.c    | 4 ++++
>>   hw/mips/boston.c     | 1 +
>>   hw/mips/mips_malta.c | 9 +++++++++
>>   hw/sparc/sun4m.c     | 2 ++
>>   hw/sparc64/sun4u.c   | 2 ++
>>   hw/xtensa/sim.c      | 1 +
>>   hw/xtensa/xtfpga.c   | 1 +
>>   8 files changed, 21 insertions(+)
>>
>> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
>> index 0347eb8..ee5d432 100644
>> --- a/hw/alpha/dp264.c
>> +++ b/hw/alpha/dp264.c
>> @@ -63,6 +63,7 @@ static void clipper_init(MachineState *machine)
>>       char *palcode_filename;
>>       uint64_t palcode_entry, palcode_low, palcode_high;
>>       uint64_t kernel_entry, kernel_low, kernel_high;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>   
>>       /* Create up to 4 cpus.  */
>>       memset(cpus, 0, sizeof(cpus));
>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
>> index d1b1d3c..f652891 100644
>> --- a/hw/hppa/machine.c
>> +++ b/hw/hppa/machine.c
>> @@ -16,6 +16,7 @@
>>   #include "hw/ide.h"
>>   #include "hw/timer/i8254.h"
>>   #include "hw/char/serial.h"
>> +#include "hw/boards.h"
>>   #include "hppa_sys.h"
>>   #include "qemu/units.h"
>>   #include "qapi/error.h"
>> @@ -72,6 +73,7 @@ static void machine_hppa_init(MachineState *machine)
>>       MemoryRegion *ram_region;
>>       MemoryRegion *cpu_region;
>>       long i;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> I'd prefer to replace smp_cpus with machine->topo.smp_cpus
> directly at places it's used, as it makes affected sites
> more visible in the patch.
> And use local smp_cpus only in places where using machine->topo.smp_cpus
> makes core less readable.
> (but it's just personal preference so I don't insist on it)
> 
>>   
>>       ram_size = machine->ram_size;
>>   
>> @@ -242,7 +244,9 @@ static void machine_hppa_init(MachineState *machine)
>>   
>>   static void hppa_machine_reset(void)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> 
> ***)
> It would be better to pass MachineState as argument to
> hppa_machine_reset(), a patch to so should go before this one.
> 
> Quick look shows only 3 overrides (hppa, pc, pnv) and one caller,
> so I'd rather fix it than calling qdev_get_machine() unnecessarily
> 
>>   
>>       qemu_devices_reset();
>>   
>> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>> index e5bab3c..7752c10 100644
>> --- a/hw/mips/boston.c
>> +++ b/hw/mips/boston.c
>> @@ -434,6 +434,7 @@ static void boston_mach_init(MachineState *machine)
>>       DriveInfo *hd[6];
>>       Chardev *chr;
>>       int fw_size, fit_err;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>       bool is_64b;
>>   
>>       if ((machine->ram_size % GiB) ||
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index 439665a..d595375 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -1095,6 +1095,8 @@ static int64_t load_kernel (void)
>>   
>>   static void malta_mips_config(MIPSCPU *cpu)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>       CPUMIPSState *env = &cpu->env;
>>       CPUState *cs = CPU(cpu);
> this one also called from reset, so the same [***] applies here too.
> 
>>   
>> @@ -1127,9 +1129,11 @@ static void main_cpu_reset(void *opaque)
>>   static void create_cpu_without_cps(const char *cpu_type,
>>                                      qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
> caller has an access to MachineState so pass it down call chain all the way
> 
>>       CPUMIPSState *env;
>>       MIPSCPU *cpu;
>>       int i;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>   
>>       for (i = 0; i < smp_cpus; i++) {
>>           cpu = MIPS_CPU(cpu_create(cpu_type));
>> @@ -1149,7 +1153,9 @@ static void create_cpu_without_cps(const char *cpu_type,
>>   static void create_cps(MaltaState *s, const char *cpu_type,
>>                          qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
> ditto
> 
>>       Error *err = NULL;
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>>   
>>       s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
>>       qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
>> @@ -1171,6 +1177,9 @@ static void create_cps(MaltaState *s, const char *cpu_type,
>>   static void mips_create_cpu(MaltaState *s, const char *cpu_type,
>>                               qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
> ditto
> 
>> +    unsigned int smp_cpus = ms->topo.smp_cpus;
>> +
>>       if ((smp_cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
>>           create_cps(s, cpu_type, cbus_irq, i8259_irq);
>>       } else {
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index ca1e382..2321cfb 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -853,6 +853,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
>>       unsigned int num_vsimms;
>>       DeviceState *dev;
>>       SysBusDevice *s;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>> +    unsigned int max_cpus = machine->topo.max_cpus;
>>   
>>       /* init CPUs */
>>       for(i = 0; i < smp_cpus; i++) {
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index 399f2d7..d089c4d 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -546,6 +546,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
>>       NICInfo *nd;
>>       MACAddr macaddr;
>>       bool onboard_nic;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>> +    unsigned int max_cpus = machine->topo.max_cpus;
>>   
>>       /* init CPUs */
>>       cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
>> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
>> index 12c7437..cc0396b 100644
>> --- a/hw/xtensa/sim.c
>> +++ b/hw/xtensa/sim.c
>> @@ -59,6 +59,7 @@ static void xtensa_sim_init(MachineState *machine)
>>       ram_addr_t ram_size = machine->ram_size;
>>       const char *kernel_filename = machine->kernel_filename;
>>       int n;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>   
>>       for (n = 0; n < smp_cpus; n++) {
>>           cpu = XTENSA_CPU(cpu_create(machine->cpu_type));
>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>> index e05ef75..f71a15e 100644
>> --- a/hw/xtensa/xtfpga.c
>> +++ b/hw/xtensa/xtfpga.c
>> @@ -238,6 +238,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
>>       const unsigned system_io_size = 224 * MiB;
>>       uint32_t freq = 10000000;
>>       int n;
>> +    unsigned int smp_cpus = machine->topo.smp_cpus;
>>   
>>       if (smp_cpus > 1) {
>>           mx_pic = xtensa_mx_pic_init(31);
> 
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 3/9] cpu/topology: add uncommon arch support for smp machine properties

[Igor Mammedov]

On Tue, 16 Apr 2019 16:47:55 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> On 2019/4/8 20:54, Igor Mammedov wrote:
> > On Fri, 29 Mar 2019 16:48:39 +0800
> > Like Xu <like.xu@linux.intel.com> wrote:
> > 
> > here should be a commit message explaining what patch does
> > in more detail.
> > 
> >   
> >> Signed-off-by: Like Xu <like.xu@linux.intel.com>  
> > 
> > Generic note, try not call qdev_get_machine() every time
> > you replace smp_cpus or other variables. It's often possible
> > to pass MachineState via call chain with trivial fixups.  
> 
> Hi Igor,
> 
> I have some doubts on this comments after some attempts.
> 
> I'm not sure if this idea could apply to all qdev_get_machine()
> usages in tree or just for this smp-touch-only patch.
Ideally it would apply to all users, but realistically it's
probably too much of refactoring for one series.
So whether we use qdev_get_machine() or not depends on
concrete usecase and how complex 'right' refactoring would be.

> It takes a lot of efforts on hooks overrides when we
> undo calls to qdev_get_machine() with modification of incoming parameters.
sometime it's too much of efforts and sometimes efforts are
justified if it makes code more robust and clear.
What I've seen in this series it's mostly trivial replacement
of -smp related globals with qdev_get_machine() without
looking up the call stack to see if Machine is already available
to the caller and just a several call frames away.

In case of this concrete patch wrt reset hook it's logical
to pass Machine as argument to the caller as I commented earlier
and it's not that complex change.

> The implementation of qdev_get_machine() couldn't be simpler more
> and it doesn't seem to bring much overhead compared with parameter stack.
It's not only about overhead, basically with qdev_get_machine()
you are replacing one global variable with another wrapped in
function call. That however introduces implicit dependency on
machine and the order it's initialized, which makes code a bit
more fragile and it's hard to review.

Or putting it in the form of question: what the reason for
replacing one global with another and why we are getting rid of
globals in the first place?

   
> >> ---
> >>   hw/alpha/dp264.c     | 1 +
> >>   hw/hppa/machine.c    | 4 ++++
> >>   hw/mips/boston.c     | 1 +
> >>   hw/mips/mips_malta.c | 9 +++++++++
> >>   hw/sparc/sun4m.c     | 2 ++
> >>   hw/sparc64/sun4u.c   | 2 ++
> >>   hw/xtensa/sim.c      | 1 +
> >>   hw/xtensa/xtfpga.c   | 1 +
> >>   8 files changed, 21 insertions(+)
> >>
> >> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> >> index 0347eb8..ee5d432 100644
> >> --- a/hw/alpha/dp264.c
> >> +++ b/hw/alpha/dp264.c
> >> @@ -63,6 +63,7 @@ static void clipper_init(MachineState *machine)
> >>       char *palcode_filename;
> >>       uint64_t palcode_entry, palcode_low, palcode_high;
> >>       uint64_t kernel_entry, kernel_low, kernel_high;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >>   
> >>       /* Create up to 4 cpus.  */
> >>       memset(cpus, 0, sizeof(cpus));
> >> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> >> index d1b1d3c..f652891 100644
> >> --- a/hw/hppa/machine.c
> >> +++ b/hw/hppa/machine.c
> >> @@ -16,6 +16,7 @@
> >>   #include "hw/ide.h"
> >>   #include "hw/timer/i8254.h"
> >>   #include "hw/char/serial.h"
> >> +#include "hw/boards.h"
> >>   #include "hppa_sys.h"
> >>   #include "qemu/units.h"
> >>   #include "qapi/error.h"
> >> @@ -72,6 +73,7 @@ static void machine_hppa_init(MachineState *machine)
> >>       MemoryRegion *ram_region;
> >>       MemoryRegion *cpu_region;
> >>       long i;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;  
> > I'd prefer to replace smp_cpus with machine->topo.smp_cpus
> > directly at places it's used, as it makes affected sites
> > more visible in the patch.
> > And use local smp_cpus only in places where using machine->topo.smp_cpus
> > makes core less readable.
> > (but it's just personal preference so I don't insist on it)
> >   
> >>   
> >>       ram_size = machine->ram_size;
> >>   
> >> @@ -242,7 +244,9 @@ static void machine_hppa_init(MachineState *machine)
> >>   
> >>   static void hppa_machine_reset(void)
> >>   {
> >> +    MachineState *ms = MACHINE(qdev_get_machine());
> >>       int i;
> >> +    unsigned int smp_cpus = ms->topo.smp_cpus;  
> > 
> > ***)
> > It would be better to pass MachineState as argument to
> > hppa_machine_reset(), a patch to so should go before this one.
> > 
> > Quick look shows only 3 overrides (hppa, pc, pnv) and one caller,
> > so I'd rather fix it than calling qdev_get_machine() unnecessarily
> >   
> >>   
> >>       qemu_devices_reset();
> >>   
> >> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> >> index e5bab3c..7752c10 100644
> >> --- a/hw/mips/boston.c
> >> +++ b/hw/mips/boston.c
> >> @@ -434,6 +434,7 @@ static void boston_mach_init(MachineState *machine)
> >>       DriveInfo *hd[6];
> >>       Chardev *chr;
> >>       int fw_size, fit_err;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >>       bool is_64b;
> >>   
> >>       if ((machine->ram_size % GiB) ||
> >> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> >> index 439665a..d595375 100644
> >> --- a/hw/mips/mips_malta.c
> >> +++ b/hw/mips/mips_malta.c
> >> @@ -1095,6 +1095,8 @@ static int64_t load_kernel (void)
> >>   
> >>   static void malta_mips_config(MIPSCPU *cpu)
> >>   {
> >> +    MachineState *ms = MACHINE(qdev_get_machine());
> >> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >>       CPUMIPSState *env = &cpu->env;
> >>       CPUState *cs = CPU(cpu);  
> > this one also called from reset, so the same [***] applies here too.
> >   
> >>   
> >> @@ -1127,9 +1129,11 @@ static void main_cpu_reset(void *opaque)
> >>   static void create_cpu_without_cps(const char *cpu_type,
> >>                                      qemu_irq *cbus_irq, qemu_irq *i8259_irq)
> >>   {
> >> +    MachineState *ms = MACHINE(qdev_get_machine());  
> > caller has an access to MachineState so pass it down call chain all the way
> >   
> >>       CPUMIPSState *env;
> >>       MIPSCPU *cpu;
> >>       int i;
> >> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >>   
> >>       for (i = 0; i < smp_cpus; i++) {
> >>           cpu = MIPS_CPU(cpu_create(cpu_type));
> >> @@ -1149,7 +1153,9 @@ static void create_cpu_without_cps(const char *cpu_type,
> >>   static void create_cps(MaltaState *s, const char *cpu_type,
> >>                          qemu_irq *cbus_irq, qemu_irq *i8259_irq)
> >>   {
> >> +    MachineState *ms = MACHINE(qdev_get_machine());  
> > ditto
> >   
> >>       Error *err = NULL;
> >> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >>   
> >>       s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
> >>       qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
> >> @@ -1171,6 +1177,9 @@ static void create_cps(MaltaState *s, const char *cpu_type,
> >>   static void mips_create_cpu(MaltaState *s, const char *cpu_type,
> >>                               qemu_irq *cbus_irq, qemu_irq *i8259_irq)
> >>   {
> >> +    MachineState *ms = MACHINE(qdev_get_machine());  
> > ditto
> >   
> >> +    unsigned int smp_cpus = ms->topo.smp_cpus;
> >> +
> >>       if ((smp_cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
> >>           create_cps(s, cpu_type, cbus_irq, i8259_irq);
> >>       } else {
> >> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> >> index ca1e382..2321cfb 100644
> >> --- a/hw/sparc/sun4m.c
> >> +++ b/hw/sparc/sun4m.c
> >> @@ -853,6 +853,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
> >>       unsigned int num_vsimms;
> >>       DeviceState *dev;
> >>       SysBusDevice *s;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >> +    unsigned int max_cpus = machine->topo.max_cpus;
> >>   
> >>       /* init CPUs */
> >>       for(i = 0; i < smp_cpus; i++) {
> >> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> >> index 399f2d7..d089c4d 100644
> >> --- a/hw/sparc64/sun4u.c
> >> +++ b/hw/sparc64/sun4u.c
> >> @@ -546,6 +546,8 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
> >>       NICInfo *nd;
> >>       MACAddr macaddr;
> >>       bool onboard_nic;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >> +    unsigned int max_cpus = machine->topo.max_cpus;
> >>   
> >>       /* init CPUs */
> >>       cpu = sparc64_cpu_devinit(machine->cpu_type, hwdef->prom_addr);
> >> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> >> index 12c7437..cc0396b 100644
> >> --- a/hw/xtensa/sim.c
> >> +++ b/hw/xtensa/sim.c
> >> @@ -59,6 +59,7 @@ static void xtensa_sim_init(MachineState *machine)
> >>       ram_addr_t ram_size = machine->ram_size;
> >>       const char *kernel_filename = machine->kernel_filename;
> >>       int n;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >>   
> >>       for (n = 0; n < smp_cpus; n++) {
> >>           cpu = XTENSA_CPU(cpu_create(machine->cpu_type));
> >> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> >> index e05ef75..f71a15e 100644
> >> --- a/hw/xtensa/xtfpga.c
> >> +++ b/hw/xtensa/xtfpga.c
> >> @@ -238,6 +238,7 @@ static void xtfpga_init(const XtfpgaBoardDesc *board, MachineState *machine)
> >>       const unsigned system_io_size = 224 * MiB;
> >>       uint32_t freq = 10000000;
> >>       int n;
> >> +    unsigned int smp_cpus = machine->topo.smp_cpus;
> >>   
> >>       if (smp_cpus > 1) {
> >>           mx_pic = xtensa_mx_pic_init(31);  
> > 
> >   
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Like Xu]

On 2019/4/4 22:25, Igor Mammedov wrote:
> On Fri, 29 Mar 2019 16:48:38 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 

<snipp>
> 
>> diff --git a/cpus.c b/cpus.c
>> index e83f72b..834a697 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -2067,6 +2067,10 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
>>   
>>   void qemu_init_vcpu(CPUState *cpu)
>>   {
>> +    MachineState *ms = MACHINE(qdev_get_machine());
>> +    unsigned int smp_cores = ms->topo.smp_cores;
>> +    unsigned int smp_threads = ms->topo.smp_threads;
> 
> (***)
> for once it probably will crash *-user builds
> and secondly the purpose of getting rid of smp_foo globals
> is disentangle layer violations and not replace it with another global
> (qdev_get_machine()).

I am happy to follow this rule on cpu-topo refactoring work, but 
sometimes calling qdev_get_machine() is inevitable.

> 
> What should be done is to make a properties of nr_cores/nr_threads and set
> them from the parent object that creates CPUs. The point is CPUs shouldn't
> reach out outside itself to fish out data bits it needs, it's responsibility
> of creator to feed to being create CPU needed properties.
> 
> This kind of refactoring probably deserves its own series and should precede
> -smp refactoring as it doesn't depend on CpuTopology at all.
> 

The division of responsibility for this case (refactoring 
qemu_init_vcpu) seems to be a poisonous apple.

The prerequisite for setting cpu-> nr_cores / nr_threads from the parent 
is that the CPU has been created, so if any process during 
initialization needs this topo information, it will use the default 
values form cpu_common_initfn() instead of user-configured parameters.

We may not want to repeat those assignment operations using the new 
values and what do you think, Igor?

<snipp>

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Igor Mammedov]

On Tue, 30 Apr 2019 15:30:31 +0800
Like Xu <like.xu@linux.intel.com> wrote:

> On 2019/4/4 22:25, Igor Mammedov wrote:
> > On Fri, 29 Mar 2019 16:48:38 +0800
> > Like Xu <like.xu@linux.intel.com> wrote:
> > 

[...]

> 
> The division of responsibility for this case (refactoring 
> qemu_init_vcpu) seems to be a poisonous apple.
> 
> The prerequisite for setting cpu-> nr_cores / nr_threads from the parent 
> is that the CPU has been created, so if any process during 
> initialization needs this topo information, it will use the default 
> values form cpu_common_initfn() instead of user-configured parameters.

can you point to concrete place that needs access to nr_cores / nr_threads
before cpu is 'realized'?

 
> We may not want to repeat those assignment operations using the new 
> values and what do you think, Igor?
> 
> <snipp>
> 

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Eduardo Habkost]

On Tue, Apr 30, 2019 at 03:30:31PM +0800, Like Xu wrote:
> On 2019/4/4 22:25, Igor Mammedov wrote:
> > On Fri, 29 Mar 2019 16:48:38 +0800
> > Like Xu <like.xu@linux.intel.com> wrote:
> > 
> 
> <snipp>
> > 
> > > diff --git a/cpus.c b/cpus.c
> > > index e83f72b..834a697 100644
> > > --- a/cpus.c
> > > +++ b/cpus.c
> > > @@ -2067,6 +2067,10 @@ static void qemu_dummy_start_vcpu(CPUState *cpu)
> > >   void qemu_init_vcpu(CPUState *cpu)
> > >   {
> > > +    MachineState *ms = MACHINE(qdev_get_machine());
> > > +    unsigned int smp_cores = ms->topo.smp_cores;
> > > +    unsigned int smp_threads = ms->topo.smp_threads;
> > 
> > (***)
> > for once it probably will crash *-user builds

Will it?  cpus.o is compiled only on CONFIG_SOFTMMU.

> > and secondly the purpose of getting rid of smp_foo globals
> > is disentangle layer violations and not replace it with another global
> > (qdev_get_machine()).
> 
> I am happy to follow this rule on cpu-topo refactoring work, but sometimes
> calling qdev_get_machine() is inevitable.

I agree with Igor's goal, but let's do it one step at a time.
Replacing the smp_* globals with qdev_get_machine() is a step in
the right direction, IMO.


> 
> > 
> > What should be done is to make a properties of nr_cores/nr_threads and set
> > them from the parent object that creates CPUs. The point is CPUs shouldn't
> > reach out outside itself to fish out data bits it needs, it's responsibility
> > of creator to feed to being create CPU needed properties.
> > 
> > This kind of refactoring probably deserves its own series and should precede
> > -smp refactoring as it doesn't depend on CpuTopology at all.

I don't see why it should precede -smp refactoring.

We have a very specific user-visible goal here: making one extra
CPU topology option (CPU dies) available on PC only.  Asking the
author to refactor some code while implementing that is
reasonable.  Requiring the author to touch every single place
where a CPU object is created in QEMU just to avoid a
qdev_get_machine() call doesn't seem reasonable.


> > 
> 
> The division of responsibility for this case (refactoring qemu_init_vcpu)
> seems to be a poisonous apple.
> 
> The prerequisite for setting cpu-> nr_cores / nr_threads from the parent is
> that the CPU has been created, so if any process during initialization needs
> this topo information, it will use the default values form
> cpu_common_initfn() instead of user-configured parameters.
> 
> We may not want to repeat those assignment operations using the new values
> and what do you think, Igor?

I believe we need a better CPU creation API that all machines can
use, otherwise all of them will have to duplicate the same code
between object_new() and object_propert_set_bool("realized", true).

But I really don't think designing and implementing this new API
should be a requirement to get useful work done.

-- 
Eduardo

Re: [Qemu-trivial] [Qemu-devel] [PATCH 2/9] cpu/topology: add general support for machine properties

[Eduardo Habkost]

On Thu, May 02, 2019 at 05:09:28PM +0200, Igor Mammedov wrote:
> On Tue, 30 Apr 2019 15:30:31 +0800
> Like Xu <like.xu@linux.intel.com> wrote:
> 
> > On 2019/4/4 22:25, Igor Mammedov wrote:
> > > On Fri, 29 Mar 2019 16:48:38 +0800
> > > Like Xu <like.xu@linux.intel.com> wrote:
> > > 
> 
> [...]
> 
> > 
> > The division of responsibility for this case (refactoring 
> > qemu_init_vcpu) seems to be a poisonous apple.
> > 
> > The prerequisite for setting cpu-> nr_cores / nr_threads from the parent 
> > is that the CPU has been created, so if any process during 
> > initialization needs this topo information, it will use the default 
> > values form cpu_common_initfn() instead of user-configured parameters.
> 
> can you point to concrete place that needs access to nr_cores / nr_threads
> before cpu is 'realized'?

We have very few architectures actually using
nr_cores/nr_threads/smp_cores/smp_threads.  I think those
variables are used only on x86, ppc, and mips.

I believe I suggested some time ago that we should get rid of the
nr_cores/nr_threads CPUState fields and move them to
arch-specific types.  This would help us avoid confusion when
different architectures have different semantics for
nr_cores/nr_threads.

See https://www.mail-archive.com/qemu-devel@nongnu.org/msg587105.html
("[Qemu-devel] Meaning of '-smp threads' on mips_malta") for one
example of confusing arch-specific semantics.

-- 
Eduardo