Message ID | 648b317918eef89b1fe0147dcdf3d6aeb9ea7ab9.1399974281.git.chen.fan.fnst@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
On Tue, 13 May 2014 18:08:49 +0800 Chen Fan <chen.fan.fnst@cn.fujitsu.com> wrote: > In order to implement adding cpu with device_add, we should make the > check of APIC ID after object_init(), so add UserCreatable complete > method for checking APIC ID availability, and introduce cpu_physid_mask > for saving occupied APIC ID, then we could use -device foo-x86_64-cpu > without setting apic-id property to add default APIC IDs. Currently there is 2 places where APIC id could be checked: x86_cpuid_set_apic_id() - property setter, currently checks for duplicate x86_cpu_realizefn() -> x86_cpu_apic_create() -> qdev_prop_set_uint8(cpu->apic_state, "id", env->cpuid_apic_id); It would be better to add necessary checks there instead of adding yet another bitmap for APIC id to maintain. And I didn't quite get why user_creatable interface was used here, would you elaborate on it? > > Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> > --- > include/qom/cpu.h | 2 ++ > qdev-monitor.c | 11 ++++++ > target-i386/cpu.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++- > target-i386/topology.h | 18 ++++++++++ > 4 files changed, 121 insertions(+), 1 deletion(-) > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index b8f46b1..8ba9f7b 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -151,6 +151,7 @@ typedef struct CPUClass { > const char *gdb_core_xml_file; > > DECLARE_BITMAP(cpu_present_mask, MAX_CPUMASK_BITS); > + DECLARE_BITMAP(cpu_physid_mask, MAX_CPUMASK_BITS); > } CPUClass; > > #ifdef HOST_WORDS_BIGENDIAN > @@ -296,6 +297,7 @@ struct CPUState { > QTAILQ_HEAD(CPUTailQ, CPUState); > extern struct CPUTailQ cpus; > #define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node) > +#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node) > #define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node) > #define CPU_FOREACH_SAFE(cpu, next_cpu) \ > QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu) > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 02cbe43..36c200e 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -24,6 +24,7 @@ > #include "qmp-commands.h" > #include "sysemu/arch_init.h" > #include "qemu/config-file.h" > +#include "qom/object_interfaces.h" > > /* > * Aliases were a bad idea from the start. Let's keep them > @@ -556,6 +557,16 @@ DeviceState *qdev_device_add(QemuOpts *opts) > return NULL; > } > > + user_creatable_complete(OBJECT(dev), &err); > + if (err != NULL) { > + qerror_report_err(err); > + error_free(err); > + object_unparent(OBJECT(dev)); > + object_unref(OBJECT(dev)); > + qerror_report(QERR_DEVICE_INIT_FAILED, driver); > + return NULL; > + } > + > dev->opts = opts; > object_property_set_bool(OBJECT(dev), true, "realized", &err); > if (err != NULL) { > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 8f193a9..56cc3ad 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -48,6 +48,7 @@ > #include "hw/i386/apic_internal.h" > #endif > > +#include "qom/object_interfaces.h" > > /* Cache topology CPUID constants: */ > > @@ -158,7 +159,7 @@ > #define L2_ITLB_4K_ASSOC 4 > #define L2_ITLB_4K_ENTRIES 512 > > - > +static int64_t cpu_2_physid[MAX_CPUMASK_BITS]; > > static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > uint32_t vendor2, uint32_t vendor3) > @@ -1546,12 +1547,16 @@ static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque, > static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, > const char *name, Error **errp) > { > + CPUState *cs = CPU(obj); > + CPUClass *cc = CPU_GET_CLASS(obj); > X86CPU *cpu = X86_CPU(obj); > DeviceState *dev = DEVICE(obj); > const int64_t min = 0; > const int64_t max = UINT32_MAX; > Error *error = NULL; > int64_t value; > + X86CPUTopoInfo topo; > + int64_t phys_id; > > if (dev->realized) { > error_setg(errp, "Attempt to set property '%s' on '%s' after " > @@ -1571,10 +1576,28 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, > return; > } > > + if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) { > + error_setg(errp, "CPU with APIC ID %" PRIi64 > + " is more than MAX APIC ID limits", value); > + return; > + } > + > + x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo); > + if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) { > + error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match " > + "topology configuration.", value); > + return; > + } > + > if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) { > error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value); > return; > } > + > + phys_id = (topo.smt_id + topo.core_id * smp_threads > + + topo.pkg_id * smp_cores * smp_threads); > + set_bit(phys_id, cc->cpu_physid_mask); > + cpu_2_physid[cs->cpu_index] = phys_id; > cpu->env.cpuid_apic_id = value; > } > > @@ -1999,12 +2022,57 @@ out: > return cpu; > } > > +static void x86_cpu_cpudef_instance_init(Object *obj) > +{ > + DeviceState *dev = DEVICE(obj); > + X86CPU *cpu = X86_CPU(obj); > + CPUX86State *env = &cpu->env; > + > + dev->hotplugged = true; > + > + env->cpuid_apic_id = ~0U; > +} > + > +static void x86_cpu_cpudef_complete(UserCreatable *uc, Error **errp) > +{ > + CPUState *cs = CPU(uc); > + X86CPU *cpu = X86_CPU(uc); > + CPUClass *cc = CPU_GET_CLASS(OBJECT(uc)); > + int64_t phys_id; > + > + if (cpu->env.cpuid_apic_id != ~0U) { > + return; > + } > + > + phys_id = find_first_zero_bit(cc->cpu_physid_mask, MAX_CPUMASK_BITS); > + if (phys_id > max_cpus - 1) { > + error_setg(errp, "Unable to add CPU %" PRIi64 " is larger than " > + "max allowed: %d", phys_id, max_cpus - 1); > + return; > + } > + > + set_bit(phys_id, cc->cpu_physid_mask); > + cpu_2_physid[cs->cpu_index] = phys_id; > + cpu->env.cpuid_apic_id = x86_cpu_apic_id_from_index(phys_id); > +} > + > static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data) > { > X86CPUDefinition *cpudef = data; > X86CPUClass *xcc = X86_CPU_CLASS(oc); > + DeviceClass *dc = DEVICE_CLASS(oc); > + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); > + int i; > > xcc->cpu_def = cpudef; > + > + ucc->complete = x86_cpu_cpudef_complete; > + > + dc->cannot_instantiate_with_device_add_yet = false; > + > + for (i = 0; i < MAX_CPUMASK_BITS; i++) { > + cpu_2_physid[i] = -1; > + } > } > > static void x86_register_cpudef_type(X86CPUDefinition *def) > @@ -2013,8 +2081,14 @@ static void x86_register_cpudef_type(X86CPUDefinition *def) > TypeInfo ti = { > .name = typename, > .parent = TYPE_X86_CPU, > + .instance_size = sizeof(X86CPU), > + .instance_init = x86_cpu_cpudef_instance_init, > .class_init = x86_cpu_cpudef_class_init, > .class_data = def, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_USER_CREATABLE }, > + { } > + } > }; > > type_register(&ti); > @@ -2738,6 +2812,20 @@ static void x86_cpu_initfn(Object *obj) > } > } > > +static void x86_cpu_finalizefn(Object *obj) > +{ > + CPUState *cs = CPU(obj); > + CPUClass *cc = CPU_GET_CLASS(obj); > + > + CPU_REMOVE(cs); > + > + clear_bit(cs->cpu_index, cc->cpu_present_mask); > + if (cpu_2_physid[cs->cpu_index] != -1) { > + clear_bit(cpu_2_physid[cs->cpu_index], cc->cpu_physid_mask); > + cpu_2_physid[cs->cpu_index] = -1; > + } > +} > + > static int64_t x86_cpu_get_arch_id(CPUState *cs) > { > X86CPU *cpu = X86_CPU(cs); > @@ -2837,6 +2925,7 @@ static const TypeInfo x86_cpu_type_info = { > .parent = TYPE_CPU, > .instance_size = sizeof(X86CPU), > .instance_init = x86_cpu_initfn, > + .instance_finalize = x86_cpu_finalizefn, > .abstract = true, > .class_size = sizeof(X86CPUClass), > .class_init = x86_cpu_common_class_init, > diff --git a/target-i386/topology.h b/target-i386/topology.h > index e9ff89c..dcb4988 100644 > --- a/target-i386/topology.h > +++ b/target-i386/topology.h > @@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, > return apicid_from_topo_ids(nr_cores, nr_threads, &topo); > } > > +/* Calculate CPU topology based on CPU APIC ID. > + */ > +static inline void x86_topo_ids_from_apic_id(unsigned nr_cores, > + unsigned nr_threads, > + apic_id_t apic_id, > + X86CPUTopoInfo *topo) > +{ > + unsigned offset_mask; > + topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads); > + > + offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1; > + topo->core_id = (apic_id & offset_mask) > + >> apicid_core_offset(nr_cores, nr_threads); > + > + offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1; > + topo->smt_id = apic_id & offset_mask; > +} > + > #endif /* TARGET_I386_TOPOLOGY_H */
diff --git a/include/qom/cpu.h b/include/qom/cpu.h index b8f46b1..8ba9f7b 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -151,6 +151,7 @@ typedef struct CPUClass { const char *gdb_core_xml_file; DECLARE_BITMAP(cpu_present_mask, MAX_CPUMASK_BITS); + DECLARE_BITMAP(cpu_physid_mask, MAX_CPUMASK_BITS); } CPUClass; #ifdef HOST_WORDS_BIGENDIAN @@ -296,6 +297,7 @@ struct CPUState { QTAILQ_HEAD(CPUTailQ, CPUState); extern struct CPUTailQ cpus; #define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node) +#define CPU_REMOVE(cpu) QTAILQ_REMOVE(&cpus, cpu, node) #define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node) #define CPU_FOREACH_SAFE(cpu, next_cpu) \ QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu) diff --git a/qdev-monitor.c b/qdev-monitor.c index 02cbe43..36c200e 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -24,6 +24,7 @@ #include "qmp-commands.h" #include "sysemu/arch_init.h" #include "qemu/config-file.h" +#include "qom/object_interfaces.h" /* * Aliases were a bad idea from the start. Let's keep them @@ -556,6 +557,16 @@ DeviceState *qdev_device_add(QemuOpts *opts) return NULL; } + user_creatable_complete(OBJECT(dev), &err); + if (err != NULL) { + qerror_report_err(err); + error_free(err); + object_unparent(OBJECT(dev)); + object_unref(OBJECT(dev)); + qerror_report(QERR_DEVICE_INIT_FAILED, driver); + return NULL; + } + dev->opts = opts; object_property_set_bool(OBJECT(dev), true, "realized", &err); if (err != NULL) { diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8f193a9..56cc3ad 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -48,6 +48,7 @@ #include "hw/i386/apic_internal.h" #endif +#include "qom/object_interfaces.h" /* Cache topology CPUID constants: */ @@ -158,7 +159,7 @@ #define L2_ITLB_4K_ASSOC 4 #define L2_ITLB_4K_ENTRIES 512 - +static int64_t cpu_2_physid[MAX_CPUMASK_BITS]; static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, uint32_t vendor2, uint32_t vendor3) @@ -1546,12 +1547,16 @@ static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque, static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { + CPUState *cs = CPU(obj); + CPUClass *cc = CPU_GET_CLASS(obj); X86CPU *cpu = X86_CPU(obj); DeviceState *dev = DEVICE(obj); const int64_t min = 0; const int64_t max = UINT32_MAX; Error *error = NULL; int64_t value; + X86CPUTopoInfo topo; + int64_t phys_id; if (dev->realized) { error_setg(errp, "Attempt to set property '%s' on '%s' after " @@ -1571,10 +1576,28 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, void *opaque, return; } + if (value > x86_cpu_apic_id_from_index(max_cpus - 1)) { + error_setg(errp, "CPU with APIC ID %" PRIi64 + " is more than MAX APIC ID limits", value); + return; + } + + x86_topo_ids_from_apic_id(smp_cores, smp_threads, value, &topo); + if (topo.smt_id >= smp_threads || topo.core_id >= smp_cores) { + error_setg(errp, "CPU with APIC ID %" PRIi64 " does not match " + "topology configuration.", value); + return; + } + if ((value != cpu->env.cpuid_apic_id) && cpu_exists(value)) { error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value); return; } + + phys_id = (topo.smt_id + topo.core_id * smp_threads + + topo.pkg_id * smp_cores * smp_threads); + set_bit(phys_id, cc->cpu_physid_mask); + cpu_2_physid[cs->cpu_index] = phys_id; cpu->env.cpuid_apic_id = value; } @@ -1999,12 +2022,57 @@ out: return cpu; } +static void x86_cpu_cpudef_instance_init(Object *obj) +{ + DeviceState *dev = DEVICE(obj); + X86CPU *cpu = X86_CPU(obj); + CPUX86State *env = &cpu->env; + + dev->hotplugged = true; + + env->cpuid_apic_id = ~0U; +} + +static void x86_cpu_cpudef_complete(UserCreatable *uc, Error **errp) +{ + CPUState *cs = CPU(uc); + X86CPU *cpu = X86_CPU(uc); + CPUClass *cc = CPU_GET_CLASS(OBJECT(uc)); + int64_t phys_id; + + if (cpu->env.cpuid_apic_id != ~0U) { + return; + } + + phys_id = find_first_zero_bit(cc->cpu_physid_mask, MAX_CPUMASK_BITS); + if (phys_id > max_cpus - 1) { + error_setg(errp, "Unable to add CPU %" PRIi64 " is larger than " + "max allowed: %d", phys_id, max_cpus - 1); + return; + } + + set_bit(phys_id, cc->cpu_physid_mask); + cpu_2_physid[cs->cpu_index] = phys_id; + cpu->env.cpuid_apic_id = x86_cpu_apic_id_from_index(phys_id); +} + static void x86_cpu_cpudef_class_init(ObjectClass *oc, void *data) { X86CPUDefinition *cpudef = data; X86CPUClass *xcc = X86_CPU_CLASS(oc); + DeviceClass *dc = DEVICE_CLASS(oc); + UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); + int i; xcc->cpu_def = cpudef; + + ucc->complete = x86_cpu_cpudef_complete; + + dc->cannot_instantiate_with_device_add_yet = false; + + for (i = 0; i < MAX_CPUMASK_BITS; i++) { + cpu_2_physid[i] = -1; + } } static void x86_register_cpudef_type(X86CPUDefinition *def) @@ -2013,8 +2081,14 @@ static void x86_register_cpudef_type(X86CPUDefinition *def) TypeInfo ti = { .name = typename, .parent = TYPE_X86_CPU, + .instance_size = sizeof(X86CPU), + .instance_init = x86_cpu_cpudef_instance_init, .class_init = x86_cpu_cpudef_class_init, .class_data = def, + .interfaces = (InterfaceInfo[]) { + { TYPE_USER_CREATABLE }, + { } + } }; type_register(&ti); @@ -2738,6 +2812,20 @@ static void x86_cpu_initfn(Object *obj) } } +static void x86_cpu_finalizefn(Object *obj) +{ + CPUState *cs = CPU(obj); + CPUClass *cc = CPU_GET_CLASS(obj); + + CPU_REMOVE(cs); + + clear_bit(cs->cpu_index, cc->cpu_present_mask); + if (cpu_2_physid[cs->cpu_index] != -1) { + clear_bit(cpu_2_physid[cs->cpu_index], cc->cpu_physid_mask); + cpu_2_physid[cs->cpu_index] = -1; + } +} + static int64_t x86_cpu_get_arch_id(CPUState *cs) { X86CPU *cpu = X86_CPU(cs); @@ -2837,6 +2925,7 @@ static const TypeInfo x86_cpu_type_info = { .parent = TYPE_CPU, .instance_size = sizeof(X86CPU), .instance_init = x86_cpu_initfn, + .instance_finalize = x86_cpu_finalizefn, .abstract = true, .class_size = sizeof(X86CPUClass), .class_init = x86_cpu_common_class_init, diff --git a/target-i386/topology.h b/target-i386/topology.h index e9ff89c..dcb4988 100644 --- a/target-i386/topology.h +++ b/target-i386/topology.h @@ -132,4 +132,22 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, return apicid_from_topo_ids(nr_cores, nr_threads, &topo); } +/* Calculate CPU topology based on CPU APIC ID. + */ +static inline void x86_topo_ids_from_apic_id(unsigned nr_cores, + unsigned nr_threads, + apic_id_t apic_id, + X86CPUTopoInfo *topo) +{ + unsigned offset_mask; + topo->pkg_id = apic_id >> apicid_pkg_offset(nr_cores, nr_threads); + + offset_mask = (1L << apicid_pkg_offset(nr_cores, nr_threads)) - 1; + topo->core_id = (apic_id & offset_mask) + >> apicid_core_offset(nr_cores, nr_threads); + + offset_mask = (1L << apicid_core_offset(nr_cores, nr_threads)) - 1; + topo->smt_id = apic_id & offset_mask; +} + #endif /* TARGET_I386_TOPOLOGY_H */
In order to implement adding cpu with device_add, we should make the check of APIC ID after object_init(), so add UserCreatable complete method for checking APIC ID availability, and introduce cpu_physid_mask for saving occupied APIC ID, then we could use -device foo-x86_64-cpu without setting apic-id property to add default APIC IDs. Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com> --- include/qom/cpu.h | 2 ++ qdev-monitor.c | 11 ++++++ target-i386/cpu.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++- target-i386/topology.h | 18 ++++++++++ 4 files changed, 121 insertions(+), 1 deletion(-)