[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/17] Openrisc: add target stubs
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/17] Openrisc: add target stubs |
Date: |
Sun, 27 May 2012 14:44:20 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
Am 27.05.2012 07:32, schrieb Jia Liu:
> add openrisc target stubs.
>
> Signed-off-by: Jia Liu <address@hidden>
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 <address@hidden>
> + *
> + * 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 <address@hidden>
> + *
> + * 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 <address@hidden>
> + *
> + * 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 <address@hidden>
> + *
> + * 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
- [Qemu-devel] [PATCH v2 00/17] Qemu Openrisc support, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 01/17] Openrisc: add target stubs, Jia Liu, 2012/05/27
- Re: [Qemu-devel] [PATCH v2 01/17] Openrisc: add target stubs,
Andreas Färber <=
- [Qemu-devel] [PATCH v2 02/17] Openrisc: add cpu QOM implement, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 03/17] Openrisc: add basic machine, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 04/17] Openrisc: add MMU support, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 06/17] Openrisc: add exception support, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 07/17] Openrisc: add int instruction helpers, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 08/17] Openrisc: add float instruction helpers, Jia Liu, 2012/05/27
- [Qemu-devel] [PATCH v2 09/17] Openrisc: add instruction translation routines, Jia Liu, 2012/05/27