> -----Original Message----- > From: Richard Henderson [mailto:richard.hender...@linaro.org] > Sent: Saturday, October 10, 2020 9:23 PM > To: Jiangyifei <jiangyi...@huawei.com>; qemu-devel@nongnu.org; > qemu-ri...@nongnu.org > Cc: pal...@dabbelt.com; alistair.fran...@wdc.com; > sag...@eecs.berkeley.edu; kbast...@mail.uni-paderborn.de; Zhangxiaofeng > (F) <victor.zhangxiaof...@huawei.com>; Wubin (H) <wu.wu...@huawei.com>; > Zhanghailiang <zhang.zhanghaili...@huawei.com>; dengkai (A) > <dengk...@huawei.com>; yinyipeng <yinyipe...@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~
