Re: [Qemu-devel] [PATCH v19 03/21] target/rx: CPU definition

[Igor Mammedov]

On Tue, 11 Jun 2019 13:37:13 +0200
Philippe Mathieu-Daudé <address@hidden> wrote:

> From: Yoshinori Sato <address@hidden>
> 
> Signed-off-by: Yoshinori Sato <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> [PMD: Use newer QOM style, split cpu-qom.h, restrict access to
>  extable array, use rx_cpu_tlb_fill() extracted from patch of
>  Yoshinori Sato 'Convert to CPUClass::tlb_fill']
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  target/rx/cpu-qom.h |  42 ++++++++
>  target/rx/cpu.c     | 252 ++++++++++++++++++++++++++++++++++++++++++++
>  target/rx/cpu.h     | 201 +++++++++++++++++++++++++++++++++++
>  target/rx/gdbstub.c | 112 ++++++++++++++++++++
>  target/rx/monitor.c |  38 +++++++
>  5 files changed, 645 insertions(+)
>  create mode 100644 target/rx/cpu-qom.h
>  create mode 100644 target/rx/cpu.c
>  create mode 100644 target/rx/cpu.h
>  create mode 100644 target/rx/gdbstub.c
>  create mode 100644 target/rx/monitor.c
> 
> diff --git a/target/rx/cpu-qom.h b/target/rx/cpu-qom.h
> new file mode 100644
> index 0000000000..4ae3b38b3e
> --- /dev/null
> +++ b/target/rx/cpu-qom.h
> @@ -0,0 +1,42 @@
> +#ifndef QEMU_SUPERH_CPU_QOM_H
> +#define QEMU_SUPERH_CPU_QOM_H
> +
> +#include "qom/cpu.h"
> +/*
> + * RX CPU
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + * SPDX-License-Identifier: LGPL-2.0+
> + */
> +
> +#define TYPE_RX_CPU "rx-cpu"
> +
> +#define TYPE_RX62N_CPU RX_CPU_TYPE_NAME("rx62n")
> +
> +#define RXCPU_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(RXCPUClass, (klass), TYPE_RX_CPU)
> +#define RXCPU(obj) \
> +    OBJECT_CHECK(RXCPU, (obj), TYPE_RX_CPU)
> +#define RXCPU_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(RXCPUClass, (obj), TYPE_RX_CPU)
> +
> +/*
> + * RXCPUClass:
> + * @parent_realize: The parent class' realize handler.
> + * @parent_reset: The parent class' reset handler.
> + *
> + * A RX CPU model.
> + */
> +typedef struct RXCPUClass {
> +    /*< private >*/
> +    CPUClass parent_class;
> +    /*< public >*/
> +
> +    DeviceRealize parent_realize;
> +    void (*parent_reset)(CPUState *cpu);
> +
> +} RXCPUClass;
> +
> +#define CPUArchState struct CPURXState
> +
> +#endif
> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> new file mode 100644
> index 0000000000..4147c5c939
> --- /dev/null
> +++ b/target/rx/cpu.c
> @@ -0,0 +1,252 @@
> +/*
> + * QEMU RX CPU
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/qemu-print.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
> +#include "qemu-common.h"
> +#include "migration/vmstate.h"
> +#include "exec/exec-all.h"
> +#include "hw/loader.h"
> +#include "fpu/softfloat.h"
> +
> +static void rx_cpu_set_pc(CPUState *cs, vaddr value)
> +{
> +    RXCPU *cpu = RXCPU(cs);
> +
> +    cpu->env.pc = value;
> +}
> +
> +static void rx_cpu_synchronize_from_tb(CPUState *cs, TranslationBlock *tb)
> +{
> +    RXCPU *cpu = RXCPU(cs);
> +
> +    cpu->env.pc = tb->pc;
> +}
> +
> +static bool rx_cpu_has_work(CPUState *cs)
> +{
> +    return cs->interrupt_request &
> +        (CPU_INTERRUPT_HARD | CPU_INTERRUPT_FIR);
> +}
> +
> +static void rx_cpu_reset(CPUState *s)
> +{
> +    RXCPU *cpu = RXCPU(s);
> +    RXCPUClass *rcc = RXCPU_GET_CLASS(cpu);
> +    CPURXState *env = &cpu->env;
> +    uint32_t *resetvec;
> +
> +    rcc->parent_reset(s);
> +
> +    memset(env, 0, offsetof(CPURXState, end_reset_fields));
> +
> +    resetvec = rom_ptr(0xfffffffc, 4);
> +    if (resetvec) {
> +        /* In the case of kernel, it is ignored because it is not set. */
> +        env->pc = ldl_p(resetvec);
> +    }
> +    rx_cpu_unpack_psw(env, 0, 1);
> +    env->regs[0] = env->isp = env->usp = 0;
> +    env->fpsw = 0;
> +    set_flush_to_zero(1, &env->fp_status);
> +    set_flush_inputs_to_zero(1, &env->fp_status);
> +}
> +
> +static void rx_cpu_list_entry(gpointer data, gpointer user_data)
> +{
> +    const char *typename = object_class_get_name(OBJECT_CLASS(data));
> +    int len = strlen(typename) - strlen(RX_CPU_TYPE_SUFFIX);
you'd probably need to fix this up to account for comment below.

> +
> +    qemu_printf("%.*s\n", len, typename);
> +}
> +
> +void rx_cpu_list(void)
> +{
> +    GSList *list;
> +    list = object_class_get_list_sorted(TYPE_RX_CPU, false);
> +    g_slist_foreach(list, rx_cpu_list_entry, NULL);
> +    g_slist_free(list);
> +}
> +
> +static ObjectClass *rx_cpu_class_by_name(const char *cpu_model)
> +{
> +    ObjectClass *oc;
> +    char *typename;
> +
> +    oc = object_class_by_name(cpu_model);
> +    if (oc != NULL && object_class_dynamic_cast(oc, TYPE_RX_CPU) != NULL &&
> +        !object_class_is_abstract(oc)) {
> +        return oc;
> +    }
Leave only this part and drop the rest, so that it would be consistent
with other interfaces (hmp/qmp/-device) which use pure type names.
The less invariants we support the better.

It's true that pr-existing cpu types support various ways to name CPUs
but that's mostly due to us keeping compatibility for what used to work
and definitely not from desire to support that madness.

> +    typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);
> +    oc = object_class_by_name(typename);
> +    if (oc != NULL && object_class_is_abstract(oc)) {
> +        oc = NULL;
> +    }
> +    g_free(typename);
> +
> +    if (!oc) {
> +        /* default to rx62n */
> +        oc = object_class_by_name(TYPE_RX62N_CPU);
> +    }
pls don't do fallbacks on nonsense user input, it's much better to return NULL
here and make user fix error in his CLI.

> +
> +    return oc;
> +}

PS:
 it's fine to reply with v20 here is it doesn't break the rest of the series.

[...]

> +void rx_load_image(RXCPU *cpu, const char *filename,
> +                   uint32_t start, uint32_t size)
> +{
> +    static uint32_t extable[32];
> +    long kernel_size;
> +    int i;
> +
> +    kernel_size = load_image_targphys(filename, start, size);
> +    if (kernel_size < 0) {
> +        fprintf(stderr, "qemu: could not load kernel '%s'\n", filename);
> +        exit(1);
> +    }
> +    cpu->env.pc = start;
> +
> +    /* setup exception trap trampoline */
> +    /* linux kernel only works little-endian mode */
> +    for (i = 0; i < ARRAY_SIZE(extable); i++) {
> +        extable[i] = cpu_to_le32(0x10 + i * 4);
> +    }
> +    rom_add_blob_fixed("extable", extable, sizeof(extable), 0xffffff80);
> +}
well, it doesn't look like code that belongs to cpu.c,
typically it is done from board specific code.

> diff --git a/target/rx/cpu.h b/target/rx/cpu.h
> new file mode 100644
> index 0000000000..3e5f371f51
> --- /dev/null
> +++ b/target/rx/cpu.h
> @@ -0,0 +1,201 @@
> +/*
> + *  RX emulation definition
> + *
> + *  Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef RX_CPU_H
> +#define RX_CPU_H
> +
> +#include "qemu/bitops.h"
> +#include "qemu-common.h"
> +#include "hw/registerfields.h"
> +#include "cpu-qom.h"
> +#include "qom/cpu.h"
> +
> +#define TARGET_LONG_BITS 32
> +#define TARGET_PAGE_BITS 12
> +
> +#include "exec/cpu-defs.h"
> +
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
> +/* PSW define */
> +REG32(PSW, 0)
> +FIELD(PSW, C, 0, 1)
> +FIELD(PSW, Z, 1, 1)
> +FIELD(PSW, S, 2, 1)
> +FIELD(PSW, O, 3, 1)
> +FIELD(PSW, I, 16, 1)
> +FIELD(PSW, U, 17, 1)
> +FIELD(PSW, PM, 20, 1)
> +FIELD(PSW, IPL, 24, 4)
> +
> +/* FPSW define */
> +REG32(FPSW, 0)
> +FIELD(FPSW, RM, 0, 2)
> +FIELD(FPSW, CV, 2, 1)
> +FIELD(FPSW, CO, 3, 1)
> +FIELD(FPSW, CZ, 4, 1)
> +FIELD(FPSW, CU, 5, 1)
> +FIELD(FPSW, CX, 6, 1)
> +FIELD(FPSW, CE, 7, 1)
> +FIELD(FPSW, CAUSE, 2, 6)
> +FIELD(FPSW, DN, 8, 1)
> +FIELD(FPSW, EV, 10, 1)
> +FIELD(FPSW, EO, 11, 1)
> +FIELD(FPSW, EZ, 12, 1)
> +FIELD(FPSW, EU, 13, 1)
> +FIELD(FPSW, EX, 14, 1)
> +FIELD(FPSW, ENABLE, 10, 5)
> +FIELD(FPSW, FV, 26, 1)
> +FIELD(FPSW, FO, 27, 1)
> +FIELD(FPSW, FZ, 28, 1)
> +FIELD(FPSW, FU, 29, 1)
> +FIELD(FPSW, FX, 30, 1)
> +FIELD(FPSW, FLAGS, 26, 4)
> +FIELD(FPSW, FS, 31, 1)
> +
> +#define NB_MMU_MODES 1
> +#define MMU_MODE0_SUFFIX _all
> +
> +enum {
> +    NUM_REGS = 16,
> +};
> +
> +typedef struct CPURXState {
> +    /* CPU registers */
> +    uint32_t regs[NUM_REGS];    /* general registers */
> +    uint32_t psw_o;             /* O bit of status register */
> +    uint32_t psw_s;             /* S bit of status register */
> +    uint32_t psw_z;             /* Z bit of status register */
> +    uint32_t psw_c;             /* C bit of status register */
> +    uint32_t psw_u;
> +    uint32_t psw_i;
> +    uint32_t psw_pm;
> +    uint32_t psw_ipl;
> +    uint32_t bpsw;              /* backup status */
> +    uint32_t bpc;               /* backup pc */
> +    uint32_t isp;               /* global base register */
> +    uint32_t usp;               /* vector base register */
> +    uint32_t pc;                /* program counter */
> +    uint32_t intb;              /* interrupt vector */
> +    uint32_t fintv;
> +    uint32_t fpsw;
> +    uint64_t acc;
> +
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
> +    /* Internal use */
> +    uint32_t in_sleep;
> +    uint32_t req_irq;           /* Requested interrupt no (hard) */
> +    uint32_t req_ipl;           /* Requested interrupt level */
> +    uint32_t ack_irq;           /* execute irq */
> +    uint32_t ack_ipl;           /* execute ipl */
> +    float_status fp_status;
> +    qemu_irq ack;               /* Interrupt acknowledge */
> +
> +    CPU_COMMON
> +} CPURXState;
> +
> +/*
> + * RXCPU:
> + * @env: #CPURXState
> + *
> + * A RX CPU
> + */
> +struct RXCPU {
> +    /*< private >*/
> +    CPUState parent_obj;
> +    /*< public >*/
> +
> +    CPURXState env;
> +};
> +
> +typedef struct RXCPU RXCPU;
> +typedef RXCPU ArchCPU;
> +
> +static inline RXCPU *rx_env_get_cpu(CPURXState *env)
> +{
> +    return container_of(env, RXCPU, env);
> +}
> +
> +#define ENV_GET_CPU(e) CPU(rx_env_get_cpu(e))
> +
> +#define ENV_OFFSET offsetof(RXCPU, env)
> +
> +#define RX_CPU_TYPE_SUFFIX "-" TYPE_RX_CPU
> +#define RX_CPU_TYPE_NAME(model) model RX_CPU_TYPE_SUFFIX
> +#define CPU_RESOLVING_TYPE TYPE_RX_CPU
> +
> +extern const char rx_crname[][6];
> +
> +void rx_cpu_do_interrupt(CPUState *cpu);
> +bool rx_cpu_exec_interrupt(CPUState *cpu, int int_req);
> +void rx_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
> +int rx_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> +int rx_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> +hwaddr rx_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> +
> +void rx_translate_init(void);
> +int cpu_rx_signal_handler(int host_signum, void *pinfo,
> +                           void *puc);
> +
> +void rx_cpu_list(void);
> +void rx_load_image(RXCPU *cpu, const char *filename,
> +                   uint32_t start, uint32_t size);
> +void rx_cpu_unpack_psw(CPURXState *env, uint32_t psw, int rte);
> +
> +#define cpu_signal_handler cpu_rx_signal_handler
> +#define cpu_list rx_cpu_list
> +
> +#include "exec/cpu-all.h"
> +
> +#define CPU_INTERRUPT_SOFT CPU_INTERRUPT_TGT_INT_0
> +#define CPU_INTERRUPT_FIR  CPU_INTERRUPT_TGT_INT_1
> +
> +#define RX_CPU_IRQ 0
> +#define RX_CPU_FIR 1
> +
> +static inline void cpu_get_tb_cpu_state(CPURXState *env, target_ulong *pc,
> +                                        target_ulong *cs_base, uint32_t 
> *flags)
> +{
> +    *pc = env->pc;
> +    *cs_base = 0;
> +    *flags = FIELD_DP32(0, PSW, PM, env->psw_pm);
> +}
> +
> +static inline int cpu_mmu_index(CPURXState *env, bool ifetch)
> +{
> +    return 0;
> +}
> +
> +static inline uint32_t rx_cpu_pack_psw(CPURXState *env)
> +{
> +    uint32_t psw = 0;
> +    psw = FIELD_DP32(psw, PSW, IPL, env->psw_ipl);
> +    psw = FIELD_DP32(psw, PSW, PM,  env->psw_pm);
> +    psw = FIELD_DP32(psw, PSW, U,   env->psw_u);
> +    psw = FIELD_DP32(psw, PSW, I,   env->psw_i);
> +    psw = FIELD_DP32(psw, PSW, O,   env->psw_o >> 31);
> +    psw = FIELD_DP32(psw, PSW, S,   env->psw_s >> 31);
> +    psw = FIELD_DP32(psw, PSW, Z,   env->psw_z == 0);
> +    psw = FIELD_DP32(psw, PSW, C,   env->psw_c);
> +    return psw;
> +}
> +
> +#endif /* RX_CPU_H */
> diff --git a/target/rx/gdbstub.c b/target/rx/gdbstub.c
> new file mode 100644
> index 0000000000..d76ca52e82
> --- /dev/null
> +++ b/target/rx/gdbstub.c
> @@ -0,0 +1,112 @@
> +/*
> + * RX gdb server stub
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "exec/gdbstub.h"
> +
> +int rx_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +    RXCPU *cpu = RXCPU(cs);
> +    CPURXState *env = &cpu->env;
> +
> +    switch (n) {
> +    case 0 ... 15:
> +        return gdb_get_regl(mem_buf, env->regs[n]);
> +    case 16:
> +        return gdb_get_regl(mem_buf, (env->psw_u) ? env->regs[0] : env->usp);
> +    case 17:
> +        return gdb_get_regl(mem_buf, (!env->psw_u) ? env->regs[0] : 
> env->isp);
> +    case 18:
> +        return gdb_get_regl(mem_buf, rx_cpu_pack_psw(env));
> +    case 19:
> +        return gdb_get_regl(mem_buf, env->pc);
> +    case 20:
> +        return gdb_get_regl(mem_buf, env->intb);
> +    case 21:
> +        return gdb_get_regl(mem_buf, env->bpsw);
> +    case 22:
> +        return gdb_get_regl(mem_buf, env->bpc);
> +    case 23:
> +        return gdb_get_regl(mem_buf, env->fintv);
> +    case 24:
> +        return gdb_get_regl(mem_buf, env->fpsw);
> +    case 25:
> +        return 0;
> +    }
> +    return 0;
> +}
> +
> +int rx_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +    RXCPU *cpu = RXCPU(cs);
> +    CPURXState *env = &cpu->env;
> +    uint32_t psw;
> +    switch (n) {
> +    case 0 ... 15:
> +        env->regs[n] = ldl_p(mem_buf);
> +        if (n == 0) {
> +            if (env->psw_u) {
> +                env->usp = env->regs[0];
> +            } else {
> +                env->isp = env->regs[0];
> +            }
> +        }
> +        break;
> +    case 16:
> +        env->usp = ldl_p(mem_buf);
> +        if (env->psw_u) {
> +            env->regs[0] = ldl_p(mem_buf);
> +        }
> +        break;
> +    case 17:
> +        env->isp = ldl_p(mem_buf);
> +        if (!env->psw_u) {
> +            env->regs[0] = ldl_p(mem_buf);
> +        }
> +        break;
> +    case 18:
> +        psw = ldl_p(mem_buf);
> +        rx_cpu_unpack_psw(env, psw, 1);
> +        break;
> +    case 19:
> +        env->pc = ldl_p(mem_buf);
> +        break;
> +    case 20:
> +        env->intb = ldl_p(mem_buf);
> +        break;
> +    case 21:
> +        env->bpsw = ldl_p(mem_buf);
> +        break;
> +    case 22:
> +        env->bpc = ldl_p(mem_buf);
> +        break;
> +    case 23:
> +        env->fintv = ldl_p(mem_buf);
> +        break;
> +    case 24:
> +        env->fpsw = ldl_p(mem_buf);
> +        break;
> +    case 25:
> +        return 8;
> +    default:
> +        return 0;
> +    }
> +
> +    return 4;
> +}
> diff --git a/target/rx/monitor.c b/target/rx/monitor.c
> new file mode 100644
> index 0000000000..5d7a1e58b5
> --- /dev/null
> +++ b/target/rx/monitor.c
> @@ -0,0 +1,38 @@
> +/*
> + * QEMU monitor
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "monitor/monitor.h"
> +#include "monitor/hmp-target.h"
> +#include "hmp.h"
> +
> +void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> +{
> +    CPUArchState *env = mon_get_cpu_env();
> +
> +    if (!env) {
> +        monitor_printf(mon, "No CPU available\n");
> +        return;
> +    }
> +}