OpenXiangShan / NEMU

Other
236 stars 90 forks source link

fix the CONFIG_BBL_OFFSET_WITH_CPT parameter in the xs-cpt configuration #453

Closed zybzzz closed 2 weeks ago

zybzzz commented 2 months ago

Changed the CONFIG_BBL_OFFSET_WITH_CPT parameter in the checkpoint configuration to fix a bug introduced by the incorrect setting of this parameter when a user tries to set up a checkpoint with NEMU according to the documentation, causing NEMU to fail to start the bbl, which in turn prevents normal sampling.


更改checkpoint配置中CONFIG_BBL_OFFSET_WITH_CPT参数,以修复其带来的bug。在用户按照文档尝试使用NEMU设置检查点时,这个参数设置的错误会导致NEMU无法启动bbl,进而导致无法进行正常的采样。

我在按照文档一步一步进行香山全系统仿真环境的搭建,但是按照文档的步骤来还是会导致采样的错误。后来通过研究bbl-out.txt发现,NEMU一直将bbl加载到0x800a0000的地址,而不是按照文档一步一步来所期望的0x80100000地址,通过检查发现,原来是 xs-cpt 的 config 没有随着文档同步更新导致了这个问题,这会 make riscv64-xs-cpt_defconfig直接产生错误的参数,让新手上手的时候难以修复。我在改动了这个参数之后就能够按照文档说明正确生成采样点并恢复采样点。

shinezyy commented 2 weeks ago

Thanks for contribution! Can you merge PR #569 into this PR to fix this parameter across all config files and add co-author for @ wangxingran222

zybzzz commented 2 weeks ago

Thanks for contribution! Can you merge PR #569 into this PR to fix this parameter across all config files and add co-author for @ wangxingran222

I'd be very happy to do that. However, due to personal matters, I will have to wait until after tomorrow evening to do the pr merge. If this waiting time permits, I will be able to merge #569 into this pr.

shinezyy commented 2 weeks ago

Thanks for contribution! Can you merge PR #569 into this PR to fix this parameter across all config files and add co-author for @ wangxingran222

I'd be very happy to do that. However, due to personal matters, I will have to wait until after tomorrow evening to do the pr merge. If this waiting time permits, I will be able to merge #569 into this pr.

That's ok!

zybzzz commented 2 weeks ago

Due to my limited understanding of Git, I think I made a slight mistake in my operations. I will update my Git actions later today.

zybzzz commented 2 weeks ago

The changes in PR #453 are outdated relative to the current version, so I directly used the commit from PR #569, only slightly modifying the commit message.

This commit should have completed the configuration modifications, and based on my understanding of the project, these changes to the configuration should not affect the behavior of NEMU, therefore I believe the modifications are complete.

@shinezyy

xyyy1420 commented 2 weeks ago

@zybzzz Squash and merge, plz

zybzzz commented 2 weeks ago

@zybzzz Squash and merge, plz

I apologize that out of my lack of understanding of the pull request process, I'm not getting what you're saying. In my eyes, now that there is a non-conflicting commit that can be merged directly, I'm not sure what to squash? Could you provide me with some information? @xyyy1420

xyyy1420 commented 2 weeks ago

@zybzzz Squash and merge, plz

I apologize that out of my lack of understanding of the pull request process, I'm not getting what you're saying. In my eyes, now that there is a non-conflicting commit that can be merged directly, I'm not sure what to squash? Could you provide me with some information? @xyyy1420

image just press this button

zybzzz commented 2 weeks ago

@zybzzz Squash and merge, plz

I apologize that out of my lack of understanding of the pull request process, I'm not getting what you're saying. In my eyes, now that there is a non-conflicting commit that can be merged directly, I'm not sure what to squash? Could you provide me with some information? @xyyy1420

image just press this button

image

I don't have write access to this repository, so this action should be done by a maintainer with write access.

xyyy1420 commented 2 weeks ago

@zybzzz Squash and merge, plz

I apologize that out of my lack of understanding of the pull request process, I'm not getting what you're saying. In my eyes, now that there is a non-conflicting commit that can be merged directly, I'm not sure what to squash? Could you provide me with some information? @xyyy1420

image just press this button

image

I don't have write access to this repository, so this action should be done by a maintainer with write access.

Oh! I'll merge the patch