Re: [Qemu-devel] [PATCH v1 02/28] target/riscv: Add the virtulisation mode

From: Alistair Francis <alistair23@gmail.com>
To: Palmer Dabbelt <palmer@sifive.com>
Cc: "open list:RISC-V" <qemu-riscv@nongnu.org>,
	Anup Patel <Anup.Patel@wdc.com>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Atish Patra <Atish.Patra@wdc.com>
Subject: Re: [Qemu-devel] [PATCH v1 02/28] target/riscv: Add the virtulisation mode
Date: Mon, 16 Sep 2019 08:57:52 -0700	[thread overview]
Message-ID: <CAKmqyKP5Ld5TQH+J2bArXOEChcB+0RFTdqNS-Sg-_G-UuOz_uQ@mail.gmail.com> (raw)
In-Reply-To: <mhng-9017291b-1ed2-4cea-ab34-03825a8c8a8c@palmer-si-x1e>

On Tue, Sep 10, 2019 at 6:44 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 23 Aug 2019 16:37:54 PDT (-0700), Alistair Francis wrote:
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/cpu.h        |  4 ++++
> >  target/riscv/cpu_bits.h   |  6 ++++++
> >  target/riscv/cpu_helper.c | 23 +++++++++++++++++++++++
> >  3 files changed, 33 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 7f54fb8c87..0ef1ecb0e0 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -117,6 +117,8 @@ struct CPURISCVState {
> >
> >  #ifndef CONFIG_USER_ONLY
> >      target_ulong priv;
> > +    /* This contains QEMU specific information about the virt state. */
> > +    target_ulong virt;
> >      target_ulong resetvec;
> >
> >      target_ulong mhartid;
> > @@ -257,6 +259,8 @@ int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
> >  int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
> >  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
> >  bool riscv_cpu_fp_enabled(CPURISCVState *env);
> > +bool riscv_cpu_virt_enabled(CPURISCVState *env);
> > +void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
> >  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
> >  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> >  void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index e99834856c..1fbde516be 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -422,6 +422,12 @@
> >  #define PRV_H 2 /* Reserved */
> >  #define PRV_M 3
> >
> > +/* Virtulisation modes */
> > +#define VIRT_OFF            0
> > +#define VIRT_ON             1
> > +#define VIRT_MODE_SHIFT     0
> > +#define VIRT_MODE_MASK      (1 << VIRT_MODE_SHIFT)
> > +
> >  /* RV32 satp CSR field masks */
> >  #define SATP32_MODE         0x80000000
> >  #define SATP32_ASID         0x7fc00000
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 225e407cff..7b0bb14c01 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -81,6 +81,29 @@ bool riscv_cpu_fp_enabled(CPURISCVState *env)
> >      return false;
> >  }
> >
> > +bool riscv_cpu_virt_enabled(CPURISCVState *env)
> > +{
> > +    bool tmp;
> > +
> > +    if (!riscv_has_ext(env, RVH)) {
> > +        return false;
> > +    }
> > +
> > +    tmp = (env->virt & VIRT_MODE_MASK) >> VIRT_MODE_SHIFT;
> > +
> > +    return tmp == VIRT_ON;
> > +}
>
> extract64() is a bit cleaner.

I have already changed it to get_field() and set_field().

>
> > +
> > +void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
> > +{
> > +    if (!riscv_has_ext(env, RVH)) {
> > +        return;
> > +    }
> > +
> > +    env->virt &= ~VIRT_MODE_MASK;
> > +    env->virt |= enable << VIRT_MODE_SHIFT;
> > +}
> > +
> >  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
> >  {
> >      CPURISCVState *env = &cpu->env;
>
> Reviewed-by: Palmer Dabbelt <palmer@sifive.com>

This patch is a little out of date, so I'll not include your RB line in the v2.

Alistair