| Message ID | 20201010080623.768-2-jiangyifei@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | Support RISC-V migration | expand |
On 10/10/20 3:06 AM, Yifei Jiang wrote: > +++ b/target/riscv/cpu.h > @@ -311,6 +311,10 @@ extern const char * const riscv_fpr_regnames[]; > extern const char * const riscv_excp_names[]; > extern const char * const riscv_intr_names[]; > > +#ifndef CONFIG_USER_ONLY > +extern const VMStateDescription vmstate_riscv_cpu; > +#endif > + This is not part of the public interface to RISCVCPU, so it should go in internals.h. Not that there aren't other things in cpu.h that don't belong. No target has been perfect in differentiating what's interface and what's implementation. > + > +#ifdef TARGET_RISCV32 > + VMSTATE_UINTTL(env.mstatush, RISCVCPU), > +#endif Would this be a good time to expand mstatus to uint64_t instead of target_ulong so that it can be saved as one unit and reduce some ifdefs in the code base? Similarly with some of the other status registers that are two halved for riscv32. r~
> -----Original Message----- > From: Richard Henderson [mailto:richard.henderson@linaro.org] > Sent: Saturday, October 10, 2020 9:23 PM > To: Jiangyifei <jiangyifei@huawei.com>; qemu-devel@nongnu.org; > qemu-riscv@nongnu.org > Cc: palmer@dabbelt.com; Alistair.Francis@wdc.com; > sagark@eecs.berkeley.edu; kbastian@mail.uni-paderborn.de; Zhangxiaofeng > (F) <victor.zhangxiaofeng@huawei.com>; Wubin (H) <wu.wubin@huawei.com>; > Zhanghailiang <zhang.zhanghailiang@huawei.com>; dengkai (A) > <dengkai1@huawei.com>; yinyipeng <yinyipeng1@huawei.com> > Subject: Re: [PATCH V2 1/5] target/riscv: Add basic vmstate description of CPU > > On 10/10/20 3:06 AM, Yifei Jiang wrote: > > +++ b/target/riscv/cpu.h > > @@ -311,6 +311,10 @@ extern const char * const riscv_fpr_regnames[]; > > extern const char * const riscv_excp_names[]; extern const char * > > const riscv_intr_names[]; > > > > +#ifndef CONFIG_USER_ONLY > > +extern const VMStateDescription vmstate_riscv_cpu; #endif > > + > > This is not part of the public interface to RISCVCPU, so it should go in > internals.h. > > Not that there aren't other things in cpu.h that don't belong. No target has > been perfect in differentiating what's interface and what's implementation. > Yes, I think it should go in internals.h, although most architectures declare it in cpu.h. I would move it to internals.h in the next series. > > + > > +#ifdef TARGET_RISCV32 > > + VMSTATE_UINTTL(env.mstatush, RISCVCPU), #endif > > Would this be a good time to expand mstatus to uint64_t instead of > target_ulong so that it can be saved as one unit and reduce some ifdefs in the > code base? > > Similarly with some of the other status registers that are two halved for > riscv32. I agree with you that it should be rearranged. But I hope this series will focus on achieving migration. Can I send another patch to rearrange it later? Yifei > > > r~
On 10/14/20 3:21 AM, Jiangyifei wrote: >> Would this be a good time to expand mstatus to uint64_t instead of >> target_ulong so that it can be saved as one unit and reduce some ifdefs in the >> code base? >> >> Similarly with some of the other status registers that are two halved for >> riscv32. > > I agree with you that it should be rearranged. > But I hope this series will focus on achieving migration. > Can I send another patch to rearrange it later? Well, that changes the bit layout for migration. While we could bump the version number, it seemed easier to change the representation first. r~
On Wed, Oct 14, 2020 at 8:45 AM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 10/14/20 3:21 AM, Jiangyifei wrote: > >> Would this be a good time to expand mstatus to uint64_t instead of > >> target_ulong so that it can be saved as one unit and reduce some ifdefs in the > >> code base? > >> > >> Similarly with some of the other status registers that are two halved for > >> riscv32. > > > > I agree with you that it should be rearranged. > > But I hope this series will focus on achieving migration. > > Can I send another patch to rearrange it later? > > Well, that changes the bit layout for migration. > While we could bump the version number, it seemed > easier to change the representation first. +1 it would be great to consolidate these. Alistair > > > r~ >
> -----Original Message----- > From: Alistair Francis [mailto:alistair23@gmail.com] > Sent: Thursday, October 15, 2020 3:12 AM > To: Richard Henderson <richard.henderson@linaro.org> > Cc: Jiangyifei <jiangyifei@huawei.com>; qemu-devel@nongnu.org; > qemu-riscv@nongnu.org; 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 V2 1/5] target/riscv: Add basic vmstate description of CPU > > On Wed, Oct 14, 2020 at 8:45 AM Richard Henderson > <richard.henderson@linaro.org> wrote: > > > > On 10/14/20 3:21 AM, Jiangyifei wrote: > > >> Would this be a good time to expand mstatus to uint64_t instead of > > >> target_ulong so that it can be saved as one unit and reduce some > > >> ifdefs in the code base? > > >> > > >> Similarly with some of the other status registers that are two > > >> halved for riscv32. > > > > > > I agree with you that it should be rearranged. > > > But I hope this series will focus on achieving migration. > > > Can I send another patch to rearrange it later? > > > > Well, that changes the bit layout for migration. > > While we could bump the version number, it seemed easier to change the > > representation first. > > +1 it would be great to consolidate these. > > Alistair > OK. I will change this in the next series. Yifei > > > > > > r~ > >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 0bbfd7f457..bf396e2916 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -496,13 +496,6 @@ static void riscv_cpu_init(Object *obj) cpu_set_cpustate_pointers(cpu); } -#ifndef CONFIG_USER_ONLY -static const VMStateDescription vmstate_riscv_cpu = { - .name = "cpu", - .unmigratable = 1, -}; -#endif - static Property riscv_cpu_properties[] = { DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true), DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index de275782e6..8440ea0793 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -311,6 +311,10 @@ extern const char * const riscv_fpr_regnames[]; extern const char * const riscv_excp_names[]; extern const char * const riscv_intr_names[]; +#ifndef CONFIG_USER_ONLY +extern const VMStateDescription vmstate_riscv_cpu; +#endif + const char *riscv_cpu_get_trap_name(target_ulong cause, bool async); void riscv_cpu_do_interrupt(CPUState *cpu); int riscv_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg); diff --git a/target/riscv/machine.c b/target/riscv/machine.c new file mode 100644 index 0000000000..af2828a80a --- /dev/null +++ b/target/riscv/machine.c @@ -0,0 +1,77 @@ +/* + * RISC-V VMState Description + * + * Copyright (c) 2020 Huawei Technologies Co., Ltd + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "qemu/osdep.h" +#include "cpu.h" +#include "qemu/error-report.h" +#include "sysemu/kvm.h" +#include "migration/cpu.h" + +const VMStateDescription vmstate_riscv_cpu = { + .name = "cpu", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32), + VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32), + VMSTATE_UINTTL(env.pc, RISCVCPU), + VMSTATE_UINTTL(env.load_res, RISCVCPU), + VMSTATE_UINTTL(env.load_val, RISCVCPU), + VMSTATE_UINTTL(env.frm, RISCVCPU), + VMSTATE_UINTTL(env.badaddr, RISCVCPU), + VMSTATE_UINTTL(env.guest_phys_fault_addr, RISCVCPU), + VMSTATE_UINTTL(env.priv_ver, RISCVCPU), + VMSTATE_UINTTL(env.vext_ver, RISCVCPU), + VMSTATE_UINTTL(env.misa, RISCVCPU), + VMSTATE_UINTTL(env.misa_mask, RISCVCPU), + VMSTATE_UINT32(env.features, RISCVCPU), + VMSTATE_UINTTL(env.priv, RISCVCPU), + VMSTATE_UINTTL(env.virt, RISCVCPU), + VMSTATE_UINTTL(env.resetvec, RISCVCPU), + VMSTATE_UINTTL(env.mhartid, RISCVCPU), + VMSTATE_UINTTL(env.mstatus, RISCVCPU), + VMSTATE_UINTTL(env.mip, RISCVCPU), + VMSTATE_UINT32(env.miclaim, RISCVCPU), + VMSTATE_UINTTL(env.mie, RISCVCPU), + VMSTATE_UINTTL(env.mideleg, RISCVCPU), + VMSTATE_UINTTL(env.sptbr, RISCVCPU), + VMSTATE_UINTTL(env.satp, RISCVCPU), + VMSTATE_UINTTL(env.sbadaddr, RISCVCPU), + VMSTATE_UINTTL(env.mbadaddr, RISCVCPU), + VMSTATE_UINTTL(env.medeleg, RISCVCPU), + VMSTATE_UINTTL(env.stvec, RISCVCPU), + VMSTATE_UINTTL(env.sepc, RISCVCPU), + VMSTATE_UINTTL(env.scause, RISCVCPU), + VMSTATE_UINTTL(env.mtvec, RISCVCPU), + VMSTATE_UINTTL(env.mepc, RISCVCPU), + VMSTATE_UINTTL(env.mcause, RISCVCPU), + VMSTATE_UINTTL(env.mtval, RISCVCPU), + VMSTATE_UINTTL(env.scounteren, RISCVCPU), + VMSTATE_UINTTL(env.mcounteren, RISCVCPU), + VMSTATE_UINTTL(env.sscratch, RISCVCPU), + VMSTATE_UINTTL(env.mscratch, RISCVCPU), + VMSTATE_UINT64(env.mfromhost, RISCVCPU), + VMSTATE_UINT64(env.mtohost, RISCVCPU), + VMSTATE_UINT64(env.timecmp, RISCVCPU), + +#ifdef TARGET_RISCV32 + VMSTATE_UINTTL(env.mstatush, RISCVCPU), +#endif + VMSTATE_END_OF_LIST() + } +}; diff --git a/target/riscv/meson.build b/target/riscv/meson.build index abd647fea1..14a5c62dac 100644 --- a/target/riscv/meson.build +++ b/target/riscv/meson.build @@ -27,7 +27,8 @@ riscv_ss.add(files( riscv_softmmu_ss = ss.source_set() riscv_softmmu_ss.add(files( 'pmp.c', - 'monitor.c' + 'monitor.c', + 'machine.c' )) target_arch += {'riscv': riscv_ss}