fix(sbpctl): modify sbpctl for v3 #957
Conversation
my-mayfly
requested review from
Anzooooo,
cebarobot and
good-circle
as code owners
cebarobot
left a comment
cebarobot
left a comment
There was a problem hiding this comment.
- If you want add a new config marco, you have to edit the
Kconfigfile. - Please follow the failed CI instruction to update all defconfig.
- Is this a good way to control custom CSRs? I think we need a better way to control those custom CSRs.
src/isa/riscv64/local-include/csr.h
Outdated
| uint64_t ras_enable : 1; // [5] | ||
| uint64_t loop_enable : 1; // [6] | ||
| CSR_STRUCT_END(sbpctl) | ||
| #endif |
There was a problem hiding this comment.
Please add comment for else and #endif.
There was a problem hiding this comment.
OK, i'll make the changes as you suggested.
src/isa/riscv64/local-include/csr.h
Outdated
| #define CUSTOM_CSR_SBPCTL_WMASK 0xff | ||
| #else | ||
| #define CUSTOM_CSR_SBPCTL_WMASK 0x7f | ||
| #endif |
There was a problem hiding this comment.
Please add comment for else and #endif.
There was a problem hiding this comment.
OK, i will make changes as you suggested.
There was a problem hiding this comment.
Not this comment. Please check other #ifdef and #endif pairs.
#ifdef CONFIG_RV_MBMC
something();
#else // CONFIG_RV_MBMC
otherthing();
#endif // CONFIG_RV_MBMC
| CONFIG_INSTR_CNT_BY_INSTR=y | ||
| CONFIG_ENABLE_INSTR_CNT=y | ||
| # end of Miscellaneous | ||
| CONFIG_KUNMINGHU_V3=y No newline at end of file |
There was a problem hiding this comment.
Only adding here without adding config in Kconfig makes no sense.
There was a problem hiding this comment.
Thank you very much -- i've noticed it
my-mayfly
force-pushed
the
fix-sbpctl-v3
branch
from
d236e6e to
e50a754
Compare
src/isa/riscv64/local-include/csr.h
Outdated
| #define CUSTOM_CSR_SBPCTL_WMASK 0xff | ||
| #else | ||
| #define CUSTOM_CSR_SBPCTL_WMASK 0x7f | ||
| #endif |
There was a problem hiding this comment.
Not this comment. Please check other #ifdef and #endif pairs.
#ifdef CONFIG_RV_MBMC
something();
#else // CONFIG_RV_MBMC
otherthing();
#endif // CONFIG_RV_MBMC
src/isa/riscv64/Kconfig
Outdated
| bool "Kunminghu Version 3" | ||
| default n | ||
| help | ||
| Enables Kunminghu V3-specific configurations, which differ from V2. |
There was a problem hiding this comment.
- Please place this configuration a bit later. It is not that important.
- Please use a more specific name and prompt. This config is about custom CSRs right? So what about
config CUSTOM_CSR_KMHV3?
There was a problem hiding this comment.
OK, i will make changes as you suggested.
src/isa/riscv64/local-include/csr.h
Outdated
| uint64_t utage_enable : 1; // [7] | ||
| CSR_STRUCT_END(sbpctl) | ||
| #else | ||
| CSR_STRUCT_START(sbpctl) |
There was a problem hiding this comment.
I think put ifdef inside CSR_STRUCT_START may be a better solution. Like
CSR_STRUCT_START(sbpctl)
#ifdef CONFIG_SOMETHING
something;
#else // CONFIG_SOMETHING
something;
#endif // CONFIG_SOMETHING
CSR_STRUCT_END(sbpctl)
There was a problem hiding this comment.
OK, i will make changes as you suggested.
my-mayfly
force-pushed
the
fix-sbpctl-v3
branch
2 times, most recently
from
d708f89 to
a2b4f9c
Compare
my-mayfly
force-pushed
the
fix-sbpctl-v3
branch
from
a2b4f9c to
03b75d3
Compare
The branch predictor configuration in V3 differs from that in V2, so the custom CSR register sbpctl is also different between the two versions. Due to the differing bit widths of sbpctl, reads may return inconsistent values, causing difftest mismatches and CI test case "misc" to fail.