On Mon, May 6, 2019 at 1:34 AM Like Xu <like...@linux.intel.com> wrote:
>
> It's recommended to access smp variables via MachineState
> as an incoming parameter. This approach applies on legacy
> smbios_*_tables*(), *_machine_reset(), *__hot_add_cpu()
> and related *_create_cpu() for later smp variables usages.
>
> Suggested-by: Igor Mammedov <imamm...@redhat.com>
> Signed-off-by: Like Xu <like...@linux.intel.com>

Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>

Alistair

> ---
>  hw/arm/virt.c                |  2 +-
>  hw/hppa/machine.c            |  2 +-
>  hw/i386/pc.c                 |  9 ++++-----
>  hw/mips/mips_malta.c         | 21 +++++++++++----------
>  hw/ppc/pnv.c                 |  3 +--
>  hw/ppc/spapr.c               |  3 +--
>  hw/s390x/s390-virtio-ccw.c   |  6 +++---
>  hw/smbios/smbios.c           | 26 +++++++++++++++-----------
>  include/hw/boards.h          |  4 ++--
>  include/hw/firmware/smbios.h |  5 +++--
>  include/hw/i386/pc.h         |  2 +-
>  qmp.c                        |  2 +-
>  vl.c                         |  2 +-
>  13 files changed, 45 insertions(+), 42 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 16ba67f..1b02ba4 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1290,7 +1290,7 @@ static void virt_build_smbios(VirtMachineState *vms)
>                          vmc->smbios_old_sys_ver ? "1.0" : mc->name, false,
>                          true, SMBIOS_ENTRY_POINT_30);
>
> -    smbios_get_tables(NULL, 0, &smbios_tables, &smbios_tables_len,
> +    smbios_get_tables(MACHINE(vms), NULL, 0, &smbios_tables, 
> &smbios_tables_len,
>                        &smbios_anchor, &smbios_anchor_len);
>
>      if (smbios_anchor) {
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index d1b1d3c..416e67b 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -240,7 +240,7 @@ static void machine_hppa_init(MachineState *machine)
>      cpu[0]->env.gr[21] = smp_cpus;
>  }
>
> -static void hppa_machine_reset(void)
> +static void hppa_machine_reset(MachineState *ms)
>  {
>      int i;
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d98b737..9bcd867 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -962,7 +962,7 @@ static void pc_build_smbios(PCMachineState *pcms)
>      /* tell smbios about cpuid version and features */
>      smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
>
> -    smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
> +    smbios_tables = smbios_get_table_legacy(ms, &smbios_tables_len);
>      if (smbios_tables) {
>          fw_cfg_add_bytes(pcms->fw_cfg, FW_CFG_SMBIOS_ENTRIES,
>                           smbios_tables, smbios_tables_len);
> @@ -979,7 +979,7 @@ static void pc_build_smbios(PCMachineState *pcms)
>              array_count++;
>          }
>      }
> -    smbios_get_tables(mem_array, array_count,
> +    smbios_get_tables(ms, mem_array, array_count,
>                        &smbios_tables, &smbios_tables_len,
>                        &smbios_anchor, &smbios_anchor_len);
>      g_free(mem_array);
> @@ -1534,9 +1534,8 @@ static void pc_new_cpu(const char *typename, int64_t 
> apic_id, Error **errp)
>      error_propagate(errp, local_err);
>  }
>
> -void pc_hot_add_cpu(const int64_t id, Error **errp)
> +void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp)
>  {
> -    MachineState *ms = MACHINE(qdev_get_machine());
>      int64_t apic_id = x86_cpu_apic_id_from_index(id);
>      Error *local_err = NULL;
>
> @@ -2622,7 +2621,7 @@ static void pc_machine_initfn(Object *obj)
>      pc_system_flash_create(pcms);
>  }
>
> -static void pc_machine_reset(void)
> +static void pc_machine_reset(MachineState *machine)
>  {
>      CPUState *cs;
>      X86CPU *cpu;
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 439665a..534e705 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1124,14 +1124,14 @@ static void main_cpu_reset(void *opaque)
>      }
>  }
>
> -static void create_cpu_without_cps(const char *cpu_type,
> +static void create_cpu_without_cps(MachineState *ms, const char *cpu_type,
>                                     qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>  {
>      CPUMIPSState *env;
>      MIPSCPU *cpu;
>      int i;
>
> -    for (i = 0; i < smp_cpus; i++) {
> +    for (i = 0; i < ms->smp.cpus; i++) {
>          cpu = MIPS_CPU(cpu_create(cpu_type));
>
>          /* Init internal devices */
> @@ -1146,7 +1146,7 @@ static void create_cpu_without_cps(const char *cpu_type,
>      *cbus_irq = env->irq[4];
>  }
>
> -static void create_cps(MaltaState *s, const char *cpu_type,
> +static void create_cps(MachineState *ms, MaltaState *s, const char *cpu_type,
>                         qemu_irq *cbus_irq, qemu_irq *i8259_irq)
>  {
>      Error *err = NULL;
> @@ -1155,7 +1155,7 @@ static void create_cps(MaltaState *s, const char 
> *cpu_type,
>      qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
>
>      object_property_set_str(OBJECT(s->cps), cpu_type, "cpu-type", &err);
> -    object_property_set_int(OBJECT(s->cps), smp_cpus, "num-vp", &err);
> +    object_property_set_int(OBJECT(s->cps), ms->smp.cpus, "num-vp", &err);
>      object_property_set_bool(OBJECT(s->cps), true, "realized", &err);
>      if (err != NULL) {
>          error_report("%s", error_get_pretty(err));
> @@ -1168,13 +1168,14 @@ static void create_cps(MaltaState *s, const char 
> *cpu_type,
>      *cbus_irq = NULL;
>  }
>
> -static void mips_create_cpu(MaltaState *s, const char *cpu_type,
> -                            qemu_irq *cbus_irq, qemu_irq *i8259_irq)
> +static void mips_create_cpu(MachineState *ms, MaltaState *s,
> +                            const char *cpu_type, qemu_irq *cbus_irq,
> +                            qemu_irq *i8259_irq)
>  {
> -    if ((smp_cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
> -        create_cps(s, cpu_type, cbus_irq, i8259_irq);
> +    if ((ms->smp.cpus > 1) && cpu_supports_cps_smp(cpu_type)) {
> +        create_cps(ms, s, cpu_type, cbus_irq, i8259_irq);
>      } else {
> -        create_cpu_without_cps(cpu_type, cbus_irq, i8259_irq);
> +        create_cpu_without_cps(ms, cpu_type, cbus_irq, i8259_irq);
>      }
>  }
>
> @@ -1218,7 +1219,7 @@ void mips_malta_init(MachineState *machine)
>      qdev_init_nofail(dev);
>
>      /* create CPU */
> -    mips_create_cpu(s, machine->cpu_type, &cbus_irq, &i8259_irq);
> +    mips_create_cpu(machine, s, machine->cpu_type, &cbus_irq, &i8259_irq);
>
>      /* allocate RAM */
>      if (ram_size > 2 * GiB) {
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index dfb4ea5..1e8c505 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -520,9 +520,8 @@ static void pnv_powerdown_notify(Notifier *n, void 
> *opaque)
>      }
>  }
>
> -static void pnv_reset(void)
> +static void pnv_reset(MachineState *machine)
>  {
> -    MachineState *machine = MACHINE(qdev_get_machine());
>      PnvMachineState *pnv = PNV_MACHINE(machine);
>      void *fdt;
>      Object *obj;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2ef3ce4..7fb34de 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1689,9 +1689,8 @@ static int spapr_reset_drcs(Object *child, void *opaque)
>      return 0;
>  }
>
> -static void spapr_machine_reset(void)
> +static void spapr_machine_reset(MachineState *machine)
>  {
> -    MachineState *machine = MACHINE(qdev_get_machine());
>      SpaprMachineState *spapr = SPAPR_MACHINE(machine);
>      PowerPCCPU *first_ppc_cpu;
>      uint32_t rtas_limit;
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index bbc6e8f..ed1fe7a 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -338,7 +338,7 @@ static inline void s390_do_cpu_ipl(CPUState *cs, 
> run_on_cpu_data arg)
>      s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>  }
>
> -static void s390_machine_reset(void)
> +static void s390_machine_reset(MachineState *machine)
>  {
>      enum s390_reset reset_type;
>      CPUState *cs, *t;
> @@ -439,9 +439,9 @@ static HotplugHandler 
> *s390_get_hotplug_handler(MachineState *machine,
>      return NULL;
>  }
>
> -static void s390_hot_add_cpu(const int64_t id, Error **errp)
> +static void s390_hot_add_cpu(MachineState *machine,
> +                             const int64_t id, Error **errp)
>  {
> -    MachineState *machine = MACHINE(qdev_get_machine());
>      ObjectClass *oc;
>
>      g_assert(machine->possible_cpus->cpus[0].cpu);
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 47be907..0bf9a01 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -26,6 +26,7 @@
>  #include "sysemu/cpus.h"
>  #include "hw/firmware/smbios.h"
>  #include "hw/loader.h"
> +#include "hw/boards.h"
>  #include "exec/cpu-common.h"
>  #include "smbios_build.h"
>
> @@ -340,9 +341,10 @@ static void smbios_register_config(void)
>
>  opts_init(smbios_register_config);
>
> -static void smbios_validate_table(void)
> +static void smbios_validate_table(MachineState *ms)
>  {
> -    uint32_t expect_t4_count = smbios_legacy ? smp_cpus : smbios_smp_sockets;
> +    uint32_t expect_t4_count = smbios_legacy ?
> +                                        ms->smp.cpus : smbios_smp_sockets;
>
>      if (smbios_type4_count && smbios_type4_count != expect_t4_count) {
>          error_report("Expected %d SMBIOS Type 4 tables, got %d instead",
> @@ -427,7 +429,7 @@ static void smbios_build_type_1_fields(void)
>      }
>  }
>
> -uint8_t *smbios_get_table_legacy(size_t *length)
> +uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length)
>  {
>      if (!smbios_legacy) {
>          *length = 0;
> @@ -437,7 +439,7 @@ uint8_t *smbios_get_table_legacy(size_t *length)
>      if (!smbios_immutable) {
>          smbios_build_type_0_fields();
>          smbios_build_type_1_fields();
> -        smbios_validate_table();
> +        smbios_validate_table(ms);
>          smbios_immutable = true;
>      }
>      *length = smbios_entries_len;
> @@ -569,7 +571,7 @@ static void smbios_build_type_3_table(void)
>      SMBIOS_BUILD_TABLE_POST;
>  }
>
> -static void smbios_build_type_4_table(unsigned instance)
> +static void smbios_build_type_4_table(MachineState *ms, unsigned instance)
>  {
>      char sock_str[128];
>
> @@ -596,8 +598,8 @@ static void smbios_build_type_4_table(unsigned instance)
>      SMBIOS_TABLE_SET_STR(4, serial_number_str, type4.serial);
>      SMBIOS_TABLE_SET_STR(4, asset_tag_number_str, type4.asset);
>      SMBIOS_TABLE_SET_STR(4, part_number_str, type4.part);
> -    t->core_count = t->core_enabled = smp_cores;
> -    t->thread_count = smp_threads;
> +    t->core_count = t->core_enabled = ms->smp.cores;
> +    t->thread_count = ms->smp.threads;
>      t->processor_characteristics = cpu_to_le16(0x02); /* Unknown */
>      t->processor_family2 = cpu_to_le16(0x01); /* Other */
>
> @@ -838,7 +840,8 @@ static void smbios_entry_point_setup(void)
>      }
>  }
>
> -void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> +void smbios_get_tables(MachineState *ms,
> +                       const struct smbios_phys_mem_area *mem_array,
>                         const unsigned int mem_array_size,
>                         uint8_t **tables, size_t *tables_len,
>                         uint8_t **anchor, size_t *anchor_len)
> @@ -857,11 +860,12 @@ void smbios_get_tables(const struct 
> smbios_phys_mem_area *mem_array,
>          smbios_build_type_2_table();
>          smbios_build_type_3_table();
>
> -        smbios_smp_sockets = DIV_ROUND_UP(smp_cpus, smp_cores * smp_threads);
> +        smbios_smp_sockets = DIV_ROUND_UP(ms->smp.cpus,
> +                                          ms->smp.cores * ms->smp.threads);
>          assert(smbios_smp_sockets >= 1);
>
>          for (i = 0; i < smbios_smp_sockets; i++) {
> -            smbios_build_type_4_table(i);
> +            smbios_build_type_4_table(ms, i);
>          }
>
>          smbios_build_type_11_table();
> @@ -887,7 +891,7 @@ void smbios_get_tables(const struct smbios_phys_mem_area 
> *mem_array,
>          smbios_build_type_38_table();
>          smbios_build_type_127_table();
>
> -        smbios_validate_table();
> +        smbios_validate_table(ms);
>          smbios_entry_point_setup();
>          smbios_immutable = true;
>      }
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index dc89c6d..a7eb0bf 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -171,8 +171,8 @@ struct MachineClass {
>      const char *deprecation_reason;
>
>      void (*init)(MachineState *state);
> -    void (*reset)(void);
> -    void (*hot_add_cpu)(const int64_t id, Error **errp);
> +    void (*reset)(MachineState *state);
> +    void (*hot_add_cpu)(MachineState *state, const int64_t id, Error **errp);
>      int (*kvm_type)(MachineState *machine, const char *arg);
>
>      BlockInterfaceType block_default_type;
> diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
> index 6fef32a..02a0ced 100644
> --- a/include/hw/firmware/smbios.h
> +++ b/include/hw/firmware/smbios.h
> @@ -268,8 +268,9 @@ void smbios_set_cpuid(uint32_t version, uint32_t 
> features);
>  void smbios_set_defaults(const char *manufacturer, const char *product,
>                           const char *version, bool legacy_mode,
>                           bool uuid_encoded, SmbiosEntryPointType ep_type);
> -uint8_t *smbios_get_table_legacy(size_t *length);
> -void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
> +uint8_t *smbios_get_table_legacy(MachineState *ms, size_t *length);
> +void smbios_get_tables(MachineState *ms,
> +                       const struct smbios_phys_mem_area *mem_array,
>                         const unsigned int mem_array_size,
>                         uint8_t **tables, size_t *tables_len,
>                         uint8_t **anchor, size_t *anchor_len);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 43df723..ce3c229 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -186,7 +186,7 @@ void pc_register_ferr_irq(qemu_irq irq);
>  void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
>
>  void pc_cpus_init(PCMachineState *pcms);
> -void pc_hot_add_cpu(const int64_t id, Error **errp);
> +void pc_hot_add_cpu(MachineState *ms, const int64_t id, Error **errp);
>
>  void pc_guest_info_init(PCMachineState *pcms);
>
> diff --git a/qmp.c b/qmp.c
> index b92d62c..ba7aa04 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -123,7 +123,7 @@ void qmp_cpu_add(int64_t id, Error **errp)
>
>      mc = MACHINE_GET_CLASS(current_machine);
>      if (mc->hot_add_cpu) {
> -        mc->hot_add_cpu(id, errp);
> +        mc->hot_add_cpu(current_machine, id, errp);
>      } else {
>          error_setg(errp, "Not supported");
>      }
> diff --git a/vl.c b/vl.c
> index 43fd247..34f05b2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1706,7 +1706,7 @@ void qemu_system_reset(ShutdownCause reason)
>      cpu_synchronize_all_states();
>
>      if (mc && mc->reset) {
> -        mc->reset();
> +        mc->reset(current_machine);
>      } else {
>          qemu_devices_reset();
>      }
> --
> 1.8.3.1
>

Reply via email to