QOMfy cpu models handling introducing propper cpu types
for each cpu model.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
with this and conversion of features to properties,
it would be possible to replace cpu_sparc_init() with
cpu_generic_init() and reuse common -cpu handling
infrastructure.
CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
CC: Artyom Tarasenko <atar4qemu@gmail.com>
CC: Philippe Mathieu-Daudé <f4bug@amsat.org>
v2:
* make base class abstract (Philippe Mathieu-Daudé <f4bug@amsat.org>)
---
target/sparc/cpu-qom.h | 2 +
target/sparc/cpu.c | 121 +++++++++++++++++++++++++++++++++----------------
2 files changed, 84 insertions(+), 39 deletions(-)
diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
index f63af72..af6d57a 100644
--- a/target/sparc/cpu-qom.h
+++ b/target/sparc/cpu-qom.h
@@ -35,6 +35,7 @@
#define SPARC_CPU_GET_CLASS(obj) \
OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
+typedef struct sparc_def_t sparc_def_t;
/**
* SPARCCPUClass:
* @parent_realize: The parent class' realize handler.
@@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
DeviceRealize parent_realize;
void (*parent_reset)(CPUState *cpu);
+ sparc_def_t *cpu_def;
} SPARCCPUClass;
typedef struct SPARCCPU SPARCCPU;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index d606eb5..2994c09 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -25,8 +25,6 @@
//#define DEBUG_FEATURES
-static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *cpu_model);
-
/* CPUClass::reset() */
static void sparc_cpu_reset(CPUState *s)
{
@@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
{
CPUSPARCState *env = &cpu->env;
char *s = g_strdup(cpu_model);
- char *featurestr, *name = strtok(s, ",");
- sparc_def_t def1, *def = &def1;
+ char *featurestr = strtok(s, ",");
Error *err = NULL;
- if (cpu_sparc_find_by_name(def, name) < 0) {
- g_free(s);
- return -1;
- }
-
- env->def = g_memdup(def, sizeof(*def));
-
featurestr = strtok(NULL, ",");
sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
g_free(s);
@@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
return -1;
}
- env->version = def->iu_version;
- env->fsr = def->fpu_version;
- env->nwindows = def->nwindows;
+ env->version = env->def->iu_version;
+ env->fsr = env->def->fpu_version;
+ env->nwindows = env->def->nwindows;
#if !defined(TARGET_SPARC64)
- env->mmuregs[0] |= def->mmu_version;
+ env->mmuregs[0] |= env->def->mmu_version;
cpu_sparc_set_id(env, 0);
- env->mxccregs[7] |= def->mxcc_version;
+ env->mxccregs[7] |= env->def->mxcc_version;
#else
- env->mmu_version = def->mmu_version;
- env->maxtl = def->maxtl;
- env->version |= def->maxtl << 8;
- env->version |= def->nwindows - 1;
+ env->mmu_version = env->def->mmu_version;
+ env->maxtl = env->def->maxtl;
+ env->version |= env->def->maxtl << 8;
+ env->version |= env->def->nwindows - 1;
#endif
return 0;
}
@@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
SPARCCPU *cpu_sparc_init(const char *cpu_model)
{
SPARCCPU *cpu;
+ ObjectClass *oc;
+ char *str, *name;
+
+ str = g_strdup(cpu_model);
+ name = strtok(str, ",");
+ oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
+ if (oc == NULL) {
+ g_free(str);
+ return NULL;
+ }
+ g_free(str);
- cpu = SPARC_CPU(object_new(TYPE_SPARC_CPU));
+ cpu = SPARC_CPU(object_new(object_class_get_name(oc)));
if (cpu_sparc_register(cpu, cpu_model) < 0) {
object_unref(OBJECT(cpu));
@@ -553,23 +554,6 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features)
error_report("CPU feature %s not found", flagname);
}
-static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *name)
-{
- unsigned int i;
- const sparc_def_t *def = NULL;
-
- for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
- if (strcasecmp(name, sparc_defs[i].name) == 0) {
- def = &sparc_defs[i];
- }
- }
- if (!def) {
- return -1;
- }
- memcpy(cpu_def, def, sizeof(*def));
- return 0;
-}
-
static void sparc_cpu_parse_features(CPUState *cs, char *features,
Error **errp)
{
@@ -796,6 +780,36 @@ static bool sparc_cpu_has_work(CPUState *cs)
cpu_interrupts_enabled(env);
}
+static char *sparc_cpu_type_name(const char *cpu_model)
+{
+ char *name = g_strdup_printf("%s-" TYPE_SPARC_CPU, cpu_model);
+ char *s = name;
+
+ /* SPARC cpu model names happen to have whitespaces,
+ * as type names shouldn't have spaces replace them with '-'
+ */
+ while ((s = strchr(s, ' '))) {
+ *s = '-';
+ }
+
+ return name;
+}
+
+static ObjectClass *sparc_cpu_class_by_name(const char *cpu_model)
+{
+ ObjectClass *oc;
+ char *typename;
+
+ if (cpu_model == NULL) {
+ return NULL;
+ }
+
+ typename = sparc_cpu_type_name(cpu_model);
+ oc = object_class_by_name(typename);
+ g_free(typename);
+ return oc;
+}
+
static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
@@ -825,6 +839,7 @@ static void sparc_cpu_initfn(Object *obj)
{
CPUState *cs = CPU(obj);
SPARCCPU *cpu = SPARC_CPU(obj);
+ SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(obj);
CPUSPARCState *env = &cpu->env;
cs->env_ptr = env;
@@ -832,6 +847,8 @@ static void sparc_cpu_initfn(Object *obj)
if (tcg_enabled()) {
gen_intermediate_code_init(env);
}
+
+ env->def = g_memdup(scc->cpu_def, sizeof(*scc->cpu_def));
}
static void sparc_cpu_uninitfn(Object *obj)
@@ -854,6 +871,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
scc->parent_reset = cc->reset;
cc->reset = sparc_cpu_reset;
+ cc->class_by_name = sparc_cpu_class_by_name;
cc->has_work = sparc_cpu_has_work;
cc->do_interrupt = sparc_cpu_do_interrupt;
cc->cpu_exec_interrupt = sparc_cpu_exec_interrupt;
@@ -888,14 +906,39 @@ static const TypeInfo sparc_cpu_type_info = {
.instance_size = sizeof(SPARCCPU),
.instance_init = sparc_cpu_initfn,
.instance_finalize = sparc_cpu_uninitfn,
- .abstract = false,
+ .abstract = true,
.class_size = sizeof(SPARCCPUClass),
.class_init = sparc_cpu_class_init,
};
+static void sparc_cpu_cpudef_class_init(ObjectClass *oc, void *data)
+{
+ SPARCCPUClass *scc = SPARC_CPU_CLASS(oc);
+ scc->cpu_def = data;
+}
+
+static void sparc_register_cpudef_type(const struct sparc_def_t *def)
+{
+ char *typename = sparc_cpu_type_name(def->name);
+ TypeInfo ti = {
+ .name = typename,
+ .parent = TYPE_SPARC_CPU,
+ .class_init = sparc_cpu_cpudef_class_init,
+ .class_data = (void *)def,
+ };
+
+ type_register(&ti);
+ g_free(typename);
+}
+
static void sparc_cpu_register_types(void)
{
+ int i;
+
type_register_static(&sparc_cpu_type_info);
+ for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
+ sparc_register_cpudef_type(&sparc_defs[i]);
+ }
}
type_init(sparc_cpu_register_types)
--
2.7.4
Hi Igor,
On 08/18/2017 07:08 AM, Igor Mammedov wrote:
> QOMfy cpu models handling introducing propper cpu types
> for each cpu model.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> with this and conversion of features to properties,
> it would be possible to replace cpu_sparc_init() with
> cpu_generic_init() and reuse common -cpu handling
> infrastructure.
>
> CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> CC: Artyom Tarasenko <atar4qemu@gmail.com>
> CC: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> v2:
> * make base class abstract (Philippe Mathieu-Daudé <f4bug@amsat.org>)
> ---
> target/sparc/cpu-qom.h | 2 +
> target/sparc/cpu.c | 121 +++++++++++++++++++++++++++++++++----------------
> 2 files changed, 84 insertions(+), 39 deletions(-)
>
> diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
> index f63af72..af6d57a 100644
> --- a/target/sparc/cpu-qom.h
> +++ b/target/sparc/cpu-qom.h
> @@ -35,6 +35,7 @@
> #define SPARC_CPU_GET_CLASS(obj) \
> OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
>
> +typedef struct sparc_def_t sparc_def_t;
> /**
> * SPARCCPUClass:
> * @parent_realize: The parent class' realize handler.
> @@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
>
> DeviceRealize parent_realize;
> void (*parent_reset)(CPUState *cpu);
> + sparc_def_t *cpu_def;
> } SPARCCPUClass;
>
> typedef struct SPARCCPU SPARCCPU;
> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> index d606eb5..2994c09 100644
> --- a/target/sparc/cpu.c
> +++ b/target/sparc/cpu.c
> @@ -25,8 +25,6 @@
>
> //#define DEBUG_FEATURES
>
> -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *cpu_model);
> -
> /* CPUClass::reset() */
> static void sparc_cpu_reset(CPUState *s)
> {
> @@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> {
> CPUSPARCState *env = &cpu->env;
> char *s = g_strdup(cpu_model);
> - char *featurestr, *name = strtok(s, ",");
> - sparc_def_t def1, *def = &def1;
> + char *featurestr = strtok(s, ",");
> Error *err = NULL;
>
> - if (cpu_sparc_find_by_name(def, name) < 0) {
> - g_free(s);
> - return -1;
> - }
> -
> - env->def = g_memdup(def, sizeof(*def));
> -
> featurestr = strtok(NULL, ",");
> sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
> g_free(s);
> @@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> return -1;
> }
>
> - env->version = def->iu_version;
> - env->fsr = def->fpu_version;
> - env->nwindows = def->nwindows;
> + env->version = env->def->iu_version;
> + env->fsr = env->def->fpu_version;
> + env->nwindows = env->def->nwindows;
> #if !defined(TARGET_SPARC64)
> - env->mmuregs[0] |= def->mmu_version;
> + env->mmuregs[0] |= env->def->mmu_version;
> cpu_sparc_set_id(env, 0);
> - env->mxccregs[7] |= def->mxcc_version;
> + env->mxccregs[7] |= env->def->mxcc_version;
> #else
> - env->mmu_version = def->mmu_version;
> - env->maxtl = def->maxtl;
> - env->version |= def->maxtl << 8;
> - env->version |= def->nwindows - 1;
> + env->mmu_version = env->def->mmu_version;
> + env->maxtl = env->def->maxtl;
> + env->version |= env->def->maxtl << 8;
> + env->version |= env->def->nwindows - 1;
> #endif
> return 0;
> }
> @@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> SPARCCPU *cpu_sparc_init(const char *cpu_model)
> {
> SPARCCPU *cpu;
> + ObjectClass *oc;
> + char *str, *name;
> +
> + str = g_strdup(cpu_model);
> + name = strtok(str, ",");
you can free 'str' once here:
g_free(str);
> + oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
> + if (oc == NULL) {
> + g_free(str);
drop
> + return NULL;
> + }
> + g_free(str);
drop
>
> - cpu = SPARC_CPU(object_new(TYPE_SPARC_CPU));
> + cpu = SPARC_CPU(object_new(object_class_get_name(oc)));
>
> if (cpu_sparc_register(cpu, cpu_model) < 0) {
> object_unref(OBJECT(cpu));
> @@ -553,23 +554,6 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features)
> error_report("CPU feature %s not found", flagname);
> }
>
> -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *name)
> -{
> - unsigned int i;
> - const sparc_def_t *def = NULL;
> -
> - for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
> - if (strcasecmp(name, sparc_defs[i].name) == 0) {
> - def = &sparc_defs[i];
> - }
> - }
> - if (!def) {
> - return -1;
> - }
> - memcpy(cpu_def, def, sizeof(*def));
> - return 0;
> -}
> -
> static void sparc_cpu_parse_features(CPUState *cs, char *features,
> Error **errp)
> {
> @@ -796,6 +780,36 @@ static bool sparc_cpu_has_work(CPUState *cs)
> cpu_interrupts_enabled(env);
> }
>
> +static char *sparc_cpu_type_name(const char *cpu_model)
> +{
> + char *name = g_strdup_printf("%s-" TYPE_SPARC_CPU, cpu_model);
> + char *s = name;
> +
> + /* SPARC cpu model names happen to have whitespaces,
> + * as type names shouldn't have spaces replace them with '-'
> + */
> + while ((s = strchr(s, ' '))) {
> + *s = '-';
> + }
> +
> + return name;
> +}
> +
> +static ObjectClass *sparc_cpu_class_by_name(const char *cpu_model)
> +{
> + ObjectClass *oc;
> + char *typename;
> +
> + if (cpu_model == NULL) {
> + return NULL;
> + }
> +
> + typename = sparc_cpu_type_name(cpu_model);
> + oc = object_class_by_name(typename);
> + g_free(typename);
> + return oc;
> +}
> +
> static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cs = CPU(dev);
> @@ -825,6 +839,7 @@ static void sparc_cpu_initfn(Object *obj)
> {
> CPUState *cs = CPU(obj);
> SPARCCPU *cpu = SPARC_CPU(obj);
> + SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(obj);
> CPUSPARCState *env = &cpu->env;
>
> cs->env_ptr = env;
> @@ -832,6 +847,8 @@ static void sparc_cpu_initfn(Object *obj)
> if (tcg_enabled()) {
> gen_intermediate_code_init(env);
> }
> +
> + env->def = g_memdup(scc->cpu_def, sizeof(*scc->cpu_def));
> }
>
> static void sparc_cpu_uninitfn(Object *obj)
> @@ -854,6 +871,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
> scc->parent_reset = cc->reset;
> cc->reset = sparc_cpu_reset;
>
> + cc->class_by_name = sparc_cpu_class_by_name;
> cc->has_work = sparc_cpu_has_work;
> cc->do_interrupt = sparc_cpu_do_interrupt;
> cc->cpu_exec_interrupt = sparc_cpu_exec_interrupt;
> @@ -888,14 +906,39 @@ static const TypeInfo sparc_cpu_type_info = {
> .instance_size = sizeof(SPARCCPU),
> .instance_init = sparc_cpu_initfn,
> .instance_finalize = sparc_cpu_uninitfn,
> - .abstract = false,
> + .abstract = true,
> .class_size = sizeof(SPARCCPUClass),
> .class_init = sparc_cpu_class_init,
> };
>
> +static void sparc_cpu_cpudef_class_init(ObjectClass *oc, void *data)
> +{
> + SPARCCPUClass *scc = SPARC_CPU_CLASS(oc);
> + scc->cpu_def = data;
> +}
> +
> +static void sparc_register_cpudef_type(const struct sparc_def_t *def)
> +{
> + char *typename = sparc_cpu_type_name(def->name);
> + TypeInfo ti = {
> + .name = typename,
> + .parent = TYPE_SPARC_CPU,
> + .class_init = sparc_cpu_cpudef_class_init,
> + .class_data = (void *)def,
> + };
> +
> + type_register(&ti);
> + g_free(typename);
> +}
> +
> static void sparc_cpu_register_types(void)
> {
> + int i;
> +
> type_register_static(&sparc_cpu_type_info);
> + for (i = 0; i < ARRAY_SIZE(sparc_defs); i++) {
> + sparc_register_cpudef_type(&sparc_defs[i]);
> + }
> }
>
> type_init(sparc_cpu_register_types)
>
Anyway I can't start Aurelien's image [1] with this commit:
$ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
qemu: Unable to find Sparc CPU definition
[1] https://people.debian.org/~aurel32/qemu/sparc/
On Fri, 18 Aug 2017 16:00:37 -0300
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Igor,
>
> On 08/18/2017 07:08 AM, Igor Mammedov wrote:
> > QOMfy cpu models handling introducing propper cpu types
> > for each cpu model.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > with this and conversion of features to properties,
> > it would be possible to replace cpu_sparc_init() with
> > cpu_generic_init() and reuse common -cpu handling
> > infrastructure.
> >
> > CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > CC: Artyom Tarasenko <atar4qemu@gmail.com>
> > CC: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >
> > v2:
> > * make base class abstract (Philippe Mathieu-Daudé <f4bug@amsat.org>)
> > ---
> > target/sparc/cpu-qom.h | 2 +
> > target/sparc/cpu.c | 121 +++++++++++++++++++++++++++++++++----------------
> > 2 files changed, 84 insertions(+), 39 deletions(-)
> >
> > diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
> > index f63af72..af6d57a 100644
> > --- a/target/sparc/cpu-qom.h
> > +++ b/target/sparc/cpu-qom.h
> > @@ -35,6 +35,7 @@
> > #define SPARC_CPU_GET_CLASS(obj) \
> > OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
> >
> > +typedef struct sparc_def_t sparc_def_t;
> > /**
> > * SPARCCPUClass:
> > * @parent_realize: The parent class' realize handler.
> > @@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
> >
> > DeviceRealize parent_realize;
> > void (*parent_reset)(CPUState *cpu);
> > + sparc_def_t *cpu_def;
> > } SPARCCPUClass;
> >
> > typedef struct SPARCCPU SPARCCPU;
> > diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
> > index d606eb5..2994c09 100644
> > --- a/target/sparc/cpu.c
> > +++ b/target/sparc/cpu.c
> > @@ -25,8 +25,6 @@
> >
> > //#define DEBUG_FEATURES
> >
> > -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *cpu_model);
> > -
> > /* CPUClass::reset() */
> > static void sparc_cpu_reset(CPUState *s)
> > {
> > @@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> > {
> > CPUSPARCState *env = &cpu->env;
> > char *s = g_strdup(cpu_model);
> > - char *featurestr, *name = strtok(s, ",");
> > - sparc_def_t def1, *def = &def1;
> > + char *featurestr = strtok(s, ",");
> > Error *err = NULL;
> >
> > - if (cpu_sparc_find_by_name(def, name) < 0) {
> > - g_free(s);
> > - return -1;
> > - }
> > -
> > - env->def = g_memdup(def, sizeof(*def));
> > -
> > featurestr = strtok(NULL, ",");
> > sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
> > g_free(s);
> > @@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> > return -1;
> > }
> >
> > - env->version = def->iu_version;
> > - env->fsr = def->fpu_version;
> > - env->nwindows = def->nwindows;
> > + env->version = env->def->iu_version;
> > + env->fsr = env->def->fpu_version;
> > + env->nwindows = env->def->nwindows;
> > #if !defined(TARGET_SPARC64)
> > - env->mmuregs[0] |= def->mmu_version;
> > + env->mmuregs[0] |= env->def->mmu_version;
> > cpu_sparc_set_id(env, 0);
> > - env->mxccregs[7] |= def->mxcc_version;
> > + env->mxccregs[7] |= env->def->mxcc_version;
> > #else
> > - env->mmu_version = def->mmu_version;
> > - env->maxtl = def->maxtl;
> > - env->version |= def->maxtl << 8;
> > - env->version |= def->nwindows - 1;
> > + env->mmu_version = env->def->mmu_version;
> > + env->maxtl = env->def->maxtl;
> > + env->version |= env->def->maxtl << 8;
> > + env->version |= env->def->nwindows - 1;
> > #endif
> > return 0;
> > }
> > @@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
> > SPARCCPU *cpu_sparc_init(const char *cpu_model)
> > {
> > SPARCCPU *cpu;
> > + ObjectClass *oc;
> > + char *str, *name;
> > +
> > + str = g_strdup(cpu_model);
> > + name = strtok(str, ",");
>
> you can free 'str' once here:
>
> g_free(str);
>
> > + oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
> > + if (oc == NULL) {
> > + g_free(str);
>
> drop
>
> > + return NULL;
> > + }
> > + g_free(str);
>
> drop
I'll fix it up if I have to respin series (note this function is removed within series)
[...]
> Anyway I can't start Aurelien's image [1] with this commit:
Can't reproduce it locally (either dirty/clean tree rebuild)
with ./configure --enable-debug
> $ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
> qemu: Unable to find Sparc CPU definition
for this kind of error guest image doesn't matter as error happens during machine_init()
> [1] https://people.debian.org/~aurel32/qemu/sparc/
It boots just fine with default CPU model:
./sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
and with explicitly specified one:
sparc-softmmu/qemu-system-sparc -cpu "Fujitsu MB86904"
I get from "info qom-tree"
...
/device[0] (Fujitsu-MB86904-sparc-cpu)
...
>> On 08/18/2017 07:08 AM, Igor Mammedov wrote:
>>> QOMfy cpu models handling introducing propper cpu types
>>> for each cpu model.
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>> with this and conversion of features to properties,
>>> it would be possible to replace cpu_sparc_init() with
>>> cpu_generic_init() and reuse common -cpu handling
>>> infrastructure.
>>>
>>> CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> CC: Artyom Tarasenko <atar4qemu@gmail.com>
>>> CC: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>> v2:
>>> * make base class abstract (Philippe Mathieu-Daudé <f4bug@amsat.org>)
>>> ---
>>> target/sparc/cpu-qom.h | 2 +
>>> target/sparc/cpu.c | 121 +++++++++++++++++++++++++++++++++----------------
>>> 2 files changed, 84 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
>>> index f63af72..af6d57a 100644
>>> --- a/target/sparc/cpu-qom.h
>>> +++ b/target/sparc/cpu-qom.h
>>> @@ -35,6 +35,7 @@
>>> #define SPARC_CPU_GET_CLASS(obj) \
>>> OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
>>>
>>> +typedef struct sparc_def_t sparc_def_t;
>>> /**
>>> * SPARCCPUClass:
>>> * @parent_realize: The parent class' realize handler.
>>> @@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
>>>
>>> DeviceRealize parent_realize;
>>> void (*parent_reset)(CPUState *cpu);
>>> + sparc_def_t *cpu_def;
>>> } SPARCCPUClass;
>>>
>>> typedef struct SPARCCPU SPARCCPU;
>>> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
>>> index d606eb5..2994c09 100644
>>> --- a/target/sparc/cpu.c
>>> +++ b/target/sparc/cpu.c
>>> @@ -25,8 +25,6 @@
>>>
>>> //#define DEBUG_FEATURES
>>>
>>> -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char *cpu_model);
>>> -
>>> /* CPUClass::reset() */
>>> static void sparc_cpu_reset(CPUState *s)
>>> {
>>> @@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
>>> {
>>> CPUSPARCState *env = &cpu->env;
>>> char *s = g_strdup(cpu_model);
>>> - char *featurestr, *name = strtok(s, ",");
>>> - sparc_def_t def1, *def = &def1;
>>> + char *featurestr = strtok(s, ",");
>>> Error *err = NULL;
>>>
>>> - if (cpu_sparc_find_by_name(def, name) < 0) {
>>> - g_free(s);
>>> - return -1;
>>> - }
>>> -
>>> - env->def = g_memdup(def, sizeof(*def));
>>> -
>>> featurestr = strtok(NULL, ",");
>>> sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
>>> g_free(s);
>>> @@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
>>> return -1;
>>> }
>>>
>>> - env->version = def->iu_version;
>>> - env->fsr = def->fpu_version;
>>> - env->nwindows = def->nwindows;
>>> + env->version = env->def->iu_version;
>>> + env->fsr = env->def->fpu_version;
>>> + env->nwindows = env->def->nwindows;
>>> #if !defined(TARGET_SPARC64)
>>> - env->mmuregs[0] |= def->mmu_version;
>>> + env->mmuregs[0] |= env->def->mmu_version;
>>> cpu_sparc_set_id(env, 0);
>>> - env->mxccregs[7] |= def->mxcc_version;
>>> + env->mxccregs[7] |= env->def->mxcc_version;
>>> #else
>>> - env->mmu_version = def->mmu_version;
>>> - env->maxtl = def->maxtl;
>>> - env->version |= def->maxtl << 8;
>>> - env->version |= def->nwindows - 1;
>>> + env->mmu_version = env->def->mmu_version;
>>> + env->maxtl = env->def->maxtl;
>>> + env->version |= env->def->maxtl << 8;
>>> + env->version |= env->def->nwindows - 1;
>>> #endif
>>> return 0;
>>> }
>>> @@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu, const char *cpu_model)
>>> SPARCCPU *cpu_sparc_init(const char *cpu_model)
>>> {
>>> SPARCCPU *cpu;
>>> + ObjectClass *oc;
>>> + char *str, *name;
>>> +
>>> + str = g_strdup(cpu_model);
>>> + name = strtok(str, ",");
>>
>> you can free 'str' once here:
>>
>> g_free(str);
>>
>>> + oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
>>> + if (oc == NULL) {
>>> + g_free(str);
>>
>> drop
>>
>>> + return NULL;
>>> + }
>>> + g_free(str);
>>
>> drop
>
> I'll fix it up if I have to respin series (note this function is removed within series)
>
> [...]
>
>
>> Anyway I can't start Aurelien's image [1] with this commit:
> Can't reproduce it locally (either dirty/clean tree rebuild)
> with ./configure --enable-debug
>
>
>> $ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
>> qemu: Unable to find Sparc CPU definition
> for this kind of error guest image doesn't matter as error happens during machine_init()
>
>> [1] https://people.debian.org/~aurel32/qemu/sparc/
> It boots just fine with default CPU model:
>
> ./sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
Sorry I'v been distracted by something else :/
I might have done something wrong, I'll test it again tomorrow.
>
> and with explicitly specified one:
>
> sparc-softmmu/qemu-system-sparc -cpu "Fujitsu MB86904"
>
> I get from "info qom-tree"
> ...
> /device[0] (Fujitsu-MB86904-sparc-cpu)
> ...
>
On 08/22/2017 10:12 PM, Philippe Mathieu-Daudé wrote:
>>> On 08/18/2017 07:08 AM, Igor Mammedov wrote:
>>>> QOMfy cpu models handling introducing propper cpu types
>>>> for each cpu model.
>>>>
>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>> ---
>>>> with this and conversion of features to properties,
>>>> it would be possible to replace cpu_sparc_init() with
>>>> cpu_generic_init() and reuse common -cpu handling
>>>> infrastructure.
>>>>
>>>> CC: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> CC: Artyom Tarasenko <atar4qemu@gmail.com>
>>>> CC: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>
>>>> v2:
>>>> * make base class abstract (Philippe Mathieu-Daudé
>>>> <f4bug@amsat.org>)
>>>> ---
>>>> target/sparc/cpu-qom.h | 2 +
>>>> target/sparc/cpu.c | 121
>>>> +++++++++++++++++++++++++++++++++----------------
>>>> 2 files changed, 84 insertions(+), 39 deletions(-)
>>>>
>>>> diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
>>>> index f63af72..af6d57a 100644
>>>> --- a/target/sparc/cpu-qom.h
>>>> +++ b/target/sparc/cpu-qom.h
>>>> @@ -35,6 +35,7 @@
>>>> #define SPARC_CPU_GET_CLASS(obj) \
>>>> OBJECT_GET_CLASS(SPARCCPUClass, (obj), TYPE_SPARC_CPU)
>>>> +typedef struct sparc_def_t sparc_def_t;
>>>> /**
>>>> * SPARCCPUClass:
>>>> * @parent_realize: The parent class' realize handler.
>>>> @@ -49,6 +50,7 @@ typedef struct SPARCCPUClass {
>>>> DeviceRealize parent_realize;
>>>> void (*parent_reset)(CPUState *cpu);
>>>> + sparc_def_t *cpu_def;
>>>> } SPARCCPUClass;
>>>> typedef struct SPARCCPU SPARCCPU;
>>>> diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
>>>> index d606eb5..2994c09 100644
>>>> --- a/target/sparc/cpu.c
>>>> +++ b/target/sparc/cpu.c
>>>> @@ -25,8 +25,6 @@
>>>> //#define DEBUG_FEATURES
>>>> -static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, const char
>>>> *cpu_model);
>>>> -
>>>> /* CPUClass::reset() */
>>>> static void sparc_cpu_reset(CPUState *s)
>>>> {
>>>> @@ -111,17 +109,9 @@ static int cpu_sparc_register(SPARCCPU *cpu,
>>>> const char *cpu_model)
>>>> {
>>>> CPUSPARCState *env = &cpu->env;
>>>> char *s = g_strdup(cpu_model);
>>>> - char *featurestr, *name = strtok(s, ",");
>>>> - sparc_def_t def1, *def = &def1;
>>>> + char *featurestr = strtok(s, ",");
>>>> Error *err = NULL;
>>>> - if (cpu_sparc_find_by_name(def, name) < 0) {
>>>> - g_free(s);
>>>> - return -1;
>>>> - }
>>>> -
>>>> - env->def = g_memdup(def, sizeof(*def));
>>>> -
>>>> featurestr = strtok(NULL, ",");
>>>> sparc_cpu_parse_features(CPU(cpu), featurestr, &err);
>>>> g_free(s);
>>>> @@ -130,18 +120,18 @@ static int cpu_sparc_register(SPARCCPU *cpu,
>>>> const char *cpu_model)
>>>> return -1;
>>>> }
>>>> - env->version = def->iu_version;
>>>> - env->fsr = def->fpu_version;
>>>> - env->nwindows = def->nwindows;
>>>> + env->version = env->def->iu_version;
>>>> + env->fsr = env->def->fpu_version;
>>>> + env->nwindows = env->def->nwindows;
>>>> #if !defined(TARGET_SPARC64)
>>>> - env->mmuregs[0] |= def->mmu_version;
>>>> + env->mmuregs[0] |= env->def->mmu_version;
>>>> cpu_sparc_set_id(env, 0);
>>>> - env->mxccregs[7] |= def->mxcc_version;
>>>> + env->mxccregs[7] |= env->def->mxcc_version;
>>>> #else
>>>> - env->mmu_version = def->mmu_version;
>>>> - env->maxtl = def->maxtl;
>>>> - env->version |= def->maxtl << 8;
>>>> - env->version |= def->nwindows - 1;
>>>> + env->mmu_version = env->def->mmu_version;
>>>> + env->maxtl = env->def->maxtl;
>>>> + env->version |= env->def->maxtl << 8;
>>>> + env->version |= env->def->nwindows - 1;
>>>> #endif
>>>> return 0;
>>>> }
>>>> @@ -149,8 +139,19 @@ static int cpu_sparc_register(SPARCCPU *cpu,
>>>> const char *cpu_model)
>>>> SPARCCPU *cpu_sparc_init(const char *cpu_model)
>>>> {
>>>> SPARCCPU *cpu;
>>>> + ObjectClass *oc;
>>>> + char *str, *name;
>>>> +
>>>> + str = g_strdup(cpu_model);
>>>> + name = strtok(str, ",");
>>>
>>> you can free 'str' once here:
>>>
>>> g_free(str);
>>>
>>>> + oc = cpu_class_by_name(TYPE_SPARC_CPU, name);
>>>> + if (oc == NULL) {
>>>> + g_free(str);
>>>
>>> drop
>>>
>>>> + return NULL;
>>>> + }
>>>> + g_free(str);
>>>
>>> drop
>>
>> I'll fix it up if I have to respin series (note this function is
>> removed within series)
Indeed I didn't notice
>>
>> [...]
>>
>>> Anyway I can't start Aurelien's image [1] with this commit:
>> Can't reproduce it locally (either dirty/clean tree rebuild)
>> with ./configure --enable-debug
>>
>>> $ sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
>>> qemu: Unable to find Sparc CPU definition
>> for this kind of error guest image doesn't matter as error happens
>> during machine_init()
>>
>>> [1] https://people.debian.org/~aurel32/qemu/sparc/
>> It boots just fine with default CPU model:
>>
>> ./sparc-softmmu/qemu-system-sparc -hda debian_etch_sparc_small.qcow2
Yes, my bad!
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Sorry I'v been distracted by something else :/
> I might have done something wrong, I'll test it again tomorrow.
>
>>
>> and with explicitly specified one:
>>
>> sparc-softmmu/qemu-system-sparc -cpu "Fujitsu MB86904"
>>
>> I get from "info qom-tree"
>> ...
>> /device[0] (Fujitsu-MB86904-sparc-cpu)
>> ...
>>
© 2016 - 2026 Red Hat, Inc.