* [Qemu-devel] [PATCH v4] cpu: add suboptions support
@ 2013-12-03 3:42 Alexey Kardashevskiy
2013-12-03 11:09 ` Igor Mammedov
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-03 3:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexey Kardashevskiy, Andreas Färber
This adds suboptions support for -cpu. This keeps @cpu_model in order not
to break the existing architectures/machines.
Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* moved QemuOpts logic to qeom/cpu.c
* added cpu_opt_get() as the machine init code wants to know the CPU name
to create a CPU object
---
include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
vl.c | 23 +++++++++++++++++++++++
3 files changed, 113 insertions(+)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..07330e1 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -124,6 +124,7 @@ typedef struct CPUClass {
int cpuid, void *opaque);
int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
void *opaque);
+ void (*parse_options)(CPUState *cpu, Error **errp);
const struct VMStateDescription *vmsd;
int gdb_num_core_regs;
@@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
#endif
/**
+ * cpu_parse_options:
+ * @cpu: The CPU to set options for.
+ *
+ * Parses CPU options if the CPU class has a custom handler defined.
+ *
+ * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
+ */
+static inline int cpu_parse_options(CPUState *cpu)
+{
+ CPUClass *cc = CPU_GET_CLASS(cpu);
+ Error *err = NULL;
+
+ if (cc->parse_options) {
+ cc->parse_options(cpu, &err);
+ if (err) {
+ return -1;
+ }
+ }
+
+ /* No callback, let arch do it the old way */
+ return 0;
+}
+
+/**
+ * cpu_default_parse_options_func:
+ * The default handler for CPUClass::parse_options
+ * @cpu: the CPU to set option for.
+ * @errp: the handling error descriptor.
+ */
+void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
+
+/**
+ * cpu_get_opt:
+ * @name: The parameter name
+ *
+ * Returns: a CPU parameter value.
+ */
+const char *cpu_opt_get(const char *name);
+
+/**
* cpu_reset:
* @cpu: The CPU whose state is to be reset.
*/
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..fb95cb4 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -24,6 +24,8 @@
#include "qemu/notify.h"
#include "qemu/log.h"
#include "sysemu/sysemu.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/config-file.h"
bool cpu_exists(int64_t id)
{
@@ -273,3 +275,50 @@ static void cpu_register_types(void)
}
type_init(cpu_register_types)
+
+static int cpu_set_property(const char *name, const char *value, void *opaque)
+{
+ Error *err = NULL;
+
+ if (strcmp(name, "type") == 0) {
+ return 0;
+ }
+
+ object_property_parse(opaque, value, name, &err);
+ if (err != NULL) {
+ qerror_report_err(err);
+ error_free(err);
+ return -1;
+ }
+
+ return 0;
+}
+
+static QemuOpts *cpu_get_opts(void)
+{
+ QemuOptsList *list;
+
+ list = qemu_find_opts("cpu");
+ assert(list);
+ return qemu_opts_find(list, NULL);
+}
+
+void cpu_default_parse_options_func(CPUState *cpu, Error **errp)
+{
+ QemuOpts *opts = cpu_get_opts();
+
+ if (opts && qemu_opt_foreach(opts, cpu_set_property, cpu, 1)) {
+ error_setg(errp, "Bad option");
+ }
+}
+
+const char *cpu_opt_get(const char *name)
+{
+ QemuOpts *opts = cpu_get_opts();
+
+ if (!opts) {
+ return NULL;
+ }
+
+ return qemu_opt_get(cpu_get_opts(), name);
+}
diff --git a/vl.c b/vl.c
index 8d5d874..f7c4c0a 100644
--- a/vl.c
+++ b/vl.c
@@ -433,6 +433,16 @@ static QemuOptsList qemu_machine_opts = {
},
};
+static QemuOptsList qemu_cpu_opts = {
+ .name = "cpu",
+ .implied_opt_name = "type",
+ .merge_lists = true,
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
+ .desc = {
+ { /* End of list */ }
+ },
+};
+
static QemuOptsList qemu_boot_opts = {
.name = "boot-opts",
.implied_opt_name = "order",
@@ -2880,6 +2890,7 @@ int main(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_trace_opts);
qemu_add_opts(&qemu_option_rom_opts);
qemu_add_opts(&qemu_machine_opts);
+ qemu_add_opts(&qemu_cpu_opts);
qemu_add_opts(&qemu_smp_opts);
qemu_add_opts(&qemu_boot_opts);
qemu_add_opts(&qemu_sandbox_opts);
@@ -2975,7 +2986,19 @@ int main(int argc, char **argv, char **envp)
}
case QEMU_OPTION_cpu:
/* hw initialization will check this */
+
+ /* Store cpu_model for old style parser */
cpu_model = optarg;
+
+ /*
+ * Parse and save options in the cpu options list
+ * for a new parser
+ */
+ olist = qemu_find_opts("cpu");
+ opts = qemu_opts_parse(olist, optarg, 1);
+ if (!opts) {
+ exit(1);
+ }
break;
case QEMU_OPTION_hda:
{
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2013-12-03 3:42 [Qemu-devel] [PATCH v4] cpu: add suboptions support Alexey Kardashevskiy
@ 2013-12-03 11:09 ` Igor Mammedov
2013-12-04 2:09 ` Alexey Kardashevskiy
0 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2013-12-03 11:09 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: qemu-devel, Andreas Färber
On Tue, 3 Dec 2013 14:42:48 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> This adds suboptions support for -cpu. This keeps @cpu_model in order not
> to break the existing architectures/machines.
>
> Cc: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v4:
> * moved QemuOpts logic to qeom/cpu.c
> * added cpu_opt_get() as the machine init code wants to know the CPU name
> to create a CPU object
> ---
> include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
> qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> vl.c | 23 +++++++++++++++++++++++
> 3 files changed, 113 insertions(+)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7739e00..07330e1 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -124,6 +124,7 @@ typedef struct CPUClass {
> int cpuid, void *opaque);
> int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
> void *opaque);
> + void (*parse_options)(CPUState *cpu, Error **errp);
>
> const struct VMStateDescription *vmsd;
> int gdb_num_core_regs;
> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
> #endif
>
> /**
> + * cpu_parse_options:
> + * @cpu: The CPU to set options for.
> + *
> + * Parses CPU options if the CPU class has a custom handler defined.
> + *
> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
> + */
> +static inline int cpu_parse_options(CPUState *cpu)
why not pass up the stack errp, vs returning -1?
And of cause, there should be a patch that demonstrates an actual user
of cpu_parse_options().
> +{
> + CPUClass *cc = CPU_GET_CLASS(cpu);
> + Error *err = NULL;
> +
> + if (cc->parse_options) {
> + cc->parse_options(cpu, &err);
> + if (err) {
> + return -1;
> + }
> + }
> +
> + /* No callback, let arch do it the old way */
> + return 0;
> +}
> +
> +/**
> + * cpu_default_parse_options_func:
> + * The default handler for CPUClass::parse_options
> + * @cpu: the CPU to set option for.
> + * @errp: the handling error descriptor.
> + */
> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
where is it used?
Maybe it should be static inside of qom/cpu.c
and assigned in cpu_class_init() for TYPE_CPU.
> +
> +/**
> + * cpu_get_opt:
> + * @name: The parameter name
> + *
> + * Returns: a CPU parameter value.
> + */
> +const char *cpu_opt_get(const char *name);
> +
> +/**
> * cpu_reset:
> * @cpu: The CPU whose state is to be reset.
> */
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 818fb26..fb95cb4 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -24,6 +24,8 @@
> #include "qemu/notify.h"
> #include "qemu/log.h"
> #include "sysemu/sysemu.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qemu/config-file.h"
>
> bool cpu_exists(int64_t id)
> {
> @@ -273,3 +275,50 @@ static void cpu_register_types(void)
> }
>
> type_init(cpu_register_types)
> +
> +static int cpu_set_property(const char *name, const char *value, void *opaque)
> +{
> + Error *err = NULL;
> +
> + if (strcmp(name, "type") == 0) {
> + return 0;
> + }
> +
> + object_property_parse(opaque, value, name, &err);
> + if (err != NULL) {
> + qerror_report_err(err);
> + error_free(err);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static QemuOpts *cpu_get_opts(void)
> +{
> + QemuOptsList *list;
> +
> + list = qemu_find_opts("cpu");
> + assert(list);
> + return qemu_opts_find(list, NULL);
> +}
> +
> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp)
> +{
> + QemuOpts *opts = cpu_get_opts();
> +
> + if (opts && qemu_opt_foreach(opts, cpu_set_property, cpu, 1)) {
> + error_setg(errp, "Bad option");
> + }
> +}
> +
> +const char *cpu_opt_get(const char *name)
> +{
> + QemuOpts *opts = cpu_get_opts();
> +
> + if (!opts) {
> + return NULL;
> + }
> +
> + return qemu_opt_get(cpu_get_opts(), name);
> +}
> diff --git a/vl.c b/vl.c
> index 8d5d874..f7c4c0a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -433,6 +433,16 @@ static QemuOptsList qemu_machine_opts = {
> },
> };
>
> +static QemuOptsList qemu_cpu_opts = {
> + .name = "cpu",
> + .implied_opt_name = "type",
> + .merge_lists = true,
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
> + .desc = {
> + { /* End of list */ }
> + },
> +};
> +
> static QemuOptsList qemu_boot_opts = {
> .name = "boot-opts",
> .implied_opt_name = "order",
> @@ -2880,6 +2890,7 @@ int main(int argc, char **argv, char **envp)
> qemu_add_opts(&qemu_trace_opts);
> qemu_add_opts(&qemu_option_rom_opts);
> qemu_add_opts(&qemu_machine_opts);
> + qemu_add_opts(&qemu_cpu_opts);
> qemu_add_opts(&qemu_smp_opts);
> qemu_add_opts(&qemu_boot_opts);
> qemu_add_opts(&qemu_sandbox_opts);
> @@ -2975,7 +2986,19 @@ int main(int argc, char **argv, char **envp)
> }
> case QEMU_OPTION_cpu:
> /* hw initialization will check this */
> +
> + /* Store cpu_model for old style parser */
> cpu_model = optarg;
> +
> + /*
> + * Parse and save options in the cpu options list
> + * for a new parser
> + */
> + olist = qemu_find_opts("cpu");
> + opts = qemu_opts_parse(olist, optarg, 1);
> + if (!opts) {
> + exit(1);
> + }
> break;
> case QEMU_OPTION_hda:
> {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2013-12-03 11:09 ` Igor Mammedov
@ 2013-12-04 2:09 ` Alexey Kardashevskiy
2013-12-19 23:36 ` Alexey Kardashevskiy
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-04 2:09 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, Andreas Färber
On 12/03/2013 10:09 PM, Igor Mammedov wrote:
> On Tue, 3 Dec 2013 14:42:48 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>> This adds suboptions support for -cpu. This keeps @cpu_model in order not
>> to break the existing architectures/machines.
>>
>> Cc: Andreas Färber <afaerber@suse.de>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * moved QemuOpts logic to qeom/cpu.c
>> * added cpu_opt_get() as the machine init code wants to know the CPU name
>> to create a CPU object
>> ---
>> include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
>> qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>> vl.c | 23 +++++++++++++++++++++++
>> 3 files changed, 113 insertions(+)
>>
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 7739e00..07330e1 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -124,6 +124,7 @@ typedef struct CPUClass {
>> int cpuid, void *opaque);
>> int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
>> void *opaque);
>> + void (*parse_options)(CPUState *cpu, Error **errp);
>>
>> const struct VMStateDescription *vmsd;
>> int gdb_num_core_regs;
>> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
>> #endif
>>
>> /**
>> + * cpu_parse_options:
>> + * @cpu: The CPU to set options for.
>> + *
>> + * Parses CPU options if the CPU class has a custom handler defined.
>> + *
>> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
>> + */
>> +static inline int cpu_parse_options(CPUState *cpu)
> why not pass up the stack errp, vs returning -1?
Oh. Overlooked it. Not sure if we really need Error** here, do we?
object_property_parse() will print an error anyway and QEMU then exits and
qemu_opt_foreach() cannot carry Error** to the caller so I'll better drop
it on this level.
> And of cause, there should be a patch that demonstrates an actual user
> of cpu_parse_options().
>
>> +{
>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>> + Error *err = NULL;
>> +
>> + if (cc->parse_options) {
>> + cc->parse_options(cpu, &err);
>> + if (err) {
>> + return -1;
>> + }
>> + }
>> +
>> + /* No callback, let arch do it the old way */
>> + return 0;
>> +}
>> +
>> +/**
>> + * cpu_default_parse_options_func:
>> + * The default handler for CPUClass::parse_options
>> + * @cpu: the CPU to set option for.
>> + * @errp: the handling error descriptor.
>> + */
>> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
> where is it used?
> Maybe it should be static inside of qom/cpu.c
> and assigned in cpu_class_init() for TYPE_CPU.
It is assigned for TYPE_POWERPC_CPU as I really do not want to switch every
single CPU in QEMU to use this so this is why it is not static.
Below is how I use it for POWERPC. Next time I'll post these patches
together, sorry for inconvenience.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d93abdc..7e0fb68 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -33,6 +33,7 @@
#include "sysemu/kvm.h"
#include "kvm_ppc.h"
#include "mmu-hash64.h"
+#include "qom/cpu.h"
#include "hw/boards.h"
#include "hw/ppc/ppc.h"
@@ -1111,7 +1112,7 @@ static SaveVMHandlers savevm_htab_handlers = {
static void ppc_spapr_init(QEMUMachineInitArgs *args)
{
ram_addr_t ram_size = args->ram_size;
- const char *cpu_model = args->cpu_model;
+ const char *cpu_model;
const char *kernel_filename = args->kernel_filename;
const char *kernel_cmdline = args->kernel_cmdline;
const char *initrd_filename = args->initrd_filename;
@@ -1131,6 +1132,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
msi_supported = true;
+ cpu_model = cpu_opt_get("type");
+
spapr = g_malloc0(sizeof(*spapr));
QLIST_INIT(&spapr->phbs);
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 1b0ff38..d24920b 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7381,6 +7381,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
/* PowerPC implementation specific initialisations (SPRs, timers, ...) */
(*pcc->init_proc)(env);
+ if (cpu_parse_options(CPU(cpu))) {
+ exit(1);
+ }
+
/* MSR bits & flags consistency checks */
if (env->msr_mask & (1 << 25)) {
switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
@@ -8676,6 +8680,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
*data)
#endif
dc->fw_name = "PowerPC,UNKNOWN";
+ cc->parse_options = cpu_default_parse_options_func;
}
static const TypeInfo ppc_cpu_type_info = {
--
Alexey
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2013-12-04 2:09 ` Alexey Kardashevskiy
@ 2013-12-19 23:36 ` Alexey Kardashevskiy
2014-01-22 0:59 ` Alexey Kardashevskiy
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-19 23:36 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, Andreas Färber
On 12/04/2013 01:09 PM, Alexey Kardashevskiy wrote:
> On 12/03/2013 10:09 PM, Igor Mammedov wrote:
>> On Tue, 3 Dec 2013 14:42:48 +1100
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>> This adds suboptions support for -cpu. This keeps @cpu_model in order not
>>> to break the existing architectures/machines.
>>>
>>> Cc: Andreas Färber <afaerber@suse.de>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Ping, anyone?
Patch is too bad? Any new idea how to rework it? Someone else is coming
with a new patch?
Thanks
>>> ---
>>> Changes:
>>> v4:
>>> * moved QemuOpts logic to qeom/cpu.c
>>> * added cpu_opt_get() as the machine init code wants to know the CPU name
>>> to create a CPU object
>>> ---
>>> include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>> qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>> vl.c | 23 +++++++++++++++++++++++
>>> 3 files changed, 113 insertions(+)
>>>
>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>> index 7739e00..07330e1 100644
>>> --- a/include/qom/cpu.h
>>> +++ b/include/qom/cpu.h
>>> @@ -124,6 +124,7 @@ typedef struct CPUClass {
>>> int cpuid, void *opaque);
>>> int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
>>> void *opaque);
>>> + void (*parse_options)(CPUState *cpu, Error **errp);
>>>
>>> const struct VMStateDescription *vmsd;
>>> int gdb_num_core_regs;
>>> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
>>> #endif
>>>
>>> /**
>>> + * cpu_parse_options:
>>> + * @cpu: The CPU to set options for.
>>> + *
>>> + * Parses CPU options if the CPU class has a custom handler defined.
>>> + *
>>> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
>>> + */
>>> +static inline int cpu_parse_options(CPUState *cpu)
>> why not pass up the stack errp, vs returning -1?
>
>
> Oh. Overlooked it. Not sure if we really need Error** here, do we?
>
> object_property_parse() will print an error anyway and QEMU then exits and
> qemu_opt_foreach() cannot carry Error** to the caller so I'll better drop
> it on this level.
>
>
>> And of cause, there should be a patch that demonstrates an actual user
>> of cpu_parse_options().
>>
>>> +{
>>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>>> + Error *err = NULL;
>>> +
>>> + if (cc->parse_options) {
>>> + cc->parse_options(cpu, &err);
>>> + if (err) {
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> + /* No callback, let arch do it the old way */
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * cpu_default_parse_options_func:
>>> + * The default handler for CPUClass::parse_options
>>> + * @cpu: the CPU to set option for.
>>> + * @errp: the handling error descriptor.
>>> + */
>>> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
>> where is it used?
>> Maybe it should be static inside of qom/cpu.c
>> and assigned in cpu_class_init() for TYPE_CPU.
>
>
> It is assigned for TYPE_POWERPC_CPU as I really do not want to switch every
> single CPU in QEMU to use this so this is why it is not static.
>
> Below is how I use it for POWERPC. Next time I'll post these patches
> together, sorry for inconvenience.
>
>
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d93abdc..7e0fb68 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -33,6 +33,7 @@
> #include "sysemu/kvm.h"
> #include "kvm_ppc.h"
> #include "mmu-hash64.h"
> +#include "qom/cpu.h"
>
> #include "hw/boards.h"
> #include "hw/ppc/ppc.h"
> @@ -1111,7 +1112,7 @@ static SaveVMHandlers savevm_htab_handlers = {
> static void ppc_spapr_init(QEMUMachineInitArgs *args)
> {
> ram_addr_t ram_size = args->ram_size;
> - const char *cpu_model = args->cpu_model;
> + const char *cpu_model;
> const char *kernel_filename = args->kernel_filename;
> const char *kernel_cmdline = args->kernel_cmdline;
> const char *initrd_filename = args->initrd_filename;
> @@ -1131,6 +1132,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>
> msi_supported = true;
>
> + cpu_model = cpu_opt_get("type");
> +
> spapr = g_malloc0(sizeof(*spapr));
> QLIST_INIT(&spapr->phbs);
>
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 1b0ff38..d24920b 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7381,6 +7381,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
> /* PowerPC implementation specific initialisations (SPRs, timers, ...) */
> (*pcc->init_proc)(env);
>
> + if (cpu_parse_options(CPU(cpu))) {
> + exit(1);
> + }
> +
> /* MSR bits & flags consistency checks */
> if (env->msr_mask & (1 << 25)) {
> switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
> @@ -8676,6 +8680,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
> *data)
> #endif
>
> dc->fw_name = "PowerPC,UNKNOWN";
> + cc->parse_options = cpu_default_parse_options_func;
> }
>
> static const TypeInfo ppc_cpu_type_info = {
>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2013-12-19 23:36 ` Alexey Kardashevskiy
@ 2014-01-22 0:59 ` Alexey Kardashevskiy
2014-02-20 13:24 ` Alexey Kardashevskiy
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2014-01-22 0:59 UTC (permalink / raw)
To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel
On 12/20/2013 10:36 AM, Alexey Kardashevskiy wrote:
> On 12/04/2013 01:09 PM, Alexey Kardashevskiy wrote:
>> On 12/03/2013 10:09 PM, Igor Mammedov wrote:
>>> On Tue, 3 Dec 2013 14:42:48 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> This adds suboptions support for -cpu. This keeps @cpu_model in order not
>>>> to break the existing architectures/machines.
>>>>
>>>> Cc: Andreas Färber <afaerber@suse.de>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
>
> Ping, anyone?
> Patch is too bad? Any new idea how to rework it? Someone else is coming
> with a new patch?
Andreas, is that what you asked me to ping you about? :)
I guess it is outdated and I have to rework it, I just did not have chance
yet...
>
> Thanks
>
>
>>>> ---
>>>> Changes:
>>>> v4:
>>>> * moved QemuOpts logic to qeom/cpu.c
>>>> * added cpu_opt_get() as the machine init code wants to know the CPU name
>>>> to create a CPU object
>>>> ---
>>>> include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>>> qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>> vl.c | 23 +++++++++++++++++++++++
>>>> 3 files changed, 113 insertions(+)
>>>>
>>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>>> index 7739e00..07330e1 100644
>>>> --- a/include/qom/cpu.h
>>>> +++ b/include/qom/cpu.h
>>>> @@ -124,6 +124,7 @@ typedef struct CPUClass {
>>>> int cpuid, void *opaque);
>>>> int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
>>>> void *opaque);
>>>> + void (*parse_options)(CPUState *cpu, Error **errp);
>>>>
>>>> const struct VMStateDescription *vmsd;
>>>> int gdb_num_core_regs;
>>>> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
>>>> #endif
>>>>
>>>> /**
>>>> + * cpu_parse_options:
>>>> + * @cpu: The CPU to set options for.
>>>> + *
>>>> + * Parses CPU options if the CPU class has a custom handler defined.
>>>> + *
>>>> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
>>>> + */
>>>> +static inline int cpu_parse_options(CPUState *cpu)
>>> why not pass up the stack errp, vs returning -1?
>>
>>
>> Oh. Overlooked it. Not sure if we really need Error** here, do we?
>>
>> object_property_parse() will print an error anyway and QEMU then exits and
>> qemu_opt_foreach() cannot carry Error** to the caller so I'll better drop
>> it on this level.
>>
>>
>>> And of cause, there should be a patch that demonstrates an actual user
>>> of cpu_parse_options().
>>>
>>>> +{
>>>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>>>> + Error *err = NULL;
>>>> +
>>>> + if (cc->parse_options) {
>>>> + cc->parse_options(cpu, &err);
>>>> + if (err) {
>>>> + return -1;
>>>> + }
>>>> + }
>>>> +
>>>> + /* No callback, let arch do it the old way */
>>>> + return 0;
>>>> +}
>>>> +
>>>> +/**
>>>> + * cpu_default_parse_options_func:
>>>> + * The default handler for CPUClass::parse_options
>>>> + * @cpu: the CPU to set option for.
>>>> + * @errp: the handling error descriptor.
>>>> + */
>>>> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
>>> where is it used?
>>> Maybe it should be static inside of qom/cpu.c
>>> and assigned in cpu_class_init() for TYPE_CPU.
>>
>>
>> It is assigned for TYPE_POWERPC_CPU as I really do not want to switch every
>> single CPU in QEMU to use this so this is why it is not static.
>>
>> Below is how I use it for POWERPC. Next time I'll post these patches
>> together, sorry for inconvenience.
>>
>>
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d93abdc..7e0fb68 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -33,6 +33,7 @@
>> #include "sysemu/kvm.h"
>> #include "kvm_ppc.h"
>> #include "mmu-hash64.h"
>> +#include "qom/cpu.h"
>>
>> #include "hw/boards.h"
>> #include "hw/ppc/ppc.h"
>> @@ -1111,7 +1112,7 @@ static SaveVMHandlers savevm_htab_handlers = {
>> static void ppc_spapr_init(QEMUMachineInitArgs *args)
>> {
>> ram_addr_t ram_size = args->ram_size;
>> - const char *cpu_model = args->cpu_model;
>> + const char *cpu_model;
>> const char *kernel_filename = args->kernel_filename;
>> const char *kernel_cmdline = args->kernel_cmdline;
>> const char *initrd_filename = args->initrd_filename;
>> @@ -1131,6 +1132,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>
>> msi_supported = true;
>>
>> + cpu_model = cpu_opt_get("type");
>> +
>> spapr = g_malloc0(sizeof(*spapr));
>> QLIST_INIT(&spapr->phbs);
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 1b0ff38..d24920b 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -7381,6 +7381,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
>> /* PowerPC implementation specific initialisations (SPRs, timers, ...) */
>> (*pcc->init_proc)(env);
>>
>> + if (cpu_parse_options(CPU(cpu))) {
>> + exit(1);
>> + }
>> +
>> /* MSR bits & flags consistency checks */
>> if (env->msr_mask & (1 << 25)) {
>> switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
>> @@ -8676,6 +8680,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
>> *data)
>> #endif
>>
>> dc->fw_name = "PowerPC,UNKNOWN";
>> + cc->parse_options = cpu_default_parse_options_func;
>> }
>>
>> static const TypeInfo ppc_cpu_type_info = {
>>
>>
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2014-01-22 0:59 ` Alexey Kardashevskiy
@ 2014-02-20 13:24 ` Alexey Kardashevskiy
2014-02-20 14:28 ` Andreas Färber
0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kardashevskiy @ 2014-02-20 13:24 UTC (permalink / raw)
To: Andreas Färber; +Cc: Igor Mammedov, qemu-devel
On 01/22/2014 11:59 AM, Alexey Kardashevskiy wrote:
> On 12/20/2013 10:36 AM, Alexey Kardashevskiy wrote:
>> On 12/04/2013 01:09 PM, Alexey Kardashevskiy wrote:
>>> On 12/03/2013 10:09 PM, Igor Mammedov wrote:
>>>> On Tue, 3 Dec 2013 14:42:48 +1100
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>
>>>>> This adds suboptions support for -cpu. This keeps @cpu_model in order not
>>>>> to break the existing architectures/machines.
>>>>>
>>>>> Cc: Andreas Färber <afaerber@suse.de>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>>
>> Ping, anyone?
>> Patch is too bad? Any new idea how to rework it? Someone else is coming
>> with a new patch?
>
>
> Andreas, is that what you asked me to ping you about? :)
>
> I guess it is outdated and I have to rework it, I just did not have chance
> yet...
Makes sense to repost it against current upstream or not yet?
>
>
>>
>> Thanks
>>
>>
>>>>> ---
>>>>> Changes:
>>>>> v4:
>>>>> * moved QemuOpts logic to qeom/cpu.c
>>>>> * added cpu_opt_get() as the machine init code wants to know the CPU name
>>>>> to create a CPU object
>>>>> ---
>>>>> include/qom/cpu.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>>>> qom/cpu.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> vl.c | 23 +++++++++++++++++++++++
>>>>> 3 files changed, 113 insertions(+)
>>>>>
>>>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>>>> index 7739e00..07330e1 100644
>>>>> --- a/include/qom/cpu.h
>>>>> +++ b/include/qom/cpu.h
>>>>> @@ -124,6 +124,7 @@ typedef struct CPUClass {
>>>>> int cpuid, void *opaque);
>>>>> int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
>>>>> void *opaque);
>>>>> + void (*parse_options)(CPUState *cpu, Error **errp);
>>>>>
>>>>> const struct VMStateDescription *vmsd;
>>>>> int gdb_num_core_regs;
>>>>> @@ -327,6 +328,46 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState *cpu, vaddr addr)
>>>>> #endif
>>>>>
>>>>> /**
>>>>> + * cpu_parse_options:
>>>>> + * @cpu: The CPU to set options for.
>>>>> + *
>>>>> + * Parses CPU options if the CPU class has a custom handler defined.
>>>>> + *
>>>>> + * Returns: -1 if a custom handler is defined and failed, 0 otherwise.
>>>>> + */
>>>>> +static inline int cpu_parse_options(CPUState *cpu)
>>>> why not pass up the stack errp, vs returning -1?
>>>
>>>
>>> Oh. Overlooked it. Not sure if we really need Error** here, do we?
>>>
>>> object_property_parse() will print an error anyway and QEMU then exits and
>>> qemu_opt_foreach() cannot carry Error** to the caller so I'll better drop
>>> it on this level.
>>>
>>>
>>>> And of cause, there should be a patch that demonstrates an actual user
>>>> of cpu_parse_options().
>>>>
>>>>> +{
>>>>> + CPUClass *cc = CPU_GET_CLASS(cpu);
>>>>> + Error *err = NULL;
>>>>> +
>>>>> + if (cc->parse_options) {
>>>>> + cc->parse_options(cpu, &err);
>>>>> + if (err) {
>>>>> + return -1;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* No callback, let arch do it the old way */
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * cpu_default_parse_options_func:
>>>>> + * The default handler for CPUClass::parse_options
>>>>> + * @cpu: the CPU to set option for.
>>>>> + * @errp: the handling error descriptor.
>>>>> + */
>>>>> +void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
>>>> where is it used?
>>>> Maybe it should be static inside of qom/cpu.c
>>>> and assigned in cpu_class_init() for TYPE_CPU.
>>>
>>>
>>> It is assigned for TYPE_POWERPC_CPU as I really do not want to switch every
>>> single CPU in QEMU to use this so this is why it is not static.
>>>
>>> Below is how I use it for POWERPC. Next time I'll post these patches
>>> together, sorry for inconvenience.
>>>
>>>
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index d93abdc..7e0fb68 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -33,6 +33,7 @@
>>> #include "sysemu/kvm.h"
>>> #include "kvm_ppc.h"
>>> #include "mmu-hash64.h"
>>> +#include "qom/cpu.h"
>>>
>>> #include "hw/boards.h"
>>> #include "hw/ppc/ppc.h"
>>> @@ -1111,7 +1112,7 @@ static SaveVMHandlers savevm_htab_handlers = {
>>> static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>> {
>>> ram_addr_t ram_size = args->ram_size;
>>> - const char *cpu_model = args->cpu_model;
>>> + const char *cpu_model;
>>> const char *kernel_filename = args->kernel_filename;
>>> const char *kernel_cmdline = args->kernel_cmdline;
>>> const char *initrd_filename = args->initrd_filename;
>>> @@ -1131,6 +1132,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>
>>> msi_supported = true;
>>>
>>> + cpu_model = cpu_opt_get("type");
>>> +
>>> spapr = g_malloc0(sizeof(*spapr));
>>> QLIST_INIT(&spapr->phbs);
>>>
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 1b0ff38..d24920b 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -7381,6 +7381,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
>>> /* PowerPC implementation specific initialisations (SPRs, timers, ...) */
>>> (*pcc->init_proc)(env);
>>>
>>> + if (cpu_parse_options(CPU(cpu))) {
>>> + exit(1);
>>> + }
>>> +
>>> /* MSR bits & flags consistency checks */
>>> if (env->msr_mask & (1 << 25)) {
>>> switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
>>> @@ -8676,6 +8680,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void
>>> *data)
>>> #endif
>>>
>>> dc->fw_name = "PowerPC,UNKNOWN";
>>> + cc->parse_options = cpu_default_parse_options_func;
>>> }
>>>
>>> static const TypeInfo ppc_cpu_type_info = {
>>>
>>>
>>>
>>
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support
2014-02-20 13:24 ` Alexey Kardashevskiy
@ 2014-02-20 14:28 ` Andreas Färber
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2014-02-20 14:28 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Igor Mammedov, qemu-devel
Am 20.02.2014 14:24, schrieb Alexey Kardashevskiy:
> On 01/22/2014 11:59 AM, Alexey Kardashevskiy wrote:
>> On 12/20/2013 10:36 AM, Alexey Kardashevskiy wrote:
>>> On 12/04/2013 01:09 PM, Alexey Kardashevskiy wrote:
>>>> On 12/03/2013 10:09 PM, Igor Mammedov wrote:
>>>>> On Tue, 3 Dec 2013 14:42:48 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>>
>>>>>> This adds suboptions support for -cpu. This keeps @cpu_model in order not
>>>>>> to break the existing architectures/machines.
>>>>>>
>>>>>> Cc: Andreas Färber <afaerber@suse.de>
>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>
>>>
>>> Ping, anyone?
>>> Patch is too bad? Any new idea how to rework it? Someone else is coming
>>> with a new patch?
>>
>>
>> Andreas, is that what you asked me to ping you about? :)
>>
>> I guess it is outdated and I have to rework it, I just did not have chance
>> yet...
>
> Makes sense to repost it against current upstream or not yet?
No, I just mentioned on IRC today that I have it on my TODO list after
X86CPU subclasses, to rework in a linux-user compatible way (which does
not use QemuOpts but an environment variable). Sorry for the delays.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-02-20 14:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 3:42 [Qemu-devel] [PATCH v4] cpu: add suboptions support Alexey Kardashevskiy
2013-12-03 11:09 ` Igor Mammedov
2013-12-04 2:09 ` Alexey Kardashevskiy
2013-12-19 23:36 ` Alexey Kardashevskiy
2014-01-22 0:59 ` Alexey Kardashevskiy
2014-02-20 13:24 ` Alexey Kardashevskiy
2014-02-20 14:28 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).