Re: [Qemu-devel] [PATCH v1 03/21] RISC-V CPU Core Definition

[Michael Clark]

On Fri, 5 Jan 2018 at 6:39 AM, Antony Pavlov <address@hidden>
wrote:

> On Thu, 4 Jan 2018 20:33:57 +1300
> Michael Clark <address@hidden> wrote:
>
> > On Thu, Jan 4, 2018 at 7:47 PM, Antony Pavlov <address@hidden>
> > wrote:
> >
> > > On Wed,  3 Jan 2018 13:44:07 +1300
> > > Michael Clark <address@hidden> wrote:
> > >
> > > > Add CPU state header, CPU definitions and initialization routines
> > > >
> > > > Signed-off-by: Michael Clark <address@hidden>
> > > > ---
> > > >  target/riscv/cpu.c      | 338
> +++++++++++++++++++++++++++++++++++++++
> > > >  target/riscv/cpu.h      | 363 ++++++++++++++++++++++++++++++
> > > ++++++++++++
> > > >  target/riscv/cpu_bits.h | 411 ++++++++++++++++++++++++++++++
> > > ++++++++++++++++++
> > > >  3 files changed, 1112 insertions(+)
> > > >  create mode 100644 target/riscv/cpu.c
> > > >  create mode 100644 target/riscv/cpu.h
> > > >  create mode 100644 target/riscv/cpu_bits.h
> > > >
> > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > >
> > > ...
> > >
> > > > +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);
> > > > +    env->priv = PRV_M;
> > > > +    env->mtvec = DEFAULT_MTVEC;
> > > > +#endif
> > > > +    env->pc = DEFAULT_RSTVEC;
> > >
> > > The RISC-V Privileged Architecture Manual v1.10 states that
> > >
> > >   The pc is set to an implementation-defined reset vector.
> > >
> > > But hard-coded DEFAULT_RSTVEC leaves no chance for changing reset
> vector.
> > >
> > > Can we add a mechanism for changing reset vector?
> > >
> >
> > That can be added very easily at some point when necessary.
> >
> > All 5 RISC-V machines in the QEMU port currently have their emulated Mask
> > ROMs at 0x1000 so its not necessary until we add a machine that needs a
> > different value. I certainly wouldn't reject a patch that adds that
> > functionality if we had a machine with a different reset vector, although
> > given we have 5 machines using the same vector, it may remain a sensible
> > default. I would think twice about adding a property that no machines
> sets,
> > or duplicate code and have all machines set their reset vector even when
> > they are all the same? Shall we add the functionality when we need it?
>
> Actually it is me who needs this functionality.
>
> At the moment my board code needs this dirty hack:
>
>
> https://github.com/miet-riscv-workgroup/riscv-qemu/commit/bfc8221d89b9bb828f3742f17eb89d8513a75aae#diff-429448b1b26e0bc4256cc290758c0ab5


Okay, we can add a property. It’s also possible to register a cpu_reset
callback and set the pc there, within the machine.

>
> > I'd categorise this as a feature request. #define DEFAULT_RSTVEC
> 0x00001000
> > is the "implementation-defined reset vector"
> >
> > Folk on the RISC-V mailing list are actually seeking guidance on the
> blanks
> > in the RISC-V specification so it may be that a de-facto standard emerges
> > for some of these "implementation defined" blanks, in which case it may
> > become part of a platform spec (vs the ISA spec).
> >
> > E.g. there is the "reset-hivecs" property in the ARM emulation code
> > > so SoC-specific code can change reset vector.
>
> --
> Best regards,
>   Antony Pavlov
>