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

[Antony Pavlov]

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

> 
> 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