Liao Xiongfei has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/37315 )

Change subject: cpu-minor: this is a bug fix for MinorCPU for thread cloning.
......................................................................

cpu-minor: this is a bug fix for MinorCPU for thread cloning.

Inside the code of cloneFunc(…)  //syscall_emul.hh

    cp->initState();  //line 1483
    p->clone(tc, ctc, cp, flags);  //line 1484
    …
    ctc->clearArchRegs(); //line 1503

    OS::archClone(flags, p, cp, tc, ctc, newStack, tlsPtr); //line 1505
    …

At line 1483, initState() is called and the activateContext() of the corresponding MinorCPU is eventually called. The actual architecture clone happens at line 1505 where PC of the new thread could have a correct value.

In the existing implementation of MinorCPU::activateContext(ThreadID thread_id), the below line 275 is called
    pipeline->wakeupFetch(thread_id);
to start fetching instruction with current value of PC, which is 0x0,
leading to panic “Page table fault when accessing virtual address 0”.

This is because the OS::archClone() is not yet called. So, the below bug fix handles
the wakeup fetch for a thread for two scenarios:
    ...
    if ( curTick() == 0 ) { //the first thread
        pipeline->wakeupFetch(thread_id);
    } else {//the thread from clone
        if (fetchEventWrapper != NULL)
            delete fetchEventWrapper;
        fetchEventWrapper = new EventFunctionWrapper([this, thread_id]
          {pipeline->wakeupFetch(thread_id);}, "wakeupFetch");
        schedule(*fetchEventWrapper, clockEdge(Cycles(0)));
    }
    ...
For the first thread, the activateContext() is called at tick 0 while the other cloned
threads are not at tick 0.
For the cloned thread, the above bug fix delays the execution of pipeline->wakeupFetch()
after the OS::archClone is done.

A member variable fetchEventWrapper is added to MinorCPU class for delayed fetch event.

A few previously failed RISC-V ASM tests have been open after the bug fix works.

JIRA issue: https://gem5.atlassian.net/browse/GEM5-374

Change-Id: Ibffe46522e2617443d29f49df180692c54830f14
---
M src/cpu/minor/cpu.cc
M src/cpu/minor/cpu.hh
M tests/gem5/asmtest/tests.py
3 files changed, 29 insertions(+), 15 deletions(-)



diff --git a/src/cpu/minor/cpu.cc b/src/cpu/minor/cpu.cc
index 18a3fee..1d6b1fe 100644
--- a/src/cpu/minor/cpu.cc
+++ b/src/cpu/minor/cpu.cc
@@ -77,12 +77,17 @@

     pipeline = new Minor::Pipeline(*this, params);
     activityRecorder = pipeline->getActivityRecorder();
+
+    fetchEventWrapper = NULL;
 }

 MinorCPU::~MinorCPU()
 {
     delete pipeline;

+    if (fetchEventWrapper != NULL)
+        delete fetchEventWrapper;
+
     for (ThreadID thread_id = 0; thread_id < threads.size(); thread_id++) {
         delete threads[thread_id];
     }
@@ -266,7 +271,15 @@
     /* Wake up the thread, wakeup the pipeline tick */
     threads[thread_id]->activate();
     wakeupOnEvent(Minor::Pipeline::CPUStageId);
-    pipeline->wakeupFetch(thread_id);
+    if ( curTick() == 0 ) { //the first thread
+        pipeline->wakeupFetch(thread_id);
+    } else {//the thread from clone
+        if (fetchEventWrapper != NULL)
+            delete fetchEventWrapper;
+        fetchEventWrapper = new EventFunctionWrapper([this, thread_id]
+                  { pipeline->wakeupFetch(thread_id); }, "wakeupFetch");
+        schedule(*fetchEventWrapper, clockEdge(Cycles(0)));
+    }

     BaseCPU::activateContext(thread_id);
 }
diff --git a/src/cpu/minor/cpu.hh b/src/cpu/minor/cpu.hh
index ac9831a..0e919f33 100644
--- a/src/cpu/minor/cpu.hh
+++ b/src/cpu/minor/cpu.hh
@@ -186,6 +186,7 @@
      *  already been idled.  The stage argument should be from the
      *  enumeration Pipeline::StageId */
     void wakeupOnEvent(unsigned int stage_id);
+    EventFunctionWrapper *fetchEventWrapper;
 };

 #endif /* __CPU_MINOR_CPU_HH__ */
diff --git a/tests/gem5/asmtest/tests.py b/tests/gem5/asmtest/tests.py
index b2a9249..026b230 100755
--- a/tests/gem5/asmtest/tests.py
+++ b/tests/gem5/asmtest/tests.py
@@ -41,7 +41,7 @@
              cpu_type,
              num_cpus=4,
              max_tick=10000000000,
-             ruby=True,
+             ruby=False,
              debug_flags=None, # Debug flags passed to gem5
              full_system = False
              ):
@@ -96,10 +96,10 @@
 # https://gem5.atlassian.net/browse/GEM5-496
 # https://gem5.atlassian.net/browse/GEM5-497
 binaries = (
-#    'rv64samt-ps-sysclone_d',
-#    'rv64samt-ps-sysfutex1_d',
+    'rv64samt-ps-sysclone_d',
+    'rv64samt-ps-sysfutex1_d',
 #    'rv64samt-ps-sysfutex2_d',
-#    'rv64samt-ps-sysfutex3_d',
+    'rv64samt-ps-sysfutex3_d',
 #    'rv64samt-ps-sysfutex_d',
     'rv64ua-ps-amoadd_d',
     'rv64ua-ps-amoadd_w',
@@ -120,16 +120,16 @@
     'rv64ua-ps-amoxor_d',
     'rv64ua-ps-amoxor_w',
     'rv64ua-ps-lrsc',
-#    'rv64uamt-ps-amoadd_d',
-#    'rv64uamt-ps-amoand_d',
-#    'rv64uamt-ps-amomax_d',
-#    'rv64uamt-ps-amomaxu_d',
-#    'rv64uamt-ps-amomin_d',
-#    'rv64uamt-ps-amominu_d',
-#    'rv64uamt-ps-amoor_d',
-#    'rv64uamt-ps-amoswap_d',
-#    'rv64uamt-ps-amoxor_d',
-#    'rv64uamt-ps-lrsc_d',
+    'rv64uamt-ps-amoadd_d',
+    'rv64uamt-ps-amoand_d',
+    'rv64uamt-ps-amomax_d',
+    'rv64uamt-ps-amomaxu_d',
+    'rv64uamt-ps-amomin_d',
+    'rv64uamt-ps-amominu_d',
+    'rv64uamt-ps-amoor_d',
+    'rv64uamt-ps-amoswap_d',
+    'rv64uamt-ps-amoxor_d',
+    'rv64uamt-ps-lrsc_d',
     'rv64ud-ps-fadd',
     'rv64ud-ps-fclass',
     'rv64ud-ps-fcmp',

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/37315
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ibffe46522e2617443d29f49df180692c54830f14
Gerrit-Change-Number: 37315
Gerrit-PatchSet: 1
Gerrit-Owner: Liao Xiongfei <xiongfei.l...@gmail.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to