Am 16.04.2013 00:12, schrieb Igor Mammedov:
> ... and use it from board level to set APIC ID for CPUs
> it creates.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> Note:
> * pc_new_cpu() function will be reused later in CPU hot-plug hook.
Suggest as main commit message to avoid the "...":
The property is used from board level to set APIC ID for CPUs it
creates. Do so in a new pc_new_cpu() helper, to be reused for hot-plug.
>
> v3:
> * user error_propagate() in property setter
> v2:
> * use generic cpu_exists() instead of custom one
> * make apic-id dynamic property, so it won't be possible to use it
> as global property, since it's instance specific
> ---
> hw/i386/pc.c | 25 ++++++++++++++++++++++++-
> target-i386/cpu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index dc1a78b..cb57878 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -889,9 +889,29 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> }
> }
>
> +static void pc_new_cpu(const char *cpu_model, int64_t apic_id, Error **errp)
> +{
> + X86CPU *cpu;
> +
> + cpu = cpu_x86_create(cpu_model, errp);
> + if (!cpu) {
> + return;
> + }
> +
> + object_property_set_int(OBJECT(cpu), apic_id, "apic-id", errp);
> + object_property_set_bool(OBJECT(cpu), true, "realized", errp);
> +
> + if (error_is_set(errp)) {
Please use a local Error* variable with error_propagate, otherwise this
is fragile.
> + if (cpu != NULL) {
> + object_unref(OBJECT(cpu));
> + }
> + }
> +}
> +
> void pc_cpus_init(const char *cpu_model)
> {
> int i;
> + Error *error = NULL;
>
> /* init CPUs */
> if (cpu_model == NULL) {
> @@ -903,7 +923,10 @@ void pc_cpus_init(const char *cpu_model)
> }
>
> for (i = 0; i < smp_cpus; i++) {
> - if (!cpu_x86_init(cpu_model)) {
> + pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), &error);
> + if (error) {
> + fprintf(stderr, "%s\n", error_get_pretty(error));
> + error_free(error);
> exit(1);
> }
> }
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index a415fa3..6d6c527 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1271,6 +1271,45 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
> Visitor *v, void *opaque,
> cpu->env.tsc_khz = value / 1000;
> }
>
> +static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + X86CPU *cpu = X86_CPU(obj);
> + int64_t value = cpu->env.cpuid_apic_id;
> +
> + visit_type_int(v, &value, name, errp);
> +}
> +
> +static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque,
> + const char *name, Error **errp)
> +{
> + X86CPU *cpu = X86_CPU(obj);
> + const int64_t min = 0;
> + const int64_t max = UINT32_MAX;
> + Error *error = NULL;
> + int64_t value;
> +
> + visit_type_int(v, &value, name, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> + if (value < min || value > max) {
> + error_setg(&error, "Property %s.%s doesn't take value %" PRId64
> + " (minimum: %" PRId64 ", maximum: %" PRId64 ")" ,
> + object_get_typename(obj), name, value, min, max);
> + error_propagate(errp, error);
> + return;
> + }
> +
> + if (cpu_exists(value)) {
> + error_setg(&error, "CPU with APIC ID %" PRIi64 " exists", value);
> + error_propagate(errp, error);
You could do both error_setg()s directly on errp.
> + return;
> + }
> + cpu->env.cpuid_apic_id = value;
> +}
> +
> static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *name)
> {
> x86_def_t *def;
> @@ -2259,6 +2298,9 @@ static void x86_cpu_initfn(Object *obj)
> object_property_add(obj, "tsc-frequency", "int",
> x86_cpuid_get_tsc_freq,
> x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> + object_property_add(obj, "apic-id", "int",
> + x86_cpuid_get_apic_id,
> + x86_cpuid_set_apic_id, NULL, NULL, NULL);
>
> env->cpuid_apic_id = x86_cpu_apic_id_from_index(cs->cpu_index);
>
Otherwise I like this x86-specific version now!
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
