From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56540) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vo1uP-00051e-0Y for qemu-devel@nongnu.org; Tue, 03 Dec 2013 21:09:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vo1uJ-0000M8-IO for qemu-devel@nongnu.org; Tue, 03 Dec 2013 21:09:48 -0500 Received: from mail-pb0-f43.google.com ([209.85.160.43]:44233) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vo1uJ-0000Hd-9j for qemu-devel@nongnu.org; Tue, 03 Dec 2013 21:09:43 -0500 Received: by mail-pb0-f43.google.com with SMTP id rq2so22356355pbb.16 for ; Tue, 03 Dec 2013 18:09:21 -0800 (PST) Message-ID: <529E8ECC.7000403@ozlabs.ru> Date: Wed, 04 Dec 2013 13:09:16 +1100 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1386042168-20077-1-git-send-email-aik@ozlabs.ru> <20131203120918.255b0d47@nial.usersys.redhat.com> In-Reply-To: <20131203120918.255b0d47@nial.usersys.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v4] cpu: add suboptions support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= On 12/03/2013 10:09 PM, Igor Mammedov wrote: > On Tue, 3 Dec 2013 14:42:48 +1100 > Alexey Kardashevskiy wrote: > >> This adds suboptions support for -cpu. This keeps @cpu_model in order not >> to break the existing architectures/machines. >> >> Cc: Andreas Färber >> Signed-off-by: Alexey Kardashevskiy >> --- >> 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