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

[Jiangyifei]

> -Original Message-
> From: Alistair Francis [mailto:alistai...@gmail.com]
> Sent: Tuesday, October 6, 2020 6:11 AM
> To: Jiangyifei 
> Cc: qemu-devel@nongnu.org Developers ; open
> list:RISC-V ; Zhanghailiang
> ; Sagar Karandikar
> ; Bastian Koppelmann
> ; Zhangxiaofeng (F)
> ; Alistair Francis
> ; yinyipeng ; Palmer
> Dabbelt ; Wubin (H) ;
> dengkai (A) 
> Subject: Re: [PATCH 2/5] target/riscv: Add PMP state description
> 
> On Mon, Sep 28, 2020 at 7:05 PM Yifei Jiang  wrote:
> >
> > In the case of supporting PMP feature, add PMP state description to
> > vmstate_riscv_cpu.
> >
> > Signed-off-by: Yifei Jiang 
> > Signed-off-by: Yipeng Yin 
> > ---
> >  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 = >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 * []) {
> > +_pmp,
> > +NULL
> >  }
> >  };
> > --
> > 2.19.1
> >
> >

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

[Alistair Francis]

On Mon, Sep 28, 2020 at 7:05 PM Yifei Jiang  wrote:
>
> In the case of supporting PMP feature, add PMP state description
> to vmstate_riscv_cpu.
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 
> ---
>  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 = >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

> +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 * []) {
> +_pmp,
> +NULL
>  }
>  };
> --
> 2.19.1
>
>

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

[Yifei Jiang]

In the case of supporting PMP feature, add PMP state description
to vmstate_riscv_cpu.

Signed-off-by: Yifei Jiang 
Signed-off-by: Yipeng Yin 
---
 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 = >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),
+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 * []) {
+_pmp,
+NULL
 }
 };
-- 
2.19.1