Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,10 @@ AGENTS.md

microbench/build/
microbench/output/
microbench/dramsim3*
microbench/dramsim3*

*.bin
*.db
*.log
*.gz
*.zstd
8 changes: 3 additions & 5 deletions configs/common/Options.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,14 @@ def addCommonOptions(parser, configure_xiangshan=False):
"that are present under any of the roots. If not given, dump all "
"stats. ")

parser.add_argument("--smt", action="store_true", default=False,
help=""" RISCV SMT support, which requires multitThread-supported gcpt restore and diff-ref-so""")

Comment on lines +347 to +349
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in help text.

There's a typo in the help text: "multitThread" should be "multiThread" or "multi-thread".

✏️ Proposed fix
     parser.add_argument("--smt", action="store_true", default=False,
-                        help=""" RISCV SMT support, which requires multitThread-supported gcpt restore and diff-ref-so""")
+                        help="""RISCV SMT support, which requires multi-thread-supported gcpt restore and diff-ref-so""")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
parser.add_argument("--smt", action="store_true", default=False,
help=""" RISCV SMT support, which requires multitThread-supported gcpt restore and diff-ref-so""")
parser.add_argument("--smt", action="store_true", default=False,
help="""RISCV SMT support, which requires multi-thread-supported gcpt restore and diff-ref-so""")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@configs/common/Options.py` around lines 347 - 349, The help text for the
"--smt" argument contains a typo "multitThread-supported"; update the help
string in the parser.add_argument call for "--smt" (the Option defined with
action="store_true") to use the correct phrasing such as
"multi-thread-supported" or "multiThread-supported" (e.g., replace
"multitThread-supported" with "multi-thread-supported") so the help message
reads correctly.

if configure_xiangshan:
return
# Following options are not available in XiangShan

parser.add_argument("--checker", action="store_true")
parser.add_argument("--smt", action="store_true", default=False,
help="""
Only used if multiple programs are specified. If true,
then the number of threads per cpu is same as the
number of programs.""")
parser.add_argument(
"--elastic-trace-en", action="store_true",
help="""Enable capture of data dependency and instruction
Expand Down
17 changes: 11 additions & 6 deletions configs/common/xiangshan.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def resolve_xiangshan_ref_so(args: argparse.Namespace):
if args.difftest_ref_so is not None:
ref_so = args.difftest_ref_so
print("Obtained ref_so from args.difftest_ref_so: ", ref_so)
elif args.num_cpus > 1 and "GCBV_MULTI_CORE_REF_SO" in os.environ:
elif (args.num_cpus > 1 or args.smt) and "GCBV_MULTI_CORE_REF_SO" in os.environ:
ref_so = os.environ["GCBV_MULTI_CORE_REF_SO"]
print("Obtained ref_so from GCBV_MULTI_CORE_REF_SO: ", ref_so)
elif "GCBV_REF_SO" in os.environ:
Expand Down Expand Up @@ -330,12 +330,12 @@ def config_xiangshan_inputs(args: argparse.Namespace, sys):
if args.raw_cpt:
# If using raw binary, no restorer is needed.
gcpt_restorer = None
elif args.num_cpus > 1:
elif args.num_cpus > 1 or args.smt:
if "GCB_MULTI_CORE_RESTORER" in os.environ:
gcpt_restorer = os.environ["GCB_MULTI_CORE_RESTORER"]
print("Obtained gcpt_restorer from GCB_MULTI_CORE_RESTORER: ", gcpt_restorer)
else:
fatal("Plz set $GCB_MULTI_CORE_RESTORER when model Xiangshan with multi-core")
fatal("Plz set $GCB_MULTI_CORE_RESTORER when model Xiangshan with multi-context difftest")
elif args.restore_rvv_cpt:
if "GCBV_RESTORER" in os.environ:
gcpt_restorer = os.environ["GCBV_RESTORER"]
Expand All @@ -359,8 +359,8 @@ def config_xiangshan_inputs(args: argparse.Namespace, sys):
print("Obtained gcpt_restorer from args.gcpt_restorer: ", args.gcpt_restorer)
gcpt_restorer = args.gcpt_restorer

if args.num_cpus > 1:
print("Simulating a multi-core system, demanding a larger GCPT restorer size (2M).")
if args.num_cpus > 1 or args.smt:
print("Simulating a multi-context system, demanding a larger GCPT restorer size (2M).")
sys.gcpt_restorer_size_limit = 2**20
elif args.restore_rvv_cpt:
print("Simulating single core with RVV, demanding GCPT restorer size of 0x1000.")
Expand Down Expand Up @@ -407,7 +407,7 @@ def config_difftest(cpu_list, args, sys):
if not args.enable_difftest:
return
else:
if len(cpu_list) > 1:
if len(cpu_list) > 1 or args.smt:
sys.enable_mem_dedup = True
for cpu in cpu_list:
cpu.enable_mem_dedup = True
Expand Down Expand Up @@ -443,7 +443,12 @@ def _finish_xiangshan_system(args, test_sys, TestCPUClass, ruby):
test_sys.cpu = [TestCPUClass(clk_domain=test_sys.cpu_clk_domain, cpu_id=i)
for i in range(np)]
# Configure MMU for trace-aware FS mode
if args.smt:
test_sys.multi_thread = True

for cpu in test_sys.cpu:
if args.smt:
cpu.numThreads = 2
cpu.mmu.pma_checker = PMAChecker(
uncacheable=[AddrRange(0, size=0x80000000)])
cpu.mmu.functional = args.functional_tlb
Expand Down
123 changes: 79 additions & 44 deletions src/cpu/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

#include "cpu/base.hh"

#include <algorithm>
#include <iostream>
#include <sstream>
#include <string>
Expand Down Expand Up @@ -208,40 +209,50 @@ BaseCPU::BaseCPU(const Params &p, bool is_checker)
"of threads (%i).\n", params().isa.size(), numThreads);
}

diffAllStates = std::make_shared<DiffAllStates>();
diffAllStates.resize(numThreads);
if (enableDifftest) {
assert(params().difftest_ref_so.length() > 2);
diffAllStates->diff.nemu_reg = &(diffAllStates->referenceRegFile);
diffAllStates->diff.nemu_this_pc = 0x80000000u;
diffAllStates->diff.cpu_id = params().cpu_id;
warn("cpu_id set to %d\n", params().cpu_id);

if (params().difftest_ref_so.find("spike") != std::string::npos) {
assert(!system->multiCore());
diffAllStates->proxy = new SpikeProxy(
params().cpu_id, params().difftest_ref_so.c_str(),
params().nemuSDimg.size() && params().nemuSDCptBin.size());
} else {
diffAllStates->proxy =
new NemuProxy(params().cpu_id, params().difftest_ref_so.c_str(),
params().nemuSDimg.size() && params().nemuSDCptBin.size(), system->enabledMemDedup(),
system->multiCore());
}
for (ThreadID tid = 0; tid < numThreads; ++tid) {
diffAllStates[tid] = std::make_shared<DiffAllStates>();
auto diff_state = diffAllStates[tid];
diff_state->diff.nemu_reg = &(diff_state->referenceRegFile);
diff_state->diff.nemu_this_pc = 0x80000000u;
diff_state->diff.cpu_id = difftestHartId(tid);
warn("difftest hart id set to %d for tid %d\n",
diff_state->diff.cpu_id, tid);

if (params().difftest_ref_so.find("spike") != std::string::npos) {
assert(!system->multiContextDifftest());
diff_state->proxy = new SpikeProxy(
params().cpu_id, params().difftest_ref_so.c_str(),
params().nemuSDimg.size() && params().nemuSDCptBin.size());
} else {
diff_state->proxy =
new NemuProxy(params().cpu_id, params().difftest_ref_so.c_str(),
Comment on lines +222 to +233
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Derive difftest identity from the resolved CPU id, not the raw param.

BaseCPU can auto-assign _cpuId when cpu_id == -1, but this new code still seeds diff.cpu_id and the proxy constructors from params().cpu_id. That leaves auto-assigned CPUs with stale ids, and params().cpu_id * numThreads + tid also stops being globally unique once CPUs have different SMT widths. Please key this off _cpuId and use a system-wide stride such as maxThreadsPerCPU.

Also applies to: 882-885

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cpu/base.cc` around lines 220 - 231, Use the resolved CPU id (_cpuId) and
a system-wide stride when deriving difftest identities instead of
params().cpu_id and numThreads: compute a hart id like base = _cpuId *
system->maxThreadsPerCPU() and set diff_state->diff.cpu_id = base + tid (or
update difftestHartId to use _cpuId and maxThreadsPerCPU). Also pass _cpuId (not
params().cpu_id) into the proxy constructors (SpikeProxy/NemuProxy). Apply the
same change to the other occurrence mentioned (lines ~882-885) so all difftest
IDs and proxy initializations use the resolved _cpuId and the
system->maxThreadsPerCPU() stride.

params().nemuSDimg.size() && params().nemuSDCptBin.size(),
system->enabledMemDedup(),
system->multiContextDifftest());
}

warn("Difftest is enabled with ref so: %s.\n", params().difftest_ref_so.c_str());
warn("Difftest is enabled with ref so: %s.\n",
params().difftest_ref_so.c_str());

diffAllStates->proxy->regcpy(&(diffAllStates->gem5RegFile), REF_TO_DUT);
diffAllStates->diff.dynamic_config.ignore_illegal_mem_access = false;
diffAllStates->diff.dynamic_config.debug_difftest = false;
diffAllStates->proxy->update_config(&diffAllStates->diff.dynamic_config);
if (params().nemuSDimg.size() && params().nemuSDCptBin.size()) {
diffAllStates->proxy->sdcard_init(params().nemuSDimg.c_str(),
params().nemuSDCptBin.c_str());
diff_state->proxy->regcpy(&(diff_state->gem5RegFile), REF_TO_DUT);
diff_state->diff.dynamic_config.ignore_illegal_mem_access = false;
diff_state->diff.dynamic_config.debug_difftest = false;
diff_state->proxy->update_config(&diff_state->diff.dynamic_config);
if (params().nemuSDimg.size() && params().nemuSDCptBin.size()) {
diff_state->proxy->sdcard_init(params().nemuSDimg.c_str(),
params().nemuSDCptBin.c_str());
}
diff_state->diff.will_handle_intr = false;
}
diffAllStates->diff.will_handle_intr = false;
} else {
warn("Difftest is disabled\n");
diffAllStates->hasCommit = true;
for (ThreadID tid = 0; tid < numThreads; ++tid) {
diffAllStates[tid] = std::make_shared<DiffAllStates>();
diffAllStates[tid]->hasCommit = true;
}
}

if (dumpCommitFlag) {
Expand Down Expand Up @@ -404,11 +415,14 @@ BaseCPU::startup()
if (powerState->get() == enums::PwrState::UNDEFINED)
powerState->set(enums::PwrState::ON);

if (system->multiCore()) {
if (system->multiContextDifftest()) {
goldenMemPtr = system->getGoldenMemPtr();
_goldenMemManager = system->getGoldenMemManager();

diffAllStates->proxy->initState(params().cpu_id, goldenMemPtr);
for (ThreadID tid = 0; tid < numThreads; ++tid) {
diffAllStates[tid]->proxy->initState(difftestHartId(tid),
goldenMemPtr);
}
} else {
goldenMemPtr = nullptr;
_goldenMemManager = nullptr;
Expand Down Expand Up @@ -702,7 +716,7 @@ BaseCPU::takeOverFrom(BaseCPU *oldCPU)
if (enable_diff) {
warn("Take over difftest state to new CPU\n");
enableDifftest = enable_diff;
takeOverDiffAllStates(diff_all);
takeOverDiffAllStates(std::move(diff_all));
}
}

Expand Down Expand Up @@ -865,6 +879,12 @@ BaseCPU::GlobalStats::GlobalStats(statistics::Group *parent)
hostOpRate = simOps / hostSeconds;
}

int
BaseCPU::difftestHartId(ThreadID tid) const
{
return params().cpu_id * numThreads + tid;
}

void
BaseCPU::csrDiffMessage(uint64_t gem5_val, uint64_t ref_val, int error_num, uint64_t &error_reg, InstSeqNum seq,
std::string error_csr_name, int &diff_at)
Expand All @@ -883,6 +903,8 @@ BaseCPU::csrDiffMessage(uint64_t gem5_val, uint64_t ref_val, int error_num, uint
std::pair<int, bool>
BaseCPU::diffWithNEMU(ThreadID tid, InstSeqNum seq)
{
auto diffAllStates = this->diffAllStates[tid];

int diff_at = DiffAt::NoneDiff;
bool npc_match = false;
bool is_mmio = diffInfo.curInstStrictOrdered;
Expand Down Expand Up @@ -966,7 +988,7 @@ BaseCPU::diffWithNEMU(ThreadID tid, InstSeqNum seq)

if (enableRVV) {
if (diffInfo.inst->isVector()) {
readGem5Regs();
readGem5Regs(tid);
uint64_t* nemu_val = (uint64_t*)&(diffAllStates->referenceRegFile.vr[0]);
uint64_t* gem5_val = (uint64_t*)&(diffAllStates->gem5RegFile.vr[0]);
bool maybe_error = false;
Expand Down Expand Up @@ -1431,7 +1453,8 @@ BaseCPU::diffWithNEMU(ThreadID tid, InstSeqNum seq)
diffInfo.physEffAddr, diffInfo.effSize);
}

if (system->multiCore() && (diffInfo.inst->isLoad() || diffInfo.inst->isAtomic()) &&
if (system->multiContextDifftest() &&
(diffInfo.inst->isLoad() || diffInfo.inst->isAtomic()) &&
_goldenMemManager->inPmem(diffInfo.physEffAddr)) {
warn("Difference on %s instr found in multicore mode, check in golden memory\n",
diffInfo.inst->isLoad() ? "load" : "amo");
Expand Down Expand Up @@ -1517,9 +1540,10 @@ BaseCPU::clearDiffMismatch(ThreadID tid, InstSeqNum seq) {
void
BaseCPU::reportDiffMismatch(ThreadID tid, InstSeqNum seq)
{
auto diffAllStates = this->diffAllStates[tid];
warn("%s", diffMsg.str());
diffAllStates->proxy->isa_reg_display();
displayGem5Regs();
displayGem5Regs(tid);
warn("start dump last %lu committed msg\n", diffInfo.lastCommittedMsg.size());
while (diffInfo.lastCommittedMsg.size()) {
auto &inst = diffInfo.lastCommittedMsg.front();
Expand All @@ -1531,6 +1555,8 @@ BaseCPU::reportDiffMismatch(ThreadID tid, InstSeqNum seq)
void
BaseCPU::difftestStep(ThreadID tid, InstSeqNum seq)
{
auto diffAllStates = this->diffAllStates[tid];

bool should_diff = false;
DPRINTF(DumpCommit, "[sn:%llu] %#lx, %s\n",
seq, diffInfo.pc->instAddr(), diffInfo.inst->disassemble(diffInfo.pc->instAddr()));
Expand All @@ -1550,10 +1576,10 @@ BaseCPU::difftestStep(ThreadID tid, InstSeqNum seq)
should_diff = true;
if (!diffAllStates->hasCommit && diffInfo.pc->instAddr() == 0x80000000u) {
diffAllStates->hasCommit = true;
readGem5Regs();
readGem5Regs(tid);
diffAllStates->gem5RegFile.pc = diffInfo.pc->instAddr();
if (noHypeMode) {
auto start = pmemStart + pmemSize * diffAllStates->diff.cpu_id;
auto start = pmemStart + pmemSize * difftestHartId(tid);
warn("Start memcpy to NEMU from %#lx, size=%lu\n", (uint64_t)start, pmemSize);
diffAllStates->proxy->memcpy(0x80000000u, start, pmemSize, DUT_TO_REF);
} else if (enableMemDedup) {
Expand Down Expand Up @@ -1603,9 +1629,10 @@ BaseCPU::difftestStep(ThreadID tid, InstSeqNum seq)
}

void
BaseCPU::displayGem5Regs()
BaseCPU::displayGem5Regs(ThreadID tid)
{
readGem5Regs();
auto diffAllStates = this->diffAllStates[tid];
readGem5Regs(tid);
std::string str;
//reg
for (size_t i = 0; i < 32; i++)
Expand Down Expand Up @@ -1712,28 +1739,34 @@ BaseCPU::displayGem5Regs()
}

void
BaseCPU::difftestRaiseIntr(uint64_t no)
BaseCPU::difftestRaiseIntr(uint64_t no, ThreadID tid)
{
auto diffAllStates = this->diffAllStates[tid];
diffAllStates->diff.will_handle_intr = true;
diffAllStates->proxy->raise_intr(no);
}

void
BaseCPU::clearGuideExecInfo()
{
diffAllStates->diff.guide.force_raise_exception = false;
diffAllStates->diff.guide.force_set_jump_target = false;
for (auto &diffAllStates : this->diffAllStates) {
diffAllStates->diff.guide.force_raise_exception = false;
diffAllStates->diff.guide.force_set_jump_target = false;
}
}

void
BaseCPU::enableDiffPrint()
{
diffAllStates->diff.dynamic_config.debug_difftest = true;
diffAllStates->proxy->update_config(&diffAllStates->diff.dynamic_config);
for (auto &diffAllStates : this->diffAllStates) {
diffAllStates->diff.dynamic_config.debug_difftest = true;
diffAllStates->proxy->update_config(&diffAllStates->diff.dynamic_config);
}
}

void BaseCPU::setSCSuccess(bool success, paddr_t addr)
void BaseCPU::setSCSuccess(bool success, paddr_t addr, ThreadID tid)
{
auto diffAllStates = this->diffAllStates[tid];
diffAllStates->diff.sync.lrscValid = success;
diffAllStates->diff.sync.lrscAddr = addr; // used for spike diff
}
Expand All @@ -1742,6 +1775,8 @@ void
BaseCPU::setExceptionGuideExecInfo(uint64_t exception_num, uint64_t mtval, uint64_t stval, bool force_set_jump_target,
uint64_t jump_target, ThreadID tid)
{
auto diffAllStates = this->diffAllStates[tid];

auto &gd = diffAllStates->diff.guide;
gd.force_raise_exception = true;
gd.exception_num = exception_num;
Expand Down Expand Up @@ -1769,7 +1804,7 @@ BaseCPU::setExceptionGuideExecInfo(uint64_t exception_num, uint64_t mtval, uint6
void
BaseCPU::checkL1DRefill(Addr paddr, const uint8_t* refill_data, size_t size) {
assert(size == 64);
if (system->multiCore()) {
if (system->multiContextDifftest()) {
uint8_t *golden_ptr = (uint8_t *)_goldenMemManager->guestToHost(paddr);
if (memcmp(golden_ptr, refill_data, size)) {
panic("Refill data diff with Golden addr %#lx with size %d\n", paddr, size);
Expand Down
Loading
Loading