diff mbox series

[3/5] target/riscv: Add H extention state description

Message ID 20200929020337.1559-4-jiangyifei@huawei.com
State Superseded
Headers show
Series Support RISC-V migration | expand

Commit Message

Jiangyifei Sept. 29, 2020, 2:03 a.m. UTC 
In the case of supporting H extention, add H extention 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 | 51 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Richard Henderson Oct. 1, 2020, 5:28 p.m. UTC | #1 
On 9/28/20 9:03 PM, Yifei Jiang wrote:
> +        VMSTATE_UINTTL(env.vsstatus, RISCVCPU),

> +        VMSTATE_UINTTL(env.vstvec, RISCVCPU),

> +        VMSTATE_UINTTL(env.vsscratch, RISCVCPU),

> +        VMSTATE_UINTTL(env.vsepc, RISCVCPU),

> +        VMSTATE_UINTTL(env.vscause, RISCVCPU),

> +        VMSTATE_UINTTL(env.vstval, RISCVCPU),

> +        VMSTATE_UINTTL(env.vsatp, RISCVCPU),


So... if I understand things correctly, this is synthetic state, internal to
QEMU.  It is generally better to only serialize architectural state, so that if
qemu internals are rearranged, it is easy to decide on the correct sequence of
operations.

It seems like this should be re-generated with a post_load hook, calling some
of the code currently in riscv_cpu_swap_hypervisor_regs().  Note that some
minor rearrangement would be needed to call that code from this new context.


r~
Alistair Francis Oct. 5, 2020, 10:09 p.m. UTC | #2 
On Thu, Oct 1, 2020 at 10:56 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> On 9/28/20 9:03 PM, Yifei Jiang wrote:

> > +        VMSTATE_UINTTL(env.vsstatus, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vstvec, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsscratch, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsepc, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vscause, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vstval, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsatp, RISCVCPU),

>

> So... if I understand things correctly, this is synthetic state, internal to

> QEMU.  It is generally better to only serialize architectural state, so that if

> qemu internals are rearranged, it is easy to decide on the correct sequence of

> operations.


I don't think the virtual registers are synthetic, they contain what
the guest Hypervisor/Hypervisor guest wrote to those CSRs. I don't
think we could re-generate them from anything else.

There are some other registers in this series that I think can be
re-generated. The PMP is a good example of that, where the PMP config
data could be re-generated from the CSRs.

Alistair

>

> It seems like this should be re-generated with a post_load hook, calling some

> of the code currently in riscv_cpu_swap_hypervisor_regs().  Note that some

> minor rearrangement would be needed to call that code from this new context.

>

>

> r~

>
Jiangyifei Oct. 9, 2020, 8:29 a.m. UTC | #3 
> -----Original Message-----

> From: Richard Henderson [mailto:richard.henderson@linaro.org]

> Sent: Friday, October 2, 2020 1:28 AM

> To: Jiangyifei <jiangyifei@huawei.com>; qemu-devel@nongnu.org;

> qemu-riscv@nongnu.org

> Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>;

> sagark@eecs.berkeley.edu; kbastian@mail.uni-paderborn.de; Zhangxiaofeng

> (F) <victor.zhangxiaofeng@huawei.com>; Alistair.Francis@wdc.com; yinyipeng

> <yinyipeng1@huawei.com>; palmer@dabbelt.com; Wubin (H)

> <wu.wubin@huawei.com>; dengkai (A) <dengkai1@huawei.com>

> Subject: Re: [PATCH 3/5] target/riscv: Add H extention state description

> 

> On 9/28/20 9:03 PM, Yifei Jiang wrote:

> > +        VMSTATE_UINTTL(env.vsstatus, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vstvec, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsscratch, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsepc, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vscause, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vstval, RISCVCPU),

> > +        VMSTATE_UINTTL(env.vsatp, RISCVCPU),

> 

> So... if I understand things correctly, this is synthetic state, internal to QEMU.

> It is generally better to only serialize architectural state, so that if qemu

> internals are rearranged, it is easy to decide on the correct sequence of

> operations.

> 

> It seems like this should be re-generated with a post_load hook, calling some of

> the code currently in riscv_cpu_swap_hypervisor_regs().  Note that some

> minor rearrangement would be needed to call that code from this new context.

> 

> 

> r~


Thank you for your comments.

As Alistair said, Those vs* are Virtual Supervisor CSRs. Hypervisor may write those.
Actually，except *_hs, all states are real CSRs and cannot be regenerated.
*_hs are backup of Supervisor CSRs when V=1，so, I don't think could re-generate them.

In conclusion, I think all H extension states in this patch need to be described in vmstate.

Yifei
diff mbox series

Patch

diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index b1fc839b43..6a528bc1a5 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -49,6 +49,56 @@  static const VMStateDescription vmstate_pmp = {
     }
 };
 
+static bool hyper_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return riscv_has_ext(env, RVH);
+}
+
+static const VMStateDescription vmstate_hyper = {
+    .name = "cpu/hyper",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = hyper_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(env.hstatus, RISCVCPU),
+        VMSTATE_UINTTL(env.hedeleg, RISCVCPU),
+        VMSTATE_UINTTL(env.hideleg, RISCVCPU),
+        VMSTATE_UINTTL(env.hcounteren, RISCVCPU),
+        VMSTATE_UINTTL(env.htval, RISCVCPU),
+        VMSTATE_UINTTL(env.htinst, RISCVCPU),
+        VMSTATE_UINTTL(env.hgatp, RISCVCPU),
+        VMSTATE_UINT64(env.htimedelta, RISCVCPU),
+
+        VMSTATE_UINTTL(env.vsstatus, RISCVCPU),
+        VMSTATE_UINTTL(env.vstvec, RISCVCPU),
+        VMSTATE_UINTTL(env.vsscratch, RISCVCPU),
+        VMSTATE_UINTTL(env.vsepc, RISCVCPU),
+        VMSTATE_UINTTL(env.vscause, RISCVCPU),
+        VMSTATE_UINTTL(env.vstval, RISCVCPU),
+        VMSTATE_UINTTL(env.vsatp, RISCVCPU),
+
+        VMSTATE_UINTTL(env.mtval2, RISCVCPU),
+        VMSTATE_UINTTL(env.mtinst, RISCVCPU),
+
+        VMSTATE_UINTTL(env.stvec_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.sscratch_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.sepc_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.scause_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.stval_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.satp_hs, RISCVCPU),
+        VMSTATE_UINTTL(env.mstatus_hs, RISCVCPU),
+
+#ifdef TARGET_RISCV32
+        VMSTATE_UINTTL(env.vsstatush, RISCVCPU),
+        VMSTATE_UINTTL(env.mstatush_hs, RISCVCPU),
+#endif
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 1,
@@ -103,6 +153,7 @@  const VMStateDescription vmstate_riscv_cpu = {
     },
     .subsections = (const VMStateDescription * []) {
         &vmstate_pmp,
+        &vmstate_hyper,
         NULL
     }
 };