RE: [PATCH 2/5] target/riscv: Add PMP state description

[Jiangyifei <jiangyifei@huawei.com>]

> -----Original Message-----
> From: Alistair Francis [mailto:alistair23@gmail.com]
> Sent: Tuesday, October 6, 2020 6:11 AM
> To: Jiangyifei <jiangyifei@huawei.com>
> Cc: qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>; open
> list:RISC-V <qemu-riscv@nongnu.org>; Zhanghailiang
> <zhang.zhanghailiang@huawei.com>; Sagar Karandikar
> <sagark@eecs.berkeley.edu>; Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de>; Zhangxiaofeng (F)
> <victor.zhangxiaofeng@huawei.com>; Alistair Francis
> <Alistair.Francis@wdc.com>; yinyipeng <yinyipeng1@huawei.com>; Palmer
> Dabbelt <palmer@dabbelt.com>; Wubin (H) <wu.wubin@huawei.com>;
> dengkai (A) <dengkai1@huawei.com>
> Subject: Re: [PATCH 2/5] target/riscv: Add PMP state description
> 
> On Mon, Sep 28, 2020 at 7:05 PM Yifei Jiang <jiangyifei@huawei.com> wrote:
> >
> > In the case of supporting PMP feature, add PMP state description to
> > vmstate_riscv_cpu.
> >
> > Signed-off-by: Yifei Jiang <jiangyifei@huawei.com>
> > Signed-off-by: Yipeng Yin <yinyipeng1@huawei.com>
> > ---
> >  target/riscv/machine.c | 49
> > ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c index
> > 3451b888b1..b1fc839b43 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -4,6 +4,51 @@
> >  #include "sysemu/kvm.h"
> >  #include "migration/cpu.h"
> >
> > +static bool pmp_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return riscv_feature(env, RISCV_FEATURE_PMP); }
> > +
> > +static const VMStateDescription vmstate_pmp_entry = {
> > +    .name = "cpu/pmp/entry",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(addr_reg, pmp_entry_t),
> > +        VMSTATE_UINT8(cfg_reg, pmp_entry_t),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static const VMStateDescription vmstate_pmp_addr = {
> > +    .name = "cpu/pmp/addr",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(sa, pmp_addr_t),
> > +        VMSTATE_UINTTL(ea, pmp_addr_t),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static const VMStateDescription vmstate_pmp = {
> > +    .name = "cpu/pmp",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = pmp_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_STRUCT_ARRAY(env.pmp_state.pmp, RISCVCPU,
> MAX_RISCV_PMPS,
> > +                             0, vmstate_pmp_entry, pmp_entry_t),
> > +        VMSTATE_STRUCT_ARRAY(env.pmp_state.addr, RISCVCPU,
> MAX_RISCV_PMPS,
> > +                             0, vmstate_pmp_addr, pmp_addr_t),
> > +        VMSTATE_UINT32(env.pmp_state.num_rules, RISCVCPU),
> 
> I would prefer to re-generate these from the PMP CSRs post load.
> 
> Alistair
> 

Thanks for your advice. Yes, you are right.
vmstate_pmp_entry cannot be regenerated, but vmstate_pmp_addr and num_rules could be regenerated.

I'll add post_load hook for regenerating vmstate_pmp_addr and num_rules in the next series.

Yifei

> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  const VMStateDescription vmstate_riscv_cpu = {
> >      .name = "cpu",
> >      .version_id = 1,
> > @@ -55,5 +100,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> >          VMSTATE_UINTTL(env.mstatush, RISCVCPU),  #endif
> >          VMSTATE_END_OF_LIST()
> > +    },
> > +    .subsections = (const VMStateDescription * []) {
> > +        &vmstate_pmp,
> > +        NULL
> >      }
> >  };
> > --
> > 2.19.1
> >
> >