Re: [Qemu-devel] [PATCH 1/1] s390-cpu: qom interface for S390 cpu states array

[Paolo Bonzini]

Il 07/03/2014 17:29, Jason J. Herne ha scritto:
> From: "Jason J. Herne" <address@hidden>
>
> Rename the S390 ipi_states array to cpu_states to better reflect its contents.
>
> Create machine/cpu[cpu_addr] links within the qom tree when creating a new cpu.
>
> Encapsulate the qom tree linking process and the management of the cpu_states
> array into helper functions.
>
> Signed-off-by: Jason J. Herne <address@hidden>
> ---
>  hw/s390x/s390-virtio.c | 30 ++++++++++++++++++++++++------
>  target-s390x/cpu.h     |  1 +
>  2 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
> index 9eeda97..82411e7 100644
> --- a/hw/s390x/s390-virtio.c
> +++ b/hw/s390x/s390-virtio.c
> @@ -52,15 +52,33 @@
>  #define ZIPL_FILENAME                   "s390-zipl.rom"
>
>  static VirtIOS390Bus *s390_bus;
> -static S390CPU **ipi_states;
> +static S390CPU **cpu_states;
>
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
>  {
> +    gchar *name;
> +    Object *cpu;
> +
>      if (cpu_addr >= smp_cpus) {
>          return NULL;
>      }
>
> -    return ipi_states[cpu_addr];
> +    name = g_strdup_printf("cpu[%i]", cpu_addr);
> +    cpu = object_property_get_link(qdev_get_machine(), name, NULL);
> +
> +    g_free(name);
> +    return S390_CPU(cpu);
> +}

QOM is too slow to be used in the data path.

I don't think you want a malloc + a linear scan of an array in 
css_inject_io_interrupt, so you should keep using cpu_states here.

Paolo

> +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state)
> +{
> +    gchar *name;
> +
> +    cpu_states[cpu_addr] = state;
> +    name = g_strdup_printf("cpu[%i]", cpu_addr);
> +    object_property_add_link(qdev_get_machine(), name, TYPE_S390_CPU,
> +                            (Object **) &cpu_states[cpu_addr], NULL);
> +    g_free(name);
>  }
>
>  static int s390_virtio_hcall_notify(const uint64_t *args)
> @@ -184,16 +202,16 @@ void s390_init_cpus(const char *cpu_model, uint8_t 
> *storage_keys)
>          cpu_model = "host";
>      }
>
> -    ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
> +    cpu_states = g_malloc(sizeof(S390CPU *) * smp_cpus);
>
>      for (i = 0; i < smp_cpus; i++) {
> -        S390CPU *cpu;
> +        S390CPU* cpu;
>          CPUState *cs;
>
>          cpu = cpu_s390x_init(cpu_model);
> -        cs = CPU(cpu);
> +        s390_cpu_set_cpustate(i, cpu);
>
> -        ipi_states[i] = cpu;
> +        cs = CPU(cpu_states[i]);
>          cs->halted = 1;
>          cpu->env.exception_index = EXCP_HLT;
>          cpu->env.storage_keys = storage_keys;
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 96c2b4a..6ce3b64 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -370,6 +370,7 @@ static inline void kvm_s390_interrupt_internal(S390CPU 
> *cpu, int type,
>  }
>  #endif
>  S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
> +void s390_cpu_set_cpustate(uint16_t cpu_addr, S390CPU *state);
>  void s390_add_running_cpu(S390CPU *cpu);
>  unsigned s390_del_running_cpu(S390CPU *cpu);