[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub |
Date: |
Thu, 17 May 2012 16:14:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0 |
Am 17.05.2012 10:35, schrieb Jia Liu:
> add the openrisc target stub and basic implementation.
>
> Signed-off-by: Jia Liu <address@hidden>
> ---
> diff --git a/target-openrisc/cpu-qom.h b/target-openrisc/cpu-qom.h
> new file mode 100644
> index 0000000..8c936df
> --- /dev/null
> +++ b/target-openrisc/cpu-qom.h
First of all, if you design your new target cleanly, there's no strict
need for a cpu-qom.h header - it mainly served to separate new QOM code
from legacy code. If you want, you can put the code directly into cpu.h
just as well.
> @@ -0,0 +1,70 @@
> +/*
> + * QEMU Openrisc CPU
> + *
> + * Copyright (c) 2012 Jia Liu <address@hidden>
Minor nitpick: I guess this was copied from some other header? Uses a
two-space indentation here and one-space below.
> + *
> + * 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 QEMU_OPENRISC_CPU_QOM_H
> +#define QEMU_OPENRISC_CPU_QOM_H
> +
> +#include "qemu/cpu.h"
> +#include "cpu.h"
Please don't. This was a big mistake of mine that I've been correcting
on the qom-next branch, onto which you should rebase a new target such
as this by the way. It leads to really ugly problems when someone
includes cpu-qom.h and the new struct below is not yet defined for
functions using it there.
> +
> +#define TYPE_OPENRISC_CPU "or32"
> +
> +#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;
Pleave avoid unnecessary uppercase spelling: OpenRISCCPUClass? That
distinguishes it from the all-uppercase cast macros.
Or OpenriscCPUClass as you spell it elsewhere?
> +
> +/**
> + * OPENRISCCPU:
> + * @env: #CPUOPENRISCState
> + *
> + * A Openrisc CPU.
> + */
> +typedef struct OPENRISCCPU {
> + /*< private >*/
> + CPUState parent_obj;
> + /*< public >*/
> +
> + CPUOPENRISCState env;
> +} OPENRISCCPU;
OpenRISCCPU? 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))
> +
> +#endif /* QEMU_OPENRISC_CPU_QOM_H */
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> new file mode 100644
> index 0000000..01e65a2
> --- /dev/null
> +++ b/target-openrisc/cpu.c
> @@ -0,0 +1,74 @@
> +/*
> + * 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 "cpu-qom.h"
cpu.h already does #include "cpu-qom.h", so no need to include it here
again.
> +#include "qemu-common.h"
> +
> +
> +/* CPUClass::reset() */
> +static void openrisc_cpu_reset(CPUState *s)
> +{
> + OPENRISCCPU *cpu = OPENRISC_CPU(s);
> + OPENRISCCPUClass *occ = OPENRISC_CPU_GET_CLASS(cpu);
> + CPUOPENRISCState *env = &cpu->env;
> +
> + occ->parent_reset(s);
> +
> + openrisc_reset(env);
Shouldn't this be inline here? And don't forget to reset the common
fields, too. The usual way I think is to let a helper signal an
interrupt_request and then from outside the thread call cpu_reset(),
which will call the above function.
Be warned that over time more and more fields will be moved from
CPU_COMMON into CPUState, so env is not ideal long-term.
> +
> +}
> +
> +static void openrisc_cpu_initfn(Object *obj)
> +{
> + OPENRISCCPU *cpu = OPENRISC_CPU(obj);
> + CPUOPENRISCState *env = &cpu->env;
> +
> + cpu_exec_init(env);
> +
> + env->flags = 0;
> +
> + cpu_reset(CPU(cpu));
This should be part of a realizefn, not of the initfn. We don't have the
former just yet, so it should go into the cpu_xxx_init() function for now.
> +}
> +
> +static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
> +{
> + OPENRISCCPUClass *occ = OPENRISC_CPU_CLASS(oc);
> + CPUClass *cc = CPU_CLASS(oc);
> +
> + occ->parent_reset = cc->reset;
> + cc->reset = openrisc_cpu_reset;
> +}
> +
> +static const TypeInfo openrisc_cpu_type_info = {
> + .name = TYPE_OPENRISC_CPU,
> + .parent = TYPE_CPU,
> + .instance_size = sizeof(OPENRISCCPU),
> + .instance_init = openrisc_cpu_initfn,
> + .abstract = false,
> + .class_size = sizeof(OPENRISCCPUClass),
> + .class_init = openrisc_cpu_class_init,
> +};
> +
> +static void openrisc_cpu_register_types(void)
> +{
> + type_register_static(&openrisc_cpu_type_info);
> +}
> +
> +type_init(openrisc_cpu_register_types)
[...]
> diff --git a/target-openrisc/helper.c b/target-openrisc/helper.c
> new file mode 100644
> index 0000000..dcb61c9
> --- /dev/null
> +++ b/target-openrisc/helper.c
> @@ -0,0 +1,67 @@
> +/*
> + * 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 Library 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
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301
> USA
> + */
> +
> +#include "cpu.h"
> +#include "qemu-common.h"
> +#include "gdbstub.h"
> +#include "helper.h"
> +#include "host-utils.h"
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/loader.h"
> +#endif
> +
> +typedef struct {
> + const char *name;
> +} OPENRISCDef;
> +
> +static const OPENRISCDef openrisc_defs[] = {
> + {.name = "or1200",}
> +};
Don't. Use QOM subclasses for modelling CPU types.
> +
> +void cpu_openrisc_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> + int i;
> +
> + cpu_fprintf(f, "Available CPUs:\n");
> + for (i = 0; i < ARRAY_SIZE(openrisc_defs); ++i) {
> + cpu_fprintf(f, " %s\n", openrisc_defs[i].name);
> + }
> +}
This can then walk types to list models, cf. target-arm.
> +
> +CPUOPENRISCState *cpu_openrisc_init(const char *cpu_model)
This should return OpenRISCCPU instead.
cpu_init() can return CPUOpenRISCState for backwards compatibility,
there's lots of examples on the list.
> +{
> + CPUOPENRISCState *env;
> + static int tcg_inited;
> +
> + env = g_malloc0(sizeof(*env));
No. This needs to instantiate the QOM type with object_new().
> + memset(env, 0, sizeof(*env));
> + cpu_exec_init(env);
This is already in the initn, so drop it here.
> + qemu_init_vcpu(env);
This is the second candidate for a realizefn.
> + if (!tcg_inited) {
if (tcg_enabled() && !tcg_inited) {
Note that besides TCG there's qtest that your new target should support.
> + tcg_inited = 1;
> + openrisc_translate_init();
> + }
> +
> + return env;
> +}
> +
> +void do_interrupt(CPUOPENRISCState *env)
> +{
> +}
[...]
> diff --git a/target-openrisc/machine.c b/target-openrisc/machine.c
> new file mode 100644
> index 0000000..c7ac0ea
> --- /dev/null
> +++ b/target-openrisc/machine.c
> @@ -0,0 +1,76 @@
> +/*
> + * 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 Library 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
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301
> USA
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +#include "kvm.h"
> +
> +static const VMStateDescription vmstate_cpu = {
> + .name = "cpu",
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(gpr, CPUOPENRISCState, 32),
> + VMSTATE_UINT32(sr, CPUOPENRISCState),
> + VMSTATE_UINT32(epcr, CPUOPENRISCState),
> + VMSTATE_UINT32(eear, CPUOPENRISCState),
> + VMSTATE_UINT32(esr, CPUOPENRISCState),
> + VMSTATE_UINT32(fpcsr, CPUOPENRISCState),
> + VMSTATE_UINT32(pc, CPUOPENRISCState),
> + VMSTATE_UINT32(npc, CPUOPENRISCState),
> + VMSTATE_UINT32(ppc, CPUOPENRISCState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +void cpu_save(QEMUFile *f, void *opaque)
> +{
> + CPUOPENRISCState *env = (CPUOPENRISCState *)opaque;
> + unsigned int i;
> +
> + for (i = 0; i < 32; i++) {
> + qemu_put_betls(f, &env->gpr[i]);
> + }
> +
> + qemu_put_betls(f, &env->epcr);
> + qemu_put_betls(f, &env->eear);
> + qemu_put_betls(f, &env->esr);
> +
> + qemu_put_betls(f, &env->sr);
> + qemu_put_be32s(f, &env->pc);
> + qemu_put_be32s(f, &env->fpcsr);
> +}
> +
> +int cpu_load(QEMUFile *f, void *opaque, int version_id)
> +{
> + CPUOPENRISCState *env = (CPUOPENRISCState *)opaque;
> + unsigned int i;
> +
> + for (i = 0; i < 32; i++) {
> + qemu_get_betls(f, &env->gpr[i]);
> + }
> +
> + qemu_get_betls(f, &env->epcr);
> + qemu_get_betls(f, &env->eear);
> + qemu_get_betls(f, &env->esr);
> +
> + qemu_get_betls(f, &env->sr);
> + qemu_get_betls(f, &env->pc);
> + qemu_get_be32s(f, &env->fpcsr);
> + tlb_flush(env, 1);
> +
> + return 0;
> +}
[snip]
This is a mix of two ways of doing the same thing. You should only use
VMState for new code.
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 02/15] Openrisc: add MMU support, Jia Liu, 2012/05/17