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 <a...@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 <afaer...@suse.de>
>>> Signed-off-by: Alexey Kardashevskiy <a...@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
