On 01/02/2018 04:44 PM, Michael Clark wrote:
> +#ifdef CONFIG_USER_ONLY
> +static bool riscv_cpu_has_work(CPUState *cs)
> +{
> + return 0;
> +}
> +#else
> +static bool riscv_cpu_has_work(CPUState *cs)
> +{
> + return cs->interrupt_request & CPU_INTERRUPT_HARD;
> +}
> +#endif
There's no need to conditionalize this.
> +static void riscv_cpu_reset(CPUState *cs)
> +{
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> + CPURISCVState *env = &cpu->env;
> +
> + mcc->parent_reset(cs);
> +#ifndef CONFIG_USER_ONLY
> + tlb_flush(cs);
Flush is now generic. Remove it from here.
> +static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> +{
> + CPUState *cs = CPU(dev);
> + RISCVCPU *cpu = RISCV_CPU(dev);
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
> + CPURISCVState *env = &cpu->env;
> + Error *local_err = NULL;
> +
> + cpu_exec_realizefn(cs, &local_err);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + return;
> + }
> +
> + if (env->misa & RVM) {
> + set_feature(env, RISCV_FEATURE_RVM);
> + }
What's the point of replicating this information?
> +static void cpu_register(const RISCVCPUInfo *info)
> +{
> + TypeInfo type_info = {
> + .name = g_strdup(info->name),
> + .parent = TYPE_RISCV_CPU,
> + .instance_size = sizeof(RISCVCPU),
> + .instance_init = info->initfn,
> + };
> +
> + type_register(&type_info);
> + g_free((void *)type_info.name);
> +}
I think type_register does its own strdup; you don't need to do your own.
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> new file mode 100644
> index 0000000..0480127
> --- /dev/null
> +++ b/target/riscv/cpu.h
> @@ -0,0 +1,363 @@
> +#ifndef RISCV_CPU_H
Header comment and license?
> +#define TARGET_HAS_ICE 1
What's this for?
> +#define RV(x) (1L << (x - 'A'))
L is useless since the type of long is variable. Either U or ULL.
> +typedef struct CPURISCVState CPURISCVState;
> +
> +#include "pmp.h"
> +
> +typedef struct CPURISCVState {
Duplicate typedef.
> + target_ulong gpr[32];
> + uint64_t fpr[32]; /* assume both F and D extensions */
> + target_ulong pc;
> + target_ulong load_res;
> +
> + target_ulong frm;
> + target_ulong fstatus;
> + target_ulong fflags;
> +
> + target_ulong badaddr;
> +
> + uint32_t mucounteren;
> +
> + target_ulong user_ver;
> + target_ulong priv_ver;
> + target_ulong misa_mask;
> + target_ulong misa;
> +
> +#ifdef CONFIG_USER_ONLY
> + uint32_t amoinsn;
> + target_long amoaddr;
> + target_long amotest;
> +#else
> + target_ulong priv;
> +
> + target_ulong mhartid;
> + target_ulong mstatus;
> + target_ulong mip;
> + target_ulong mie;
> + target_ulong mideleg;
> +
> + target_ulong sptbr; /* until: priv-1.9.1 */
> + target_ulong satp; /* since: priv-1.10.0 */
> + target_ulong sbadaddr;
> + target_ulong mbadaddr;
> + target_ulong medeleg;
> +
> + target_ulong stvec;
> + target_ulong sepc;
> + target_ulong scause;
> +
> + target_ulong mtvec;
> + target_ulong mepc;
> + target_ulong mcause;
> + target_ulong mtval; /* since: priv-1.10.0 */
> +
> + uint32_t mscounteren;
> + target_ulong scounteren; /* since: priv-1.10.0 */
> + target_ulong mcounteren; /* since: priv-1.10.0 */
> +
> + target_ulong sscratch;
> + target_ulong mscratch;
> +
> + /* temporary htif regs */
> + uint64_t mfromhost;
> + uint64_t mtohost;
> + uint64_t timecmp;
> +
> + /* physical memory protection */
> + pmp_table_t pmp_state;
> +#endif
> +
> + float_status fp_status;
> +
> + /* Internal CPU feature flags. */
> + uint64_t features;
> +
> + /* QEMU */
> + CPU_COMMON
> +
> + /* Fields from here on are preserved across CPU reset. */
> + void *irq[8];
> + QEMUTimer *timer; /* Internal timer */
FWIW, other targets have moved this timer to RISCVCPU struct.
> +static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
> + target_ulong *cs_base, uint32_t
> *flags)
> +{
> + *pc = env->pc;
> + *cs_base = 0;
> + *flags = 0; /* necessary to avoid compiler warning */
Remove the comment -- the assignment is necessary full stop.
> +#define MSTATUS64_UXL 0x0000000300000000
> +#define MSTATUS64_SXL 0x0000000C00000000
64-bit constants must use ULL. Otherwise builds from a 32-bit host will fail.
There are lots more instances within this file.
r~
