Am 27.05.2012 07:32, schrieb Jia Liu: > add openrisc target stubs. > > Signed-off-by: Jia Liu <pro...@gmail.com>
Minor nitpick: I'd recommend to stick to the typographic conventions outlined here: https://live.gnome.org/Git/CommitMessages In particular please start the sentence with a capital A. GNOME recommend a lowercase topic (we usually use the file/directory mainly affected) and uppercase beginning of the actual description, e.g. target-or32: Add target stubs Add OpenRISC target stubs. Signed-off-by: ... Writing it that way is not mandatory but when you're reposting and fixing the English grammar you can just as well make it perfect. ;) As Stefan pointed out, www.opencores.org writes it as OpenRISC, not Openrisc. I saw no prominent notice whether OpenRISC may be a trademark but better to respect their naming, seeing all the misspellings of QEMU. [...] > diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c > new file mode 100644 > index 0000000..ef3ffb1 > --- /dev/null > +++ b/target-openrisc/cpu.c > @@ -0,0 +1,24 @@ > +/* > + * QEMU Openrisc CPU > + * > + * Copyright (c) 2012 Jia Liu <pro...@gmail.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#include "cpu.h" > +#include "qemu-common.h" > +#if !defined(CONFIG_USER_ONLY) > +#include "hw/loader.h" > +#endif Missing TypeInfo, missing class_init, missing initfn (where you might want to move the openrisc_translate_init() call btw, following Igor's example), missing reset function. This cannot all be deferred to a later patch. > diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h > new file mode 100644 > index 0000000..80018df > --- /dev/null > +++ b/target-openrisc/cpu.h > @@ -0,0 +1,184 @@ > +/* > + * Openrisc virtual CPU header. > + * > + * Copyright (c) 2011-2012 Jia Liu <pro...@gmail.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef CPU_OPENRISC_H > +#define CPU_OPENRISC_H > + > +#define TARGET_HAS_ICE 1 > + > +#define ELF_MACHINE EM_OPENRISC > + > +#define CPUArchState struct CPUOpenriscState > + > +#define TARGET_LONG_BITS 32 > + > +#include "config.h" > +#include "qemu-common.h" > +#include "cpu-defs.h" > +#include "softfloat.h" > +#include "qemu/cpu.h" > + > +struct CPUOpenriscState; > + > +#define NB_MMU_MODES 3 > +#define MMU_NOMMU_IDX 0 > +#define MMU_SUPERVISOR_IDX 1 > +#define MMU_USER_IDX 2 Maybe make these three an enum? > + > +#define TARGET_PAGE_BITS 13 > + > +#define TARGET_PHYS_ADDR_SPACE_BITS 32 > +#define TARGET_VIRT_ADDR_SPACE_BITS 32 > + > +/* Verison Register */ > +#define SPR_VR 0x12000001 > +#define SPR_CPUCFGR 0x12000001 > + > +/* Registers */ > +enum { > + R0 = 0, R1, R2, R3, R4, R5, R6, R7, R8, R9, R10, > + R11, R12, R13, R14, R15, R16, R17, R18, R19, R20, > + R21, R22, R23, R24, R25, R26, R27, R28, R29, R30, > + R31 > +}; > + > +/* Register aliases */ > +enum { > + R_ZERO = R0, > + R_SP = R1, > + R_FP = R2, > + R_LR = R9, > + R_RV = R11, > + R_RVH = R12 > +}; > + > +typedef struct CPUOpenriscState CPUOpenriscState; > +struct CPUOpenriscState { > + target_ulong gpr[32]; /* General registers */ > + > + CPU_COMMON > + > + target_ulong pc; /* Program counter */ > + target_ulong npc; /* Next PC */ > + target_ulong ppc; /* Prev PC */ > + target_ulong jmp_pc; /* Jump PC */ > + uint32_t flags; > + /* Branch. */ > + uint32_t btaken; /* the SR_F bit */ > +}; Why are pc, etc. placed after CPU_COMMON? Are they not supposed to be reset? > + > +#define TYPE_OPENRISC_CPU "or32-cpu" > + > +#define OPENRISC_CPU_CLASS(klass) \ > + OBJECT_CLASS_CHECK(OpenriscCPUClass, (klass), TYPE_OPENRISC_CPU) > +#define OPENRISC_CPU(obj) \ > + OBJECT_CHECK(OpenriscCPU, (obj), TYPE_OPENRISC_CPU) > +#define OPENRISC_CPU_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(OpenriscCPUClass, (obj), TYPE_OPENRISC_CPU) > + > +/** > + * OpenriscCPUClass: > + * @parent_reset: The parent class' reset handler. > + * > + * A Openrisc CPU model. > + */ > +typedef struct OpenriscCPUClass { > + /*< private >*/ > + CPUClass parent_class; > + /*< public >*/ > + > + void (*parent_reset)(CPUState *cpu); > +} OpenriscCPUClass; > + > +/** > + * OpenriscCPU: > + * @env: #CPUOpenriscState > + * > + * A Openrisc CPU. > + */ > +typedef struct OpenriscCPU { > + /*< private >*/ > + CPUState parent_obj; > + /*< public >*/ > + > + CPUOpenriscState env; > +} OpenriscCPU; > + > +static inline OpenriscCPU *openrisc_env_get_cpu(CPUOpenriscState *env) > +{ > + return OPENRISC_CPU(container_of(env, OpenriscCPU, env)); > +} > + > +#define ENV_GET_CPU(e) CPU(openrisc_env_get_cpu(e)) > + > +void openrisc_cpu_realize(OpenriscCPU *cpu); This should have a second Error **errp parameter, even if currently unused. Please see target-i386 (target-arm is a bad example here). Implementation is missing and it's not being used either. > + > +void cpu_openrisc_list(FILE *f, fprintf_function cpu_fprintf); > +OpenriscCPU *cpu_openrisc_init(const char *cpu_model); > +int cpu_openrisc_exec(CPUOpenriscState *s); > +void do_interrupt(CPUOpenriscState *env); > +void openrisc_translate_init(void); > + > +#define cpu_list cpu_openrisc_list > +#define cpu_exec cpu_openrisc_exec > +#define cpu_gen_code cpu_openrisc_gen_code > + > +#define CPU_SAVE_VERSION 1 > + > +void openrisc_reset(CPUOpenriscState *env); Again, why the need? There's cpu_reset(). > +#if !defined(CONFIG_USER_ONLY) > +void openrisc_mmu_init(CPUOpenriscState *env); > +int get_phys_nommu(CPUOpenriscState *env, target_phys_addr_t *physical, > + int *prot, target_ulong address, int rw); > +#endif > + > +static inline CPUOpenriscState *cpu_init(const char *cpu_model) > +{ > + return NULL; > +} Needs to be implemented properly by calling cpu_openrisc_init(). > + > +#include "cpu-all.h" > + > +static inline void cpu_get_tb_cpu_state(CPUOpenriscState *env, > + target_ulong *pc, > + target_ulong *cs_base, int *flags) > +{ > + *pc = env->pc; > + *cs_base = 0; > + *flags = 0; > +} > + > +static inline int cpu_mmu_index(CPUOpenriscState *env) > +{ > + return 0; > +} > + > +static inline bool cpu_has_work(CPUOpenriscState *env) > +{ > + return 1; The return type is bool, so please use true rather than 1. > +} > + > +#include "exec-all.h" > + > +static inline void cpu_pc_from_tb(CPUOpenriscState *env, TranslationBlock > *tb) > +{ > + env->pc = tb->pc; > +} > + > +#endif /* CPU_OPENRISC_H */ > diff --git a/target-openrisc/helper.c b/target-openrisc/helper.c > new file mode 100644 > index 0000000..934f73b > --- /dev/null > +++ b/target-openrisc/helper.c > @@ -0,0 +1,60 @@ > +/* > + * Openrisc helpers > + * > + * Copyright (c) 2011-2012 Jia Liu <pro...@gmail.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#include "cpu.h" > +#include "qemu-common.h" > +#include "gdbstub.h" > +#include "host-utils.h" > +#include "sysemu.h" > + > +void cpu_state_reset(CPUOpenriscState *env) > +{ > + cpu_reset(ENV_GET_CPU(env)); > +} Had you rebased onto qom-next branch as requested, this would no longer be necessary. > + > +OpenriscCPU *cpu_openrisc_init(const char *cpu_model) > +{ > + OpenriscCPU *cpu; > + CPUOpenriscState *env; > + static int inited; > + inited = 0; > + > + if (!object_class_by_name(cpu_model)) { > + return NULL; > + } > + cpu = OPENRISC_CPU(object_new(cpu_model)); > + env = &cpu->env; > + env->cpu_model_str = cpu_model; > + /*realize openrisc cpu here*/ > + > + if (tcg_enabled() && !inited) { > + inited = 1; > + openrisc_translate_init(); > + } > + > + cpu_state_reset(env); cpu_reset(). > + > + return cpu; > +} This function would best be placed into cpu.c. > + > +void cpu_openrisc_list(FILE *f, fprintf_function cpu_fprintf) > +{ > + (*cpu_fprintf)(f, "Available CPUs:\n" > + "or1200\n"); Nack. Do not hardcode CPU models. Register a class "or1200" in cpu.c and call object_class_get_list() here, sort them and print the name of each. Again, compare target-arm. > +} This function should go into cpu.c, too. It's only in helper.c for many existing targets because cpu.c is pretty new. > diff --git a/target-openrisc/machine.c b/target-openrisc/machine.c > new file mode 100644 > index 0000000..31165fc > --- /dev/null > +++ b/target-openrisc/machine.c > @@ -0,0 +1,31 @@ > +/* > + * Openrisc Machine > + * > + * Copyright (c) 2011-2012 Jia Liu <pro...@gmail.com> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see > <http://www.gnu.org/licenses/>. > + */ > + > +#include "hw/hw.h" > +#include "hw/boards.h" > +#include "kvm.h" I doubt that there is KVM support for or32. > + > +void cpu_save(QEMUFile *f, void *opaque) > +{ > +} > + > +int cpu_load(QEMUFile *f, void *opaque, int version_id) > +{ > + return 0; > +} [snip] Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg