| Message ID | 20200929020337.1559-4-jiangyifei@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support RISC-V migration | expand |
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~
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~ >
> -----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 --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 } };