Re: [m5-dev] python checking error when using non-root account to build python

[Gabe Black]

Hi. You should ask this on m5-users. Most or maybe all of the people on
this list are on that list as well, along with a lot of other people.

Gabe

zhanglunkai wrote:
> Hi,
>   I am trying to build m5 with a non-root account and the operation system
> is redhat 2.6.28.
>   The system does not have recent python (the original one is 2.3.4 which
> does not fulfill m5's requirement), and I installed python 2.6.4 in my own
> directory.
>   However, when I compile m5, the system encounter an error : "Error: can't
> find library m required by python".
>   I wonder whether is problem is due to that I did not install python-devel
> package? (But python-devel do require root or sudo authority, which I do not
> have).
>   
>   Does anybody has any suggestion?
>
>
> ___
> m5-dev mailing list
> m5-dev@m5sim.org
> http://m5sim.org/mailman/listinfo/m5-dev
>   

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] python checking error when using non-root account to build python

[zhanglunkai]

Hi,
  I am trying to build m5 with a non-root account and the operation system
is redhat 2.6.28.
  The system does not have recent python (the original one is 2.3.4 which
does not fulfill m5's requirement), and I installed python 2.6.4 in my own
directory.
  However, when I compile m5, the system encounter an error : "Error: can't
find library m required by python".
  I wonder whether is problem is due to that I did not install python-devel
package? (But python-devel do require root or sudo authority, which I do not
have).
  
  Does anybody has any suggestion?


___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: Replace curTick global variable with accessor f...

[Steve Reinhardt]

changeset dac01f14f20f in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=dac01f14f20f
description:
Replace curTick global variable with accessor functions.
This step makes it easy to replace the accessor functions
(which still access a global variable) with ones that access
per-thread curTick values.

diffstat:

 src/arch/alpha/isa/decoder.isa |   2 +-
 src/arch/alpha/kernel_stats.cc |   4 +-
 src/arch/alpha/tru64/process.cc|   2 +-
 src/arch/arm/table_walker.cc   |   2 +-
 src/arch/mips/isa.cc   |   4 +-
 src/arch/mips/isa/formats/mt.isa   |   4 +-
 src/arch/mips/locked_mem.hh|   2 +-
 src/arch/mips/mt.hh|   6 +-
 src/arch/sparc/ua2005.cc   |  10 +-
 src/arch/x86/interrupts.cc |   8 +-
 src/base/cp_annotate.cc|  12 +-
 src/base/cp_annotate.hh|   2 +-
 src/base/fast_alloc.cc |   4 +-
 src/base/misc.cc   |   4 +-
 src/base/remote_gdb.cc |   4 +-
 src/base/statistics.hh |  20 +++---
 src/base/stats/mysql.cc|   2 +-
 src/base/stats/output.cc   |   6 +-
 src/base/trace.hh  |  10 +-
 src/cpu/base.cc|  22 +++---
 src/cpu/base.hh|   6 +-
 src/cpu/checker/cpu.cc |   4 +-
 src/cpu/checker/cpu_impl.hh|  18 ++--
 src/cpu/inorder/cpu.cc |  28 
 src/cpu/inorder/cpu.hh |   2 +-
 src/cpu/inorder/inorder_dyn_inst.cc|   8 +-
 src/cpu/inorder/pipeline_stage.cc  |  10 +-
 src/cpu/inorder/reg_dep_map.cc |   4 +-
 src/cpu/inorder/resource.cc|   4 +-
 src/cpu/inorder/resource_pool.9stage.cc|  20 +++---
 src/cpu/inorder/resource_pool.cc   |  16 ++--
 src/cpu/inorder/resources/branch_predictor.cc  |   4 +-
 src/cpu/inorder/resources/cache_unit.cc|   8 +-
 src/cpu/inorder/resources/execution_unit.cc|   6 +-
 src/cpu/inorder/resources/fetch_seq_unit.cc|   4 +-
 src/cpu/inorder/resources/graduation_unit.cc   |   6 +-
 src/cpu/inorder/resources/mult_div_unit.cc |   4 +-
 src/cpu/o3/commit_impl.hh  |   2 +-
 src/cpu/o3/cpu.cc  |  20 +++---
 src/cpu/o3/cpu.hh  |  12 +-
 src/cpu/o3/fetch_impl.hh   |   6 +-
 src/cpu/o3/inst_queue_impl.hh  |   2 +-
 src/cpu/o3/lsq_impl.hh |   2 +-
 src/cpu/o3/lsq_unit.hh |   2 +-
 src/cpu/o3/lsq_unit_impl.hh|   2 +-
 src/cpu/o3/thread_context_impl.hh  |   6 +-
 src/cpu/ozone/back_end.hh  |   8 +-
 src/cpu/ozone/cpu.hh   |   4 +-
 src/cpu/ozone/cpu_impl.hh  |   4 +-
 src/cpu/ozone/front_end_impl.hh|   6 +-
 src/cpu/ozone/inorder_back_end.hh  |  18 ++--
 src/cpu/ozone/inst_queue_impl.hh   |   2 +-
 src/cpu/ozone/lsq_unit.hh  |   8 +-
 src/cpu/ozone/lsq_unit_impl.hh |   6 +-
 src/cpu/ozone/lw_back_end_impl.hh  |   4 +-
 src/cpu/ozone/lw_lsq.hh|   2 +-
 src/cpu/ozone/lw_lsq_impl.hh   |   4 +-
 src/cpu/pc_event.cc|   2 +-
 src/cpu/simple/atomic.cc   |   4 +-
 src/cpu/simple/base.cc |   2 +-
 src/cpu/simple/timing.cc   |  40 ++--
 src/cpu/simple_thread.cc   |   6 +-
 src/cpu/static_inst.cc |   2 +-
 src/cpu/testers/directedtest/RubyDirectedTester.cc |   4 +-
 src/cpu/testers/memtest/memtest.cc |  12 +-
 src/cpu/testers/rubytest/Check.cc  |   6 +-
 src/cpu/testers/rubytest/RubyTester.cc |   2 +-
 src/cpu/trace/trace_cpu.cc |   8 +-
 src/dev/alpha/backdoor.cc  |   2 +-
 src/dev/arm/pl011.cc   |   6 +-
 src/dev/arm/pl111.cc   |  12 +-
 src/dev/arm/rv_ctrl.cc |   2 +-
 src/dev/arm/timer_sp804.cc |   6 +-
 src/dev/etherbus.cc|   4 +-
 src/dev/etherdump.cc   |   4 +-
 src/dev/etherlink.cc  

[m5-dev] changeset in m5: stats: rename StatEvent() function to schedStat...

[Steve Reinhardt]

changeset fc475ac7d2a4 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=fc475ac7d2a4
description:
stats: rename StatEvent() function to schedStatEvent().
This follows the style rules and is more descriptive.

diffstat:

 src/python/m5/stats.py  |   2 +-
 src/python/swig/stats.i |   3 ++-
 src/sim/pseudo_inst.cc  |   6 +++---
 src/sim/simulate.cc |   2 +-
 src/sim/stat_control.cc |  12 ++--
 src/sim/stat_control.hh |   3 ++-
 6 files changed, 15 insertions(+), 13 deletions(-)

diffs (129 lines):

diff -r f1d298b7416c -r fc475ac7d2a4 src/python/m5/stats.py
--- a/src/python/m5/stats.pyFri Jan 07 21:50:29 2011 -0800
+++ b/src/python/m5/stats.pyFri Jan 07 21:50:29 2011 -0800
@@ -28,7 +28,7 @@
 
 import internal
 
-from internal.stats import StatEvent as event
+from internal.stats import schedStatEvent as schedEvent
 from objects import Root
 
 def initText(filename, desc=True):
diff -r f1d298b7416c -r fc475ac7d2a4 src/python/swig/stats.i
--- a/src/python/swig/stats.i   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/python/swig/stats.i   Fri Jan 07 21:50:29 2011 -0800
@@ -50,7 +50,8 @@
 std::string passwd, std::string project, std::string name,
 std::string sample);
 
-void StatEvent(bool dump, bool reset, Tick when = curTick, Tick repeat = 0);
+void schedStatEvent(bool dump, bool reset,
+Tick when = curTick, Tick repeat = 0);
 
 void enable();
 void prepare();
diff -r f1d298b7416c -r fc475ac7d2a4 src/sim/pseudo_inst.cc
--- a/src/sim/pseudo_inst.ccFri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/pseudo_inst.ccFri Jan 07 21:50:29 2011 -0800
@@ -236,7 +236,7 @@
 Tick when = curTick + delay * SimClock::Int::ns;
 Tick repeat = period * SimClock::Int::ns;
 
-Stats::StatEvent(false, true, when, repeat);
+Stats::schedStatEvent(false, true, when, repeat);
 }
 
 void
@@ -249,7 +249,7 @@
 Tick when = curTick + delay * SimClock::Int::ns;
 Tick repeat = period * SimClock::Int::ns;
 
-Stats::StatEvent(true, false, when, repeat);
+Stats::schedStatEvent(true, false, when, repeat);
 }
 
 void
@@ -262,7 +262,7 @@
 Tick when = curTick + delay * SimClock::Int::ns;
 Tick repeat = period * SimClock::Int::ns;
 
-Stats::StatEvent(true, true, when, repeat);
+Stats::schedStatEvent(true, true, when, repeat);
 }
 
 void
diff -r f1d298b7416c -r fc475ac7d2a4 src/sim/simulate.cc
--- a/src/sim/simulate.cc   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/simulate.cc   Fri Jan 07 21:50:29 2011 -0800
@@ -93,7 +93,7 @@
 if (async_event) {
 async_event = false;
 if (async_statdump || async_statreset) {
-Stats::StatEvent(async_statdump, async_statreset);
+Stats::schedStatEvent(async_statdump, async_statreset);
 async_statdump = false;
 async_statreset = false;
 }
diff -r f1d298b7416c -r fc475ac7d2a4 src/sim/stat_control.cc
--- a/src/sim/stat_control.cc   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/stat_control.cc   Fri Jan 07 21:50:29 2011 -0800
@@ -48,6 +48,7 @@
 #endif
 
 #include "sim/eventq.hh"
+#include "sim/stat_control.hh"
 
 using namespace std;
 
@@ -164,7 +165,7 @@
 static Global global;
 }
 
-class _StatEvent : public Event
+class StatEvent : public Event
 {
   private:
 bool dump;
@@ -172,7 +173,7 @@
 Tick repeat;
 
   public:
-_StatEvent(bool _dump, bool _reset, Tick _repeat)
+StatEvent(bool _dump, bool _reset, Tick _repeat)
 : Event(Stat_Event_Pri), dump(_dump), reset(_reset), repeat(_repeat)
 {
 setFlags(AutoDelete);
@@ -188,16 +189,15 @@
 Stats::reset();
 
 if (repeat) {
-Event *event = new _StatEvent(dump, reset, repeat);
-mainEventQueue.schedule(event, curTick + repeat);
+Stats::schedStatEvent(dump, reset, curTick + repeat, repeat);
 }
 }
 };
 
 void
-StatEvent(bool dump, bool reset, Tick when, Tick repeat)
+schedStatEvent(bool dump, bool reset, Tick when, Tick repeat)
 {
-Event *event = new _StatEvent(dump, reset, repeat);
+Event *event = new StatEvent(dump, reset, repeat);
 mainEventQueue.schedule(event, when);
 }
 
diff -r f1d298b7416c -r fc475ac7d2a4 src/sim/stat_control.hh
--- a/src/sim/stat_control.hh   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/stat_control.hh   Fri Jan 07 21:50:29 2011 -0800
@@ -34,7 +34,8 @@
 namespace Stats {
 
 void initSimStats();
-void StatEvent(bool dump, bool reset, Tick when = curTick, Tick repeat = 0);
+void schedStatEvent(bool dump, bool reset, Tick when = curTick,
+Tick repeat = 0);
 
 } // namespace Stats
 
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: sim: clean up CountedDrainEvent slightly.

[Steve Reinhardt]

changeset f1d298b7416c in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=f1d298b7416c
description:
sim: clean up CountedDrainEvent slightly.
There's no reason for it to derive from SimLoopExitEvent.
This whole drain thing needs to be redone eventually,
but this is a stopgap to make later changes to
SimLoopExitEvent feasible.

diffstat:

 src/sim/sim_events.cc |  4 ++--
 src/sim/sim_events.hh |  2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diffs (32 lines):

diff -r 4ee66d8c1dd8 -r f1d298b7416c src/sim/sim_events.cc
--- a/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
@@ -85,14 +85,14 @@
 }
 
 CountedDrainEvent::CountedDrainEvent()
-: SimLoopExitEvent("Finished drain", 0), count(0)
+: count(0)
 { }
 
 void
 CountedDrainEvent::process()
 {
 if (--count == 0)
-exitSimLoop(cause, code);
+exitSimLoop("Finished drain", 0);
 }
 
 //
diff -r 4ee66d8c1dd8 -r f1d298b7416c src/sim/sim_events.hh
--- a/src/sim/sim_events.hh Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_events.hh Fri Jan 07 21:50:29 2011 -0800
@@ -55,7 +55,7 @@
 virtual const char *description() const;
 };
 
-class CountedDrainEvent : public SimLoopExitEvent
+class CountedDrainEvent : public Event
 {
   private:
 // Count of how many objects have not yet drained
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: sim: delete unused CheckSwapEvent code.

[Steve Reinhardt]

changeset 4ee66d8c1dd8 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=4ee66d8c1dd8
description:
sim: delete unused CheckSwapEvent code.
There's no way to even create one of these anymore.

diffstat:

 src/sim/sim_events.cc |  33 -
 src/sim/sim_events.hh |  14 --
 2 files changed, 0 insertions(+), 47 deletions(-)

diffs (63 lines):

diff -r afe8476ee9e9 -r 4ee66d8c1dd8 src/sim/sim_events.cc
--- a/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
@@ -123,36 +123,3 @@
 {
 return "counted exit";
 }
-
-CheckSwapEvent::CheckSwapEvent(int ival)
-: interval(ival)
-{
-mainEventQueue.schedule(this, curTick + interval);
-}
-
-void
-CheckSwapEvent::process()
-{
-/*  Check the amount of free swap space  */
-long swap;
-
-/*  returns free swap in KBytes  */
-swap = procInfo("/proc/meminfo", "SwapFree:");
-
-if (swap < 1000)
-ccprintf(cerr, "\a\a\aWarning! Swap space is low (%d)\n", swap);
-
-if (swap < 100) {
-cerr << "\a\aAborting Simulation! Inadequate swap space!\n\n";
-exitSimLoop("Lack of swap space");
-}
-
-assert(getFlags(IsMainQueue));
-mainEventQueue.schedule(this, curTick + interval);
-}
-
-const char *
-CheckSwapEvent::description() const
-{
-return "check swap";
-}
diff -r afe8476ee9e9 -r 4ee66d8c1dd8 src/sim/sim_events.hh
--- a/src/sim/sim_events.hh Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_events.hh Fri Jan 07 21:50:29 2011 -0800
@@ -90,19 +90,5 @@
 virtual const char *description() const;
 };
 
-//
-// Event to check swap usage
-//
-class CheckSwapEvent : public Event
-{
-  private:
-int interval;
-
-  public:
-CheckSwapEvent(int ival);
-void process(); // process event
-
-virtual const char *description() const;
-};
 
 #endif  // __SIM_SIM_EVENTS_HH__
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: pseudoinst: get rid of mainEventQueue references.

[Steve Reinhardt]

changeset afe8476ee9e9 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=afe8476ee9e9
description:
pseudoinst: get rid of mainEventQueue references.
Avoid direct references to mainEventQueue in pseudo-insts
by indirecting through associated CPU object.
Made exitSimLoop() more flexible to enable some of these.

diffstat:

 src/sim/pseudo_inst.cc |  24 +---
 src/sim/sim_events.cc  |   6 +++---
 src/sim/sim_exit.hh|   4 +++-
 3 files changed, 19 insertions(+), 15 deletions(-)

diffs (107 lines):

diff -r 5d3dad7a1b36 -r afe8476ee9e9 src/sim/pseudo_inst.cc
--- a/src/sim/pseudo_inst.ccFri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/pseudo_inst.ccFri Jan 07 21:50:29 2011 -0800
@@ -88,17 +88,19 @@
 void
 quiesceNs(ThreadContext *tc, uint64_t ns)
 {
-if (!tc->getCpuPtr()->params()->do_quiesce || ns == 0)
+BaseCPU *cpu = tc->getCpuPtr();
+
+if (!cpu->params()->do_quiesce || ns == 0)
 return;
 
 EndQuiesceEvent *quiesceEvent = tc->getQuiesceEvent();
 
 Tick resume = curTick + SimClock::Int::ns * ns;
 
-mainEventQueue.reschedule(quiesceEvent, resume, true);
+cpu->reschedule(quiesceEvent, resume, true);
 
 DPRINTF(Quiesce, "%s: quiesceNs(%d) until %d\n",
-tc->getCpuPtr()->name(), ns, resume);
+cpu->name(), ns, resume);
 
 tc->suspend();
 if (tc->getKernelStats())
@@ -108,17 +110,19 @@
 void
 quiesceCycles(ThreadContext *tc, uint64_t cycles)
 {
-if (!tc->getCpuPtr()->params()->do_quiesce || cycles == 0)
+BaseCPU *cpu = tc->getCpuPtr();
+
+if (!cpu->params()->do_quiesce || cycles == 0)
 return;
 
 EndQuiesceEvent *quiesceEvent = tc->getQuiesceEvent();
 
-Tick resume = curTick + tc->getCpuPtr()->ticks(cycles);
+Tick resume = curTick + cpu->ticks(cycles);
 
-mainEventQueue.reschedule(quiesceEvent, resume, true);
+cpu->reschedule(quiesceEvent, resume, true);
 
 DPRINTF(Quiesce, "%s: quiesceCycles(%d) until %d\n",
-tc->getCpuPtr()->name(), cycles, resume);
+cpu->name(), cycles, resume);
 
 tc->suspend();
 if (tc->getKernelStats())
@@ -153,8 +157,7 @@
 m5exit(ThreadContext *tc, Tick delay)
 {
 Tick when = curTick + delay * SimClock::Int::ns;
-Event *event = new SimLoopExitEvent("m5_exit instruction encountered", 0);
-mainEventQueue.schedule(event, when);
+exitSimLoop("m5_exit instruction encountered", 0, when);
 }
 
 #if FULL_SYSTEM
@@ -271,8 +274,7 @@
 Tick when = curTick + delay * SimClock::Int::ns;
 Tick repeat = period * SimClock::Int::ns;
 
-Event *event = new SimLoopExitEvent("checkpoint", 0, repeat);
-mainEventQueue.schedule(event, when);
+exitSimLoop("checkpoint", 0, when, repeat);
 }
 
 #if FULL_SYSTEM
diff -r 5d3dad7a1b36 -r afe8476ee9e9 src/sim/sim_events.cc
--- a/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_events.cc Fri Jan 07 21:50:29 2011 -0800
@@ -78,10 +78,10 @@
 }
 
 void
-exitSimLoop(const std::string &message, int exit_code)
+exitSimLoop(const std::string &message, int exit_code, Tick when, Tick repeat)
 {
-Event *event = new SimLoopExitEvent(message, exit_code);
-mainEventQueue.schedule(event, curTick);
+Event *event = new SimLoopExitEvent(message, exit_code, repeat);
+mainEventQueue.schedule(event, when);
 }
 
 CountedDrainEvent::CountedDrainEvent()
diff -r 5d3dad7a1b36 -r afe8476ee9e9 src/sim/sim_exit.hh
--- a/src/sim/sim_exit.hh   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/sim/sim_exit.hh   Fri Jan 07 21:50:29 2011 -0800
@@ -35,6 +35,7 @@
 #include 
 
 #include "base/types.hh"
+#include "sim/core.hh"
 
 // forward declaration
 class Callback;
@@ -49,6 +50,7 @@
 /// Python) at the end of the current cycle (curTick).  The message
 /// and exit_code parameters are saved in the SimLoopExitEvent to
 /// indicate why the exit occurred.
-void exitSimLoop(const std::string &message, int exit_code = 0);
+void exitSimLoop(const std::string &message, int exit_code = 0,
+ Tick when = curTick, Tick repeat = 0);
 
 #endif // __SIM_EXIT_HH__
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: inorder: replace schedEvent() code with resched...

[Steve Reinhardt]

changeset 5d3dad7a1b36 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=5d3dad7a1b36
description:
inorder: replace schedEvent() code with reschedule().
There were several copies of similar functions that looked
like they all replicated reschedule(), so I replaced them
with direct calls.  Keeping this separate from the previous
cset since there may be some subtle functional differences
if the code ever reschedules an event that is scheduled but
not squashed (though none were detected in the regressions).

diffstat:

 src/cpu/inorder/cpu.cc   |   8 ++--
 src/cpu/inorder/cpu.hh   |   8 ++--
 src/cpu/inorder/resource.cc  |  10 +++---
 src/cpu/inorder/resource_pool.cc |   9 ++---
 4 files changed, 9 insertions(+), 26 deletions(-)

diffs (73 lines):

diff -r 94fdc8111d7b -r 5d3dad7a1b36 src/cpu/inorder/cpu.cc
--- a/src/cpu/inorder/cpu.ccFri Jan 07 21:50:29 2011 -0800
+++ b/src/cpu/inorder/cpu.ccFri Jan 07 21:50:29 2011 -0800
@@ -157,12 +157,8 @@
 void
 InOrderCPU::CPUEvent::scheduleEvent(int delay)
 {
-Tick when = cpu->nextCycle(curTick + cpu->ticks(delay));
-
-if (squashed())
-cpu->reschedule(this, when);
-else if (!scheduled())
-cpu->schedule(this, when);
+assert(!scheduled() || squashed());
+cpu->reschedule(this, cpu->nextCycle(curTick + cpu->ticks(delay)), true);
 }
 
 void
diff -r 94fdc8111d7b -r 5d3dad7a1b36 src/cpu/inorder/cpu.hh
--- a/src/cpu/inorder/cpu.hhFri Jan 07 21:50:29 2011 -0800
+++ b/src/cpu/inorder/cpu.hhFri Jan 07 21:50:29 2011 -0800
@@ -156,12 +156,8 @@
 /** Schedule tick event, regardless of its current state. */
 void scheduleTickEvent(int delay)
 {
-Tick when = nextCycle(curTick + ticks(delay));
-
-if (tickEvent.squashed())
-reschedule(&tickEvent, when);
-else if (!tickEvent.scheduled())
-schedule(&tickEvent, when);
+assert(!tickEvent.scheduled() || tickEvent.squashed());
+reschedule(&tickEvent, nextCycle(curTick + ticks(delay)), true);
 }
 
 /** Unschedule tick event, regardless of its current state. */
diff -r 94fdc8111d7b -r 5d3dad7a1b36 src/cpu/inorder/resource.cc
--- a/src/cpu/inorder/resource.cc   Fri Jan 07 21:50:29 2011 -0800
+++ b/src/cpu/inorder/resource.cc   Fri Jan 07 21:50:29 2011 -0800
@@ -502,11 +502,7 @@
 void
 ResourceEvent::scheduleEvent(int delay)
 {
-InOrderCPU *cpu = resource->cpu;
-Tick when = curTick + resource->ticks(delay);
-
-if (squashed())
-cpu->reschedule(this, when);
-else if (!scheduled())
-cpu->schedule(this, when);
+assert(!scheduled() || squashed());
+resource->cpu->reschedule(this,
+  curTick + resource->ticks(delay), true);
 }
diff -r 94fdc8111d7b -r 5d3dad7a1b36 src/cpu/inorder/resource_pool.cc
--- a/src/cpu/inorder/resource_pool.cc  Fri Jan 07 21:50:29 2011 -0800
+++ b/src/cpu/inorder/resource_pool.cc  Fri Jan 07 21:50:29 2011 -0800
@@ -541,13 +541,8 @@
 ResourcePool::ResPoolEvent::scheduleEvent(int delay)
 {
 InOrderCPU *cpu = resPool->cpu;
-Tick when = cpu->nextCycle(curTick + cpu->ticks(delay));
-
-if (squashed()) {
-cpu->reschedule(this, when);
-} else if (!scheduled()) {
-cpu->schedule(this, when);
-}
+assert(!scheduled() || squashed());
+cpu->reschedule(this, cpu->nextCycle(curTick + cpu->ticks(delay)), true);
 }
 
 /** Unschedule resource event, regardless of its current state. */
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] changeset in m5: inorder: get rid of references to mainEventQueue.

[Steve Reinhardt]

changeset 94fdc8111d7b in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=94fdc8111d7b
description:
inorder: get rid of references to mainEventQueue.
Events need to be scheduled on the queue assigned
to the SimObject, not on the global queue (which
should be going away).
Also cleaned up a number of redundant expressions
that made the code unnecessarily verbose.

diffstat:

 src/cpu/inorder/cpu.cc   |  14 +-
 src/cpu/inorder/cpu.hh   |   8 +++---
 src/cpu/inorder/resource.cc  |  12 +
 src/cpu/inorder/resource.hh  |   8 +-
 src/cpu/inorder/resource_pool.cc |  53 ++-
 5 files changed, 43 insertions(+), 52 deletions(-)

diffs (236 lines):

diff -r b5003ac75977 -r 94fdc8111d7b src/cpu/inorder/cpu.cc
--- a/src/cpu/inorder/cpu.ccFri Jan 07 21:50:13 2011 -0800
+++ b/src/cpu/inorder/cpu.ccFri Jan 07 21:50:29 2011 -0800
@@ -157,12 +157,12 @@
 void
 InOrderCPU::CPUEvent::scheduleEvent(int delay)
 {
+Tick when = cpu->nextCycle(curTick + cpu->ticks(delay));
+
 if (squashed())
-mainEventQueue.reschedule(this, cpu->nextCycle(curTick +
-   cpu->ticks(delay)));
+cpu->reschedule(this, when);
 else if (!scheduled())
-mainEventQueue.schedule(this, cpu->nextCycle(curTick +
- cpu->ticks(delay)));
+cpu->schedule(this, when);
 }
 
 void
@@ -540,7 +540,7 @@
 } else {
 //Tick next_tick = curTick + cycles(1);
 //tickEvent.schedule(next_tick);
-mainEventQueue.schedule(&tickEvent, nextCycle(curTick + 1));
+schedule(&tickEvent, nextCycle(curTick + 1));
 DPRINTF(InOrderCPU, "Scheduled CPU for next tick @ %i.\n", 
 nextCycle(curTick + 1));
 }
@@ -701,7 +701,7 @@
 if (delay >= 0) {
 DPRINTF(InOrderCPU, "Scheduling CPU Event (%s) for cycle %i, 
[tid:%i].\n",
 eventNames[c_event], curTick + delay, tid);
-mainEventQueue.schedule(cpu_event, sked_tick);
+schedule(cpu_event, sked_tick);
 } else {
 cpu_event->process();
 cpuEventRemoveList.push(cpu_event);
@@ -1403,7 +1403,7 @@
 
 numCycles += extra_cycles;
 
-mainEventQueue.schedule(&tickEvent, nextCycle(curTick));
+schedule(&tickEvent, nextCycle(curTick));
 }
 
 #if FULL_SYSTEM
diff -r b5003ac75977 -r 94fdc8111d7b src/cpu/inorder/cpu.hh
--- a/src/cpu/inorder/cpu.hhFri Jan 07 21:50:13 2011 -0800
+++ b/src/cpu/inorder/cpu.hhFri Jan 07 21:50:29 2011 -0800
@@ -156,12 +156,12 @@
 /** Schedule tick event, regardless of its current state. */
 void scheduleTickEvent(int delay)
 {
+Tick when = nextCycle(curTick + ticks(delay));
+
 if (tickEvent.squashed())
-  mainEventQueue.reschedule(&tickEvent, 
-nextCycle(curTick + ticks(delay)));
+reschedule(&tickEvent, when);
 else if (!tickEvent.scheduled())
-  mainEventQueue.schedule(&tickEvent, 
-  nextCycle(curTick + ticks(delay)));
+schedule(&tickEvent, when);
 }
 
 /** Unschedule tick event, regardless of its current state. */
diff -r b5003ac75977 -r 94fdc8111d7b src/cpu/inorder/resource.cc
--- a/src/cpu/inorder/resource.cc   Fri Jan 07 21:50:13 2011 -0800
+++ b/src/cpu/inorder/resource.cc   Fri Jan 07 21:50:29 2011 -0800
@@ -498,3 +498,15 @@
 
 return desc.c_str();
 }
+
+void
+ResourceEvent::scheduleEvent(int delay)
+{
+InOrderCPU *cpu = resource->cpu;
+Tick when = curTick + resource->ticks(delay);
+
+if (squashed())
+cpu->reschedule(this, when);
+else if (!scheduled())
+cpu->schedule(this, when);
+}
diff -r b5003ac75977 -r 94fdc8111d7b src/cpu/inorder/resource.hh
--- a/src/cpu/inorder/resource.hh   Fri Jan 07 21:50:13 2011 -0800
+++ b/src/cpu/inorder/resource.hh   Fri Jan 07 21:50:29 2011 -0800
@@ -277,13 +277,7 @@
 void setSlot(int slot) { slotIdx = slot; }
 
 /** Schedule resource event, regardless of its current state. */
-void scheduleEvent(int delay)
-{
-if (squashed())
-  mainEventQueue.reschedule(this, curTick + resource->ticks(delay));
-else if (!scheduled())
-  mainEventQueue.schedule(this, curTick + resource->ticks(delay));
-}
+void scheduleEvent(int delay);
 
 /** Unschedule resource event, regardless of its current state. */
 void unscheduleEvent()
diff -r b5003ac75977 -r 94fdc8111d7b src/cpu/inorder/resource_pool.cc
--- a/src/cpu/inorder/resource_pool.cc  Fri Jan 07 21:50:13 2011 -0800
+++ b/src/cpu/inorder/resource_pool.cc  Fri Jan 07 21:50:29 2011 -0800
@@ -244,6 +244,8 @@
 {
 assert(delay >= 0);
 
+Tick when = cpu->nextCycle(curTick + cpu->ticks(delay));
+
 switch (e_type)
 {
   case InO

[m5-dev] changeset in m5: scons: show sources and targets when building, ...

[Steve Reinhardt]

changeset b5003ac75977 in /z/repo/m5
details: http://repo.m5sim.org/m5?cmd=changeset;node=b5003ac75977
description:
scons: show sources and targets when building, and colorize output.

I like the brevity of Ali's recent change, but the ambiguity of
sometimes showing the source and sometimes the target is a little
confusing.  This patch makes scons typically list all sources and
all targets for each action, with the common path prefix factored
out for brevity.  It's a little more verbose now but also more
informative.

Somehow Ali talked me into adding colors too, which is a whole
'nother story.

diffstat:

 SConstruct |  114 ++--
 src/SConscript |   32 +-
 src/arch/SConscript|2 +-
 src/arch/isa_parser.py |2 -
 src/python/SConscript  |1 +
 src/python/m5/util/terminal.py |  113 
 6 files changed, 227 insertions(+), 37 deletions(-)

diffs (truncated from 442 to 300 lines):

diff -r 9f9e10967912 -r b5003ac75977 SConstruct
--- a/SConstructTue Jan 04 21:40:49 2011 -0600
+++ b/SConstructFri Jan 07 21:50:13 2011 -0800
@@ -1,5 +1,6 @@
 # -*- mode:python -*-
 
+# Copyright (c) 2011 Advanced Micro Devices, Inc.
 # Copyright (c) 2009 The Hewlett-Packard Development Company
 # Copyright (c) 2004-2005 The Regents of The University of Michigan
 # All rights reserved.
@@ -120,6 +121,18 @@
 
 from m5.util import compareVersions, readCommand
 
+AddOption('--colors', dest='use_colors', action='store_true')
+AddOption('--no-colors', dest='use_colors', action='store_false')
+use_colors = GetOption('use_colors')
+
+if use_colors:
+from m5.util.terminal import termcap
+elif use_colors is None:
+# option unspecified; default behavior is to use colors iff isatty
+from m5.util.terminal import tty_termcap as termcap
+else:
+from m5.util.terminal import no_termcap as termcap
+
 
 #
 # Set up the main build environment.
@@ -357,7 +370,7 @@
 # the ext directory should be on the #includes path
 main.Append(CPPPATH=[Dir('ext')])
 
-def _STRIP(path, env):
+def strip_build_path(path, env):
 path = str(path)
 variant_base = env['BUILDROOT'] + os.path.sep
 if path.startswith(variant_base):
@@ -366,29 +379,94 @@
 path = path[6:]
 return path
 
-def _STRIP_SOURCE(target, source, env, for_signature):
-return _STRIP(source[0], env)
-main['STRIP_SOURCE'] = _STRIP_SOURCE
+# Generate a string of the form:
+#   common/path/prefix/src1, src2 -> tgt1, tgt2
+# to print while building.
+class Transform(object):
+# all specific color settings should be here and nowhere else
+tool_color = termcap.Normal
+pfx_color = termcap.Yellow
+srcs_color = termcap.Yellow + termcap.Bold
+arrow_color = termcap.Blue + termcap.Bold
+tgts_color = termcap.Yellow + termcap.Bold
 
-def _STRIP_TARGET(target, source, env, for_signature):
-return _STRIP(target[0], env)
-main['STRIP_TARGET'] = _STRIP_TARGET
+def __init__(self, tool, max_sources=99):
+self.format = self.tool_color + (" [%8s] " % tool) \
+  + self.pfx_color + "%s" \
+  + self.srcs_color + "%s" \
+  + self.arrow_color + " -> " \
+  + self.tgts_color + "%s" \
+  + termcap.Normal
+self.max_sources = max_sources
+
+def __call__(self, target, source, env, for_signature=None):
+# truncate source list according to max_sources param
+source = source[0:self.max_sources]
+def strip(f):
+return strip_build_path(str(f), env)
+if len(source) > 0:
+srcs = map(strip, source)
+else:
+srcs = ['']
+tgts = map(strip, target)
+# surprisingly, os.path.commonprefix is a dumb char-by-char string
+# operation that has nothing to do with paths.
+com_pfx = os.path.commonprefix(srcs + tgts)
+com_pfx_len = len(com_pfx)
+if com_pfx:
+# do some cleanup and sanity checking on common prefix
+if com_pfx[-1] == ".":
+# prefix matches all but file extension: ok
+# back up one to change 'foo.cc -> o' to 'foo.cc -> .o'
+com_pfx = com_pfx[0:-1]
+elif com_pfx[-1] == "/":
+# common prefix is directory path: OK
+pass
+else:
+src0_len = len(srcs[0])
+tgt0_len = len(tgts[0])
+if src0_len == com_pfx_len:
+# source is a substring of target, OK
+pass
+elif tgt0_len == com_pfx_len:
+# target is a substring of source, need to back up to
+# avoid empty string on RHS o

Re: [m5-dev] Review Request: x86: Timing support for pagetable walker

[Joel Hestness]

> On 2011-01-07 05:51:30, Gabe Black wrote:
> > The code seems ok, but why do we need to have multiple outstanding page 
> > walks in timing mode again?
> 
> Gabe Black wrote:
> Actually, I wrote the above before I'd read it carefully. My question 
> still stands, but there are some areas that need to be fixed up. Also, since 
> translation is very much on the critical path, make sure you measure how much 
> this change affects performance. I expect with the addition indirection at 
> least there will be some slow down, and we should know what that is before we 
> commit anything.

In timing mode x86, if a memory address translation misses in the TLB AND 
happens to be an unaligned access (one that straddles a page boundary), the TLB 
promptly fires both of the requests to the page table walker.  The old 
implementation of the walker doesn't support multiple outstanding requests, so 
it immediately crashes simulation with a state assertion failure (I asked a few 
questions about this in June and July, back when I made the changes to the 
walker).  The implementation in this patch can queue the requests and service 
them sequentially.  It should be a simple future extension to service them 
concurrently.

I modeled this implementation after the ARM implementation in 
arch/arm/table_walker.*.

Concerning the slowdown, the frequency of unaligned accesses that miss in the 
TLB is extremely rare (<10 in seconds of simulated system time).  Since timing 
mode doesn't work without this fix, there isn't a way to compare performance 
against a baseline.


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.hh, line 187
> > 
> >
> > Why call this reqType instead of leaving it as mode? requests have 
> > types which are orthogonal to this, and it's called mode everywhere else.

Good point.  I'm not sure why I named it that.  "mode" would be better.


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 77
> > 
> >
> > These should use FastAlloc if at all possible since they're on a 
> > critical path and the heap is slow.

Is this as simple as having WalkerState inherit from FastAlloc?


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 89
> > 
> >
> > Memory leak.

Well, that's embarrassing :P


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 179
> > 
> >
> > Is letting translations pass each other realistic? I worry we're making 
> > our walker artificially powerful. These loops will also slow things down 
> > potentially.

This is an abstract implementation just to get the walker to work.  It can be 
easily molded to order the requests appropriately.

On the topic of slowdown, having more than one request in the queue is 
extremely rare, so any slowdown should be trivial.


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 541
> > 
> >
> > Declare this where it's used.

I think I had other plans for this variable, but it doesn't look like I 
followed through.  Moving it to line 566 should be a simple fix.


> On 2011-01-07 05:51:30, Gabe Black wrote:
> > src/arch/x86/pagetable_walker.cc, line 574
> > 
> >
> > Why is this pulled out into its own switch statement? That will slow 
> > down the code and makes things more complicated.

As I recall, this was part of my intermediate solution to the unaligned access 
problem.  These lines can be moved back to the previous locations.


- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/396/#review649
---


On 2011-01-06 16:12:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/396/
> ---
> 
> (Updated 2011-01-06 16:12:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Timing support for pagetable walker
> 
> Move page table walker state to its own object type, and make the
> walker instantiate state for each outstanding walk. By storing the
> states in a queue, the walker is able to handle multiple outstanding
> timing requests. Note that functional walks use separate state
> elements.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/pagetable_walker.hh 9f9e10967912 
> 

Re: [m5-dev] Review Request: MessagePort: implemented virtual recvTiming avoiding double delete

[Gabriel Michael Black]

I'll have to look at this again and see if I can figure out what's  
going on. For now I wanted to mention that Valgrind isn't necessarily  
going to be useful in determining if there's a memory leak here  
because these messages are sent infrequently and only leak a little  
bit each time. In the course of a simulation there will be a lot of  
other stuff that doesn't get deallocated, and this would pretty easily  
slip into the noise. I think it might break out things that it only  
thinks are leaks and memory that's no longer reachable, but we've got  
enough crazy stuff going on with python/swig/etc., that may not be  
very accurate. You might keep a count of the packets that have been  
allocated but not deleted and see if it gradually goes up on average  
or stays fairly steady.


Gabe

Quoting Joel Hestness :





On 2011-01-07 04:21:05, Gabe Black wrote:
> I think there are two problems with this patch. First, if at all  
possible we should avoid the code duplication we'd now have for the  
recvTiming function. Second, while this probably does fix the  
legitimate problem of deleting packets twice, I think it creates a  
memory leak in the process. I suspect if you leave your other  
changes in place but get rid of your custom recvTiming function,  
things will still work. The packet won't be deleted by the device,  
won't be deleted after being received as a request in either atomic  
or timing mode, but will be deleted in both modes after being  
received as a response. The "virtual" you added in tport.hh could  
almost certainly go away then too.


Brad Beckmann wrote:
Joel is the one who actually wrote this patch, so hopefully he  
can elaborate on the possible the memory leak.  I'll hold off on  
this patch until he can respond.


Actually, the double delete problem still exists if we removed the  
(almost) replicated recvTiming code.  This is because  
pkt->needsResponse() returns false when the message type is  
MemCmd::MessageResp, which causes execution of the needsResponse  
else clause in SimpleTimingPort::recvTiming.  It would be freed  
there, as well as in recvAtomic.


I think when I tested this with Valgrind, I didn't see the memory  
leak (doesn't mean it doesn't exist).  However, I don't think I was  
able to justify to myself why it didn't occur.


I remember that I spent a while trying to figure out how to make  
this work nicely, but the inheritance SimpleTimingPort ->  
MessagePort -> IntPort, and the overloading that that implies makes  
this quite difficult to analyze.  For instance, I'm still not clear  
why the new MemCmd, MessageReq/Resp, needed to be defined for this.




On 2011-01-07 04:21:05, Gabe Black wrote:
> src/mem/tport.hh, line 145
> 
>
> Marking this as explicitly virtual shouldn't really be  
necessary. Is there a reason you want to?


I think I had trouble compiling since MessagePort overloads  
recvTiming.  In this patch, MessagePort would become the first  
(only) descendant class of SimpleTimingPort that overloads recvTiming.



- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/#review639
---


On 2011-01-06 15:56:19, Brad Beckmann wrote:


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/
---

(Updated 2011-01-06 15:56:19)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,  
and Nathan Binkert.



Summary
---

MessagePort: implemented virtual recvTiming avoiding double delete

Double packet delete problem is due to an interrupt device deleting a packet
that the SimpleTimingPort also deletes. Since MessagePort descends from
SimpleTimingPort, simply reimplement the failing code from SimpleTimingPort:
recvTiming.


Diffs
-

  src/arch/x86/interrupts.cc 9f9e10967912
  src/dev/x86/intdev.hh 9f9e10967912
  src/mem/mport.hh 9f9e10967912
  src/mem/mport.cc 9f9e10967912
  src/mem/tport.hh 9f9e10967912

Diff: http://reviews.m5sim.org/r/382/diff


Testing
---


Thanks,

Brad








___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[Joel Hestness]

> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 58
> > 
> >
> > Better wording might be "Need access to page tables."

I like that change


> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 70
> > 
> >
> > Having a temporary variable here seems unnecessary unless it's to 
> > prevent having to wrap the next line. It's not a big deal, though.

As far as I can tell, convention in ALL other code is to store the fault as a 
temporary variable, even if it could simply be pushed into the if-clause.


> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 73
> > 
> >
> > This is very suspicious. The request size was set to 0 when you 
> > constructed the request object, so this is anding the original address with 
> > -1. That doesn't do anything, so you're really just oring the addresses 
> > together. The TLB will already have taken care of any page offset/page 
> > number munging that you need. Actually, this whole function is suspect (not 
> > because of your code) since there's no guarantee code/data and/or different 
> > forms of data will be translated the same, or that flags aren't important.
> 
> Brad Beckmann wrote:
> I agree, something seems off here.  However, I'll let Joel respond before 
> changing it.  At least there needs to be a comment explaining why this 
> calculation is necessary.

The size field of the request is set in the functional portion of 
Walker::WalkerState::startWalk in my other patch for review.  The physical 
address that is returned from vtophys needs to include the offset into the 
page, which in x86 can have multiple different sizes.  The page table contains 
the information about the page size, so it needs to be passed in the request 
object through startFunctional().


- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/385/#review642
---


On 2011-01-06 15:59:24, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/385/
> ---
> 
> (Updated 2011-01-06 15:59:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: page table walker functional support
> 
> src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table
> src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table
> src/arch/x86/tlb.cc: Added method to return pointer to walker
> src/arch/x86/tlb.hh: Added method to return pointer to walker
> src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping
> 
> 
> Diffs
> -
> 
>   src/arch/x86/vtophys.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/385/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: m5: added work completed monitoring support

[Gabriel Michael Black]

Please update existing review requests instead of creating new ones.

Gabe

Quoting Brad Beckmann :



---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/418/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,  
and Nathan Binkert.



Summary
---

m5: added work completed monitoring support


Diffs
-

  configs/common/FSConfig.py 9f9e10967912
  configs/common/Options.py 9f9e10967912
  configs/example/fs.py 9f9e10967912
  configs/example/ruby_fs.py 9f9e10967912
  src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912
  src/cpu/base.hh 9f9e10967912
  src/cpu/base.cc 9f9e10967912
  src/sim/SConscript 9f9e10967912
  src/sim/System.py 9f9e10967912
  src/sim/pseudo_inst.hh 9f9e10967912
  src/sim/pseudo_inst.cc 9f9e10967912
  src/sim/system.hh 9f9e10967912
  src/sim/system.cc 9f9e10967912
  util/m5/m5op_x86.S 9f9e10967912
  util/m5/m5ops.h 9f9e10967912

Diff: http://reviews.m5sim.org/r/418/diff


Testing
---


Thanks,

Brad





___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: m5: added work completed monitoring support

[Gabriel Michael Black]

Pardon my ignorance, but what does it mean for a benchmark to have  
work items? Would that be if it does a number of "things" as it runs  
(processes n input files, performs y procedures, etc.) and each is  
blocked out with these new insts? Is that a well known term? If so  
then great, but if not a comment or two would be helpful.


Gabe

Quoting Brad Beckmann :





On 2011-01-07 06:13:30, Gabe Black wrote:
> What are these new pseudo instructions and script options for? I  
don't remember these being mentioned before. You should NOT check  
in the m5.disableAllListeners() line.


This patch tracks work items for those benchmarks that have had  
their work items annotated.  The disableAllListeners line should not  
have been included.  Sorry about that.




On 2011-01-07 06:13:30, Gabe Black wrote:
> configs/common/Options.py, line 57
> 
>
> What are these for? Their descriptions aren't very helpful.  
If they only do something for x86 it seems wrong that they're  
displayed for every ISA.


The are not x86 specific at all.  They can be used by any  
workload/ISA combination.



- Brad


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/398/#review652
---


On 2011-01-06 16:13:29, Brad Beckmann wrote:


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/398/
---

(Updated 2011-01-06 16:13:29)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,  
and Nathan Binkert.



Summary
---

m5: added work completed monitoring support


Diffs
-

  configs/common/FSConfig.py 9f9e10967912
  configs/common/Options.py 9f9e10967912
  configs/example/fs.py 9f9e10967912
  configs/example/ruby_fs.py 9f9e10967912
  src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912
  src/cpu/base.hh 9f9e10967912
  src/cpu/base.cc 9f9e10967912
  src/sim/SConscript 9f9e10967912
  src/sim/System.py 9f9e10967912
  src/sim/pseudo_inst.hh 9f9e10967912
  src/sim/pseudo_inst.cc 9f9e10967912
  src/sim/system.hh 9f9e10967912
  src/sim/system.cc 9f9e10967912
  util/m5/m5op_x86.S 9f9e10967912
  util/m5/m5ops.h 9f9e10967912

Diff: http://reviews.m5sim.org/r/398/diff


Testing
---


Thanks,

Brad








___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: m5: added work completed monitoring support

[Brad Beckmann]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/418/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

m5: added work completed monitoring support


Diffs
-

  configs/common/FSConfig.py 9f9e10967912 
  configs/common/Options.py 9f9e10967912 
  configs/example/fs.py 9f9e10967912 
  configs/example/ruby_fs.py 9f9e10967912 
  src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912 
  src/cpu/base.hh 9f9e10967912 
  src/cpu/base.cc 9f9e10967912 
  src/sim/SConscript 9f9e10967912 
  src/sim/System.py 9f9e10967912 
  src/sim/pseudo_inst.hh 9f9e10967912 
  src/sim/pseudo_inst.cc 9f9e10967912 
  src/sim/system.hh 9f9e10967912 
  src/sim/system.cc 9f9e10967912 
  util/m5/m5op_x86.S 9f9e10967912 
  util/m5/m5ops.h 9f9e10967912 

Diff: http://reviews.m5sim.org/r/418/diff


Testing
---


Thanks,

Brad

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: MessagePort: implemented virtual recvTiming avoiding double delete

[Joel Hestness]

> On 2011-01-07 04:21:05, Gabe Black wrote:
> > I think there are two problems with this patch. First, if at all possible 
> > we should avoid the code duplication we'd now have for the recvTiming 
> > function. Second, while this probably does fix the legitimate problem of 
> > deleting packets twice, I think it creates a memory leak in the process. I 
> > suspect if you leave your other changes in place but get rid of your custom 
> > recvTiming function, things will still work. The packet won't be deleted by 
> > the device, won't be deleted after being received as a request in either 
> > atomic or timing mode, but will be deleted in both modes after being 
> > received as a response. The "virtual" you added in tport.hh could almost 
> > certainly go away then too.
> 
> Brad Beckmann wrote:
> Joel is the one who actually wrote this patch, so hopefully he can 
> elaborate on the possible the memory leak.  I'll hold off on this patch until 
> he can respond.

Actually, the double delete problem still exists if we removed the (almost) 
replicated recvTiming code.  This is because pkt->needsResponse() returns false 
when the message type is MemCmd::MessageResp, which causes execution of the 
needsResponse else clause in SimpleTimingPort::recvTiming.  It would be freed 
there, as well as in recvAtomic.

I think when I tested this with Valgrind, I didn't see the memory leak (doesn't 
mean it doesn't exist).  However, I don't think I was able to justify to myself 
why it didn't occur.

I remember that I spent a while trying to figure out how to make this work 
nicely, but the inheritance SimpleTimingPort -> MessagePort -> IntPort, and the 
overloading that that implies makes this quite difficult to analyze.  For 
instance, I'm still not clear why the new MemCmd, MessageReq/Resp, needed to be 
defined for this.


> On 2011-01-07 04:21:05, Gabe Black wrote:
> > src/mem/tport.hh, line 145
> > 
> >
> > Marking this as explicitly virtual shouldn't really be necessary. Is 
> > there a reason you want to?

I think I had trouble compiling since MessagePort overloads recvTiming.  In 
this patch, MessagePort would become the first (only) descendant class of 
SimpleTimingPort that overloads recvTiming.


- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/#review639
---


On 2011-01-06 15:56:19, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/382/
> ---
> 
> (Updated 2011-01-06 15:56:19)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> MessagePort: implemented virtual recvTiming avoiding double delete
> 
> Double packet delete problem is due to an interrupt device deleting a packet
> that the SimpleTimingPort also deletes. Since MessagePort descends from
> SimpleTimingPort, simply reimplement the failing code from SimpleTimingPort:
> recvTiming.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/mem/mport.hh 9f9e10967912 
>   src/mem/mport.cc 9f9e10967912 
>   src/mem/tport.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/382/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: ruby: x86 fs config support

[Brad Beckmann]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/417/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

ruby: x86 fs config support


Diffs
-

  configs/common/FSConfig.py 9f9e10967912 
  configs/example/ruby_fs.py 9f9e10967912 

Diff: http://reviews.m5sim.org/r/417/diff


Testing
---


Thanks,

Brad

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: m5: added work completed monitoring support

[Brad Beckmann]

> On 2011-01-07 06:13:30, Gabe Black wrote:
> > What are these new pseudo instructions and script options for? I don't 
> > remember these being mentioned before. You should NOT check in the 
> > m5.disableAllListeners() line.

This patch tracks work items for those benchmarks that have had their work 
items annotated.  The disableAllListeners line should not have been included.  
Sorry about that.


> On 2011-01-07 06:13:30, Gabe Black wrote:
> > configs/common/Options.py, line 57
> > 
> >
> > What are these for? Their descriptions aren't very helpful. If they 
> > only do something for x86 it seems wrong that they're displayed for every 
> > ISA.

The are not x86 specific at all.  They can be used by any workload/ISA 
combination.


- Brad


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/398/#review652
---


On 2011-01-06 16:13:29, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/398/
> ---
> 
> (Updated 2011-01-06 16:13:29)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> m5: added work completed monitoring support
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py 9f9e10967912 
>   configs/common/Options.py 9f9e10967912 
>   configs/example/fs.py 9f9e10967912 
>   configs/example/ruby_fs.py 9f9e10967912 
>   src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912 
>   src/cpu/base.hh 9f9e10967912 
>   src/cpu/base.cc 9f9e10967912 
>   src/sim/SConscript 9f9e10967912 
>   src/sim/System.py 9f9e10967912 
>   src/sim/pseudo_inst.hh 9f9e10967912 
>   src/sim/pseudo_inst.cc 9f9e10967912 
>   src/sim/system.hh 9f9e10967912 
>   src/sim/system.cc 9f9e10967912 
>   util/m5/m5op_x86.S 9f9e10967912 
>   util/m5/m5ops.h 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/398/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: Assert for x86 misaligned access

[Lisa Hsu]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/390/#review671
---


Suggest using %#x instead of %x, otherwise, good.

- Lisa


On 2011-01-06 16:07:05, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/390/
> ---
> 
> (Updated 2011-01-06 16:07:05)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: Assert for x86 misaligned access
> 
> This patch ensures only aligned access are passed to ruby and includes a fix
> to the DPRINTF address print.
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/system/RubyPort.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/390/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Add checkpointing capability to arch components

[Lisa Hsu]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/386/#review670
---


I don't know about the interrupt device, the but TLB code is good - recently 
duplicated this myself :(.  Not that it was a lot of work, but still :(.

- Lisa


On 2011-01-06 15:59:44, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/386/
> ---
> 
> (Updated 2011-01-06 15:59:44)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Add checkpointing capability to arch components
> 
> Add checkpointing capability to the x86 interrupt device and the TLBs
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.hh 9f9e10967912 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/arch/x86/pagetable.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/386/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: packet latency fix

[Joel Hestness]

> On 2011-01-07 04:32:37, Gabe Black wrote:
> > There are a few problems with this. First, this is an indirect way to get 
> > around the problem. It looks like IntDev can be constructed with no latency 
> > argument which defaults to 0 which is why the call to sendTiming/schedule 
> > would fail. A better fix would be to pass the right latency to the 
> > constructor of IntDev so it has the right value as apposed to passing it in 
> > when needed (ignoring the stored value as well). Second, the latency being 
> > used in interrupts.cc is actually the pio_latency which is the latency for 
> > accessing the memory mapped registers in the local APIC. This is not 
> > necessarily the latency that should be used for sending interrupts. It 
> > looks like the IO APIC has an int_latency parameter, and it would make 
> > sense to put one on the local APIC too. Bringing it all around, that would 
> > then go to the IntDev constructor.

Here, I think you're discussing a bigger problem than this patch is trying to 
fix.  This patch simply passes the latency of the component to the port, which 
schedules the outgoing packet analogously to the SimpleTimingPort as defined in 
tport.*.

Where that latency value actually comes from is a completely different question 
that seemed more straightforward when I originally made the changes.  I think 
you're right that the IntDev (but NOT the IntPort), should have a latency data 
member that is inherited by the descendant classes (Interrupts, I8259 and 
I82094AA - each class has it's own latency member), and that data member should 
be set in their constructors.  I'd have to default to you about whether any of 
these devices should be assessing a different latency for certain types of 
transactions.


- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/383/#review640
---


On 2011-01-06 15:56:44, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/383/
> ---
> 
> (Updated 2011-01-06 15:56:44)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: packet latency fix
> 
> The IntDev should be responsible for setting the latency of packets sent
> through it's port.  This patch fixes the packet scheduling failure when the
> IntPort latency is 0 in timing mode.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/dev/x86/intdev.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/383/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Gabriel Michael Black]

Ok, yeah, looking at it again I think you probably have something. I  
keep mixing up the port and the containing device in my head when I  
think about this. A revised version of my suggestion would be to move  
the latency into IntDev and use it there. Sorry if I propagated any of  
my own confusion.


Gabe

Quoting Joel Hestness :





On 2011-01-07 04:34:28, Gabe Black wrote:
> See review of the earlier IntDev patch. Basically this is  
displacing the latency value from the base class that uses it into  
the subclass that gets it from the config. I don't think it's  
necessary as described previously, but also that decentralizes a  
value that's always used in the same place for the same purpose.


**Note that this patch removes the latency member from IntPort.**   
This patch doesn't indicate where the latency member should end up  
(I'll comment on that in the other review request).  Regardless of  
where the latency is handled, the rest of the codebase indicates  
that a port should not be responsible for assessing latency (see  
mem/port.*, mem/tport.* and mem/mport.*), so this is why I removed  
latency from the IntPort definition.



- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review641
---


On 2011-01-06 15:57:01, Brad Beckmann wrote:


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/
---

(Updated 2011-01-06 15:57:01)


Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,  
and Nathan Binkert.



Summary
---

IntDev: latency fix

Since the device should be responsible for latency of packets, remove the
latency field of the IntPort completely.


Diffs
-

  src/dev/x86/intdev.hh 9f9e10967912

Diff: http://reviews.m5sim.org/r/384/diff


Testing
---


Thanks,

Brad








___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Joel Hestness]

> On 2011-01-07 04:34:28, Gabe Black wrote:
> > See review of the earlier IntDev patch. Basically this is displacing the 
> > latency value from the base class that uses it into the subclass that gets 
> > it from the config. I don't think it's necessary as described previously, 
> > but also that decentralizes a value that's always used in the same place 
> > for the same purpose.

**Note that this patch removes the latency member from IntPort.**  This patch 
doesn't indicate where the latency member should end up (I'll comment on that 
in the other review request).  Regardless of where the latency is handled, the 
rest of the codebase indicates that a port should not be responsible for 
assessing latency (see mem/port.*, mem/tport.* and mem/mport.*), so this is why 
I removed latency from the IntPort definition.


- Joel


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review641
---


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Fix to return cache block size to CPU for split data transfers

[Lisa Hsu]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/393/#review666
---

Ship it!


- Lisa


On 2011-01-06 16:10:36, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/393/
> ---
> 
> (Updated 2011-01-06 16:10:36)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Ruby: Fix to return cache block size to CPU for split data transfers
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/system/RubyPort.hh 9f9e10967912 
>   src/mem/ruby/system/RubyPort.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/393/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: minor fix to deadlock panic message

[Lisa Hsu]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/407/#review665
---

Ship it!


Though I'm with Nate on "code cleanliness" to just use the hash for 0x.

- Lisa


On 2011-01-06 16:19:23, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/407/
> ---
> 
> (Updated 2011-01-06 16:19:23)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: minor fix to deadlock panic message
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/system/Sequencer.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/407/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[Brad Beckmann]

> On 2011-01-07 04:45:16, Gabe Black wrote:
> > I think you forgot some files so this I suppose this is only a partial 
> > review. It looks like this could be cleanly split into three different 
> > changes, and the fact that you have sub-commit messages for those 
> > independent parts suggests that you already knew that. It's best to split 
> > it up and commit it as separate changes.

This is one of Joel's patches, so I don't know for sure, but I believe the 
description is dated.  The pagetable_walker and tlb modifications were moved to 
different patches, so I think Joel came to the same realization as you did.


> On 2011-01-07 04:45:16, Gabe Black wrote:
> > src/arch/x86/vtophys.cc, line 73
> > 
> >
> > This is very suspicious. The request size was set to 0 when you 
> > constructed the request object, so this is anding the original address with 
> > -1. That doesn't do anything, so you're really just oring the addresses 
> > together. The TLB will already have taken care of any page offset/page 
> > number munging that you need. Actually, this whole function is suspect (not 
> > because of your code) since there's no guarantee code/data and/or different 
> > forms of data will be translated the same, or that flags aren't important.

I agree, something seems off here.  However, I'll let Joel respond before 
changing it.  At least there needs to be a comment explaining why this 
calculation is necessary.


- Brad


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/385/#review642
---


On 2011-01-06 15:59:24, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/385/
> ---
> 
> (Updated 2011-01-06 15:59:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: page table walker functional support
> 
> src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table
> src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table
> src/arch/x86/tlb.cc: Added method to return pointer to walker
> src/arch/x86/tlb.hh: Added method to return pointer to walker
> src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping
> 
> 
> Diffs
> -
> 
>   src/arch/x86/vtophys.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/385/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: IntDev: packet latency fix

[Brad Beckmann]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/416/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

IntDev: packet latency fix

The IntDev should be responsible for setting the latency of packets sent
through it's port.  This patch fixes the packet scheduling failure when the
IntPort latency is 0 in timing mode.


Diffs
-

  src/arch/x86/X86LocalApic.py 9f9e10967912 
  src/arch/x86/interrupts.cc 9f9e10967912 

Diff: http://reviews.m5sim.org/r/416/diff


Testing
---


Thanks,

Brad

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Gabe Black]

> On 2011-01-07 12:16:31, Nathan Binkert wrote:
> > src/dev/x86/intdev.hh, line 62
> > 
> >
> > Just because something is not in the style guide doesn't mean it's not 
> > part of the style.  I'd guess that 95% of m5 follows the convention.
> > 
> > Style is hugely about consistency of code.

None of my code does, and that's a lot more than 5%. Grepping around suggests 
that neither does Korey's. I can go through and change it all to be my way if 
all that matters is consistency.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review661
---


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Nathan Binkert]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review661
---



src/dev/x86/intdev.hh


Just because something is not in the style guide doesn't mean it's not part 
of the style.  I'd guess that 95% of m5 follows the convention.

Style is hugely about consistency of code.


- Nathan


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: MessagePort: implemented virtual recvTiming avoiding double delete

[Brad Beckmann]

> On 2011-01-07 04:21:05, Gabe Black wrote:
> > I think there are two problems with this patch. First, if at all possible 
> > we should avoid the code duplication we'd now have for the recvTiming 
> > function. Second, while this probably does fix the legitimate problem of 
> > deleting packets twice, I think it creates a memory leak in the process. I 
> > suspect if you leave your other changes in place but get rid of your custom 
> > recvTiming function, things will still work. The packet won't be deleted by 
> > the device, won't be deleted after being received as a request in either 
> > atomic or timing mode, but will be deleted in both modes after being 
> > received as a response. The "virtual" you added in tport.hh could almost 
> > certainly go away then too.

Joel is the one who actually wrote this patch, so hopefully he can elaborate on 
the possible the memory leak.  I'll hold off on this patch until he can respond.


- Brad


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/#review639
---


On 2011-01-06 15:56:19, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/382/
> ---
> 
> (Updated 2011-01-06 15:56:19)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> MessagePort: implemented virtual recvTiming avoiding double delete
> 
> Double packet delete problem is due to an interrupt device deleting a packet
> that the SimpleTimingPort also deletes. Since MessagePort descends from
> SimpleTimingPort, simply reimplement the failing code from SimpleTimingPort:
> recvTiming.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/mem/mport.hh 9f9e10967912 
>   src/mem/mport.cc 9f9e10967912 
>   src/mem/tport.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/382/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Fwd: Re: Review Request: x86: set IsCondControl flag for the appropriate microops

[Gabe Black]

 Oops, I mean to send this to m5-dev.

Gabe



 Original Message 
Subject:Re: Review Request: x86: set IsCondControl flag for the
appropriate microops
Date:   Fri, 07 Jan 2011 12:04:38 -0800
From:   Gabe Black 
To: Brad Beckmann 



Thank you for fixing this, but in the future please update old review
requests rather than making new ones. That lets us keep track of old
comments and know it was an update and not a whole new thing, and I
think even compare versions. If you're using postreview, use -u to
update, -e to pick the review number (in the URL) and -o just cause I
think you always have to for some reason.

Gabe


On 01/07/11 12:03, Brad Beckmann wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/415/
>
>
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt,
> and Nathan Binkert.
> By Brad Beckmann.
>
>
>   Description
>
> x86: set IsCondControl flag for the appropriate microops
>
>
>   Diffs
>
> * src/arch/x86/isa/microops/regop.isa (9f9e10967912)
> * src/arch/x86/isa/microops/seqop.isa (9f9e10967912)
>
> View Diff 
>


___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Gabe Black]

> On 2011-01-07 12:01:01, Nathan Binkert wrote:
> > src/dev/x86/intdev.hh, line 62
> > 
> >
> > Can you fix the style since you're editing?  The colon should be on the 
> > line below the one it's on.

That's not actually in the style guide unless I missed it just now, and I think 
having it where it is is a lot nicer. It gives you a visual clue what's on the 
next line just like putting operators at the end of lines instead of the 
beginning.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review658
---


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Review Request: x86: set IsCondControl flag for the appropriate microops

[Brad Beckmann]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/415/
---

Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and Nathan 
Binkert.


Summary
---

x86: set IsCondControl flag for the appropriate microops


Diffs
-

  src/arch/x86/isa/microops/regop.isa 9f9e10967912 
  src/arch/x86/isa/microops/seqop.isa 9f9e10967912 

Diff: http://reviews.m5sim.org/r/415/diff


Testing
---


Thanks,

Brad

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Nathan Binkert]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review658
---



src/dev/x86/intdev.hh


Can you fix the style since you're editing?  The colon should be on the 
line below the one it's on.


- Nathan


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

[Nilay Vaish]

Brad, my comments are inline.


On Fri, 7 Jan 2011, Beckmann, Brad wrote:


Hi Nilay,



Unfortunately I can't provide you an example of a protocol where 
getCacheEntry behaves in a different manner, but they do exist.  I 
reviewed your most recent patch updates and I don't think what we're 
asking for is much different than what you have on reviewboard right 
now.  Basically, all we need to do is add back in the capability for the 
programmer to write their own getCacheEntry function in the .sm file. 
I know that I initially asked you to automatically generate those 
functions, and I still think that is useful for most protocols, but Lisa 
made me realize that we need customized getCacheEntry functions as well. 
Also we may want to change the name of generated getCacheEntry function 
to getExclusiveCacheEntry so that one realizes the exclusive assumption 
made by the function.




Other than that, the only other change I suggest is to allow the trigger 
function to directly set the implicit cache_entry and tbe_entry 
variables.  Below is example of what I'm envisioning:




[Nilay] If we do things in this way, then any in_port, in which cache / tb 
entries are accessed before the trigger function, would still make calls 
to isCacheTagPresent().





Currently in MOESI_CMP_directory-L1cache.sm:



in_port(useTimerTable_in, Address, useTimerTable) {

   if (useTimerTable_in.isReady()) {

   set_cache_entry(getCacheEntry(useTimerTable.readyAddress()));

   set_tbe(TBEs[useTimerTable.readyAddress()]);

   trigger(Event:Use_Timeout, useTimerTable.readyAddress());

   }

}



Replace that with the following:



in_port(useTimerTable_in, Address, useTimerTable) {

   if (useTimerTable_in.isReady()) {

   trigger(Event:Use_Timeout, useTimerTable.readyAddress(),

   getExclusiveCacheEntry(useTimerTable.readyAddress()),

  TBEs[useTimerTable.readyAddress()]);

   }

}



[Nilay] Instead of passing cache and tb entries as arguments, we can 
create local variables in the trigger function using the address argument.





Please let me know if you have any questions.



Thanks...you're almost done.  :)



Brad







Thanks
Nilay
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Changing how CacheMemory interfaces with SLICC

[Beckmann, Brad]

Hi Nilay,



Unfortunately I can't provide you an example of a protocol where getCacheEntry 
behaves in a different manner, but they do exist.  I reviewed your most recent 
patch updates and I don't think what we're asking for is much different than 
what you have on reviewboard right now.  Basically, all we need to do is add 
back in the capability for the programmer to write their own getCacheEntry 
function in the .sm file.  I know that I initially asked you to automatically 
generate those functions, and I still think that is useful for most protocols, 
but Lisa made me realize that we need customized getCacheEntry functions as 
well.  Also we may want to change the name of generated getCacheEntry function 
to getExclusiveCacheEntry so that one realizes the exclusive assumption made by 
the function.



Other than that, the only other change I suggest is to allow the trigger 
function to directly set the implicit cache_entry and tbe_entry variables.  
Below is example of what I'm envisioning:



Currently in MOESI_CMP_directory-L1cache.sm:



in_port(useTimerTable_in, Address, useTimerTable) {

if (useTimerTable_in.isReady()) {

set_cache_entry(getCacheEntry(useTimerTable.readyAddress()));

set_tbe(TBEs[useTimerTable.readyAddress()]);

trigger(Event:Use_Timeout, useTimerTable.readyAddress());

}

}



Replace that with the following:



in_port(useTimerTable_in, Address, useTimerTable) {

if (useTimerTable_in.isReady()) {

trigger(Event:Use_Timeout, useTimerTable.readyAddress(),

getExclusiveCacheEntry(useTimerTable.readyAddress()),

   TBEs[useTimerTable.readyAddress()]);

}

}



Please let me know if you have any questions.



Thanks...you're almost done.  :)



Brad





-Original Message-
From: Nilay Vaish [mailto:ni...@cs.wisc.edu]
Sent: Thursday, January 06, 2011 6:32 AM
To: Beckmann, Brad
Cc: Default
Subject: RE: Review Request: Changing how CacheMemory interfaces with SLICC



Can you give me an example of a protocol where getCacheEntry() behaves in a 
different manner?



--

Nilay



On Wed, 5 Jan 2011, Beckmann, Brad wrote:



> Hi Nilay,

>

> Lisa Hsu (another member of the lab here at AMD) and I were discussing

> these changes a bit more and there was one particular idea that came

> out of our conversation that I wanted to relay to you.  Basically, we

> were thinking about how these changes will impact the flexibility of

> SLICC and we concluded that it is important to allow one to craft

> custom getCacheEntry functions for each protocol.  I know initially I

> was hoping to generate these functions, but I now don’t think that

> is possible without restricting what protocols can be support by SLICC.

> Instead we can use these customized getCacheEntry functions to pass

> the cache entry to the actions via the trigger function.  For those

> controllers that manage multiple cache memories, it is up to the

> programmer to understand what the cache entry pointer points to.  That

> should eliminate the need to have multiple *cacheMemory_entry

> variables in the .sm files.  Instead there is just the cache_entry

> variable that is set either by the trigger function call or set_cache_entry.

>

> Does that make sense to you?

>

> Brad

>

>
___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: minor fix to deadlock panic message

[Nathan Binkert]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/407/#review657
---



src/mem/ruby/system/Sequencer.cc


Just so you know, %#x adds the 0x for you.  This is standard printf.


- Nathan


On 2011-01-06 16:19:23, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/407/
> ---
> 
> (Updated 2011-01-06 16:19:23)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: minor fix to deadlock panic message
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/system/Sequencer.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/407/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: mcpat: Adds McPAT performance counters

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/380/#review656
---


If these are adapted from Rick's patches, what was the license on those patches?


src/cpu/BaseCPU.py


Is this supposed to be here?  It doesn't seem related to McPAT.  What does 
it do anyway?  Also, this should be done by adding a couple of new params with 
default '=None' values instead of duplicating code (note that this already 
seems broken wrt ARM and interrupt support when comparing with the preceding 
version).



src/cpu/base.hh


This needs to be a System-object field, not a global, so it works when 
doing multi-system simulations.



src/cpu/base.hh


Same here.



src/cpu/o3/inst_queue_impl.hh


I'm curious why this counts as a read and a write... is the write just the 
update to remove the inst?  Shouldn't that be cheaper than actually writing a 
new instruction into the queue?


- Steve


On 2011-01-06 15:53:31, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/380/
> ---
> 
> (Updated 2011-01-06 15:53:31)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> mcpat: Adds McPAT performance counters
> 
> Updated patches from Rick Strong's set that modify performance counters for
> McPAT
> 
> 
> Diffs
> -
> 
>   src/cpu/BaseCPU.py 9f9e10967912 
>   src/cpu/base.hh 9f9e10967912 
>   src/cpu/base.cc 9f9e10967912 
>   src/cpu/o3/commit.hh 9f9e10967912 
>   src/cpu/o3/commit_impl.hh 9f9e10967912 
>   src/cpu/o3/cpu.hh 9f9e10967912 
>   src/cpu/o3/cpu.cc 9f9e10967912 
>   src/cpu/o3/iew_impl.hh 9f9e10967912 
>   src/cpu/o3/inst_queue.hh 9f9e10967912 
>   src/cpu/o3/inst_queue_impl.hh 9f9e10967912 
>   src/cpu/o3/rename.hh 9f9e10967912 
>   src/cpu/o3/rename_impl.hh 9f9e10967912 
>   src/cpu/o3/rob.hh 9f9e10967912 
>   src/cpu/o3/rob_impl.hh 9f9e10967912 
>   src/cpu/simple/atomic.cc 9f9e10967912 
>   src/cpu/simple/base.hh 9f9e10967912 
>   src/cpu/simple/base.cc 9f9e10967912 
>   src/cpu/simple/timing.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/380/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: Ruby: Update the Ruby request type names for LL/SC

[Steve Reinhardt]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/391/#review655
---



src/mem/ruby/libruby.cc


Does it make sense to update these strings too?  I'd think so, unless 
there's some reason not to.


- Steve


On 2011-01-06 16:07:31, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/391/
> ---
> 
> (Updated 2011-01-06 16:07:31)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> Ruby: Update the Ruby request type names for LL/SC
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/libruby.hh 9f9e10967912 
>   src/mem/ruby/libruby.cc 9f9e10967912 
>   src/mem/ruby/recorder/TraceRecord.cc 9f9e10967912 
>   src/mem/ruby/system/DMASequencer.cc 9f9e10967912 
>   src/mem/ruby/system/RubyPort.cc 9f9e10967912 
>   src/mem/ruby/system/Sequencer.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/391/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: minor fix to deadlock panic message

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/407/#review654
---

Ship it!


- Nilay


On 2011-01-06 16:19:23, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/407/
> ---
> 
> (Updated 2011-01-06 16:19:23)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: minor fix to deadlock panic message
> 
> 
> Diffs
> -
> 
>   src/mem/ruby/system/Sequencer.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/407/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: packet: Added public method to identify valid data

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/399/#review653
---



src/mem/packet.hh


Review board seems to be flagging some whitespace you have at the end of 
this line. The idea of this function seems fine but its name is a little 
awkward. How about hasData()?


- Gabe


On 2011-01-06 16:13:49, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/399/
> ---
> 
> (Updated 2011-01-06 16:13:49)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> packet: Added public method to identify valid data
> 
> 
> Diffs
> -
> 
>   src/mem/packet.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/399/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: m5: added work completed monitoring support

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/398/#review652
---


What are these new pseudo instructions and script options for? I don't remember 
these being mentioned before. You should NOT check in the 
m5.disableAllListeners() line.


configs/common/Options.py


What are these for? Their descriptions aren't very helpful. If they only do 
something for x86 it seems wrong that they're displayed for every ISA.



configs/example/fs.py


This line should -NOT- be checked in.



src/sim/system.hh


Member variables use camelCase, not underscores. What is active_cpus 
tracking? Don't we already have an idea of CPUs being active or quiesced?



src/sim/system.hh


function name goes on the start of the next line.


- Gabe


On 2011-01-06 16:13:29, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/398/
> ---
> 
> (Updated 2011-01-06 16:13:29)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> m5: added work completed monitoring support
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py 9f9e10967912 
>   configs/common/Options.py 9f9e10967912 
>   configs/example/fs.py 9f9e10967912 
>   configs/example/ruby_fs.py 9f9e10967912 
>   src/arch/x86/isa/decoder/two_byte_opcodes.isa 9f9e10967912 
>   src/cpu/base.hh 9f9e10967912 
>   src/cpu/base.cc 9f9e10967912 
>   src/sim/SConscript 9f9e10967912 
>   src/sim/System.py 9f9e10967912 
>   src/sim/pseudo_inst.hh 9f9e10967912 
>   src/sim/pseudo_inst.cc 9f9e10967912 
>   src/sim/system.hh 9f9e10967912 
>   src/sim/system.cc 9f9e10967912 
>   util/m5/m5op_x86.S 9f9e10967912 
>   util/m5/m5ops.h 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/398/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: dev: fixed bugs to extend interrupt capability beyond 15 cores

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/397/#review651
---



src/dev/x86/i82094aa.hh


You don't need "virtual" here.



src/dev/x86/i82094aa.cc


This comment probably could go in /* */s. If you want to stick with //s, 
please get rid of the blank lines that are commented.



src/dev/x86/intdev.hh


I really want one of the folks that knows the memory system better (Steve, 
Ali, Nate) to comment on whether this is right. I'm worried that by only 
broadcasting its address range only at init it won't go out at all sometimes 
(checkpoint restore?) or somebody will miss somebody else's status change.


- Gabe


On 2011-01-06 16:12:56, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/397/
> ---
> 
> (Updated 2011-01-06 16:12:56)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> dev: fixed bugs to extend interrupt capability beyond 15 cores
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/i82094aa.hh 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/dev/x86/intdev.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/397/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Timing support for pagetable walker

[Gabe Black]

> On 2011-01-07 05:51:30, Gabe Black wrote:
> > The code seems ok, but why do we need to have multiple outstanding page 
> > walks in timing mode again?

Actually, I wrote the above before I'd read it carefully. My question still 
stands, but there are some areas that need to be fixed up. Also, since 
translation is very much on the critical path, make sure you measure how much 
this change affects performance. I expect with the addition indirection at 
least there will be some slow down, and we should know what that is before we 
commit anything.


- Gabe


---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/396/#review649
---


On 2011-01-06 16:12:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/396/
> ---
> 
> (Updated 2011-01-06 16:12:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Timing support for pagetable walker
> 
> Move page table walker state to its own object type, and make the
> walker instantiate state for each outstanding walk. By storing the
> states in a queue, the walker is able to handle multiple outstanding
> timing requests. Note that functional walks use separate state
> elements.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/pagetable_walker.hh 9f9e10967912 
>   src/arch/x86/pagetable_walker.cc 9f9e10967912 
>   src/arch/x86/tlb.hh 9f9e10967912 
>   src/arch/x86/tlb.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/396/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Timing support for pagetable walker

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/396/#review649
---


The code seems ok, but why do we need to have multiple outstanding page walks 
in timing mode again?


src/arch/x86/pagetable_walker.hh


Why call this reqType instead of leaving it as mode? requests have types 
which are orthogonal to this, and it's called mode everywhere else.



src/arch/x86/pagetable_walker.cc


These should use FastAlloc if at all possible since they're on a critical 
path and the heap is slow.



src/arch/x86/pagetable_walker.cc


Memory leak.



src/arch/x86/pagetable_walker.cc


Is letting translations pass each other realistic? I worry we're making our 
walker artificially powerful. These loops will also slow things down 
potentially.



src/arch/x86/pagetable_walker.cc


Declare this where it's used.



src/arch/x86/pagetable_walker.cc


Why is this pulled out into its own switch statement? That will slow down 
the code and makes things more complicated.


- Gabe


On 2011-01-06 16:12:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/396/
> ---
> 
> (Updated 2011-01-06 16:12:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Timing support for pagetable walker
> 
> Move page table walker state to its own object type, and make the
> walker instantiate state for each outstanding walk. By storing the
> states in a queue, the walker is able to handle multiple outstanding
> timing requests. Note that functional walks use separate state
> elements.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/pagetable_walker.hh 9f9e10967912 
>   src/arch/x86/pagetable_walker.cc 9f9e10967912 
>   src/arch/x86/tlb.hh 9f9e10967912 
>   src/arch/x86/tlb.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/396/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: TimingSimpleCPU: split data sender state fix

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/395/#review647
---

Ship it!


Looks ok to me.

- Gabe


On 2011-01-06 16:12:04, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/395/
> ---
> 
> (Updated 2011-01-06 16:12:04)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> TimingSimpleCPU: split data sender state fix
> 
> In sendSplitData, keep a pointer to the senderState that may be updated after
> the call to handle*Packet. This way, if the receiver updates the packet
> senderState, it can still be accessed in sendSplitData.
> 
> 
> Diffs
> -
> 
>   src/cpu/simple/timing.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/395/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: ruby: x86 fs config support

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/412/#review646
---



configs/common/FSConfig.py


How is this different from the generic makeX86System (or similarly named) 
function this was copied from? Duplicated code, especially so much of it, is 
bad news. If somebody needed to tweak the MP tables, and forgot to update both 
places, they'd get different behavior depending on if they had Ruby turned on. 
The only ruby-esque line I see is the like about dma_devices, and that's 
commented out. As a side issue that should be deleted, not commented out.



configs/common/FSConfig.py


I don't think everyone that will use this config will have this file, and 
also not everyone that uses Ruby will be using it with parsec.


- Gabe


On 2011-01-06 16:06:33, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/412/
> ---
> 
> (Updated 2011-01-06 16:06:33)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> ruby: x86 fs config support
> 
> 
> Diffs
> -
> 
>   configs/common/FSConfig.py 9f9e10967912 
>   configs/example/ruby_fs.py 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/412/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Add checkpointing capability to devices

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/387/#review645
---



src/dev/intel_8254_timer.cc


This seems to be a bit different since you're fixing existing serialization 
code instead of adding new code, but it's small enough that it's not a huge 
deal to slip it in there. Ideally you'd separate it out, though.



src/dev/x86/i8042.cc


__data is a top secret internal component of the bit union mechanism, hence 
it's double leading underscores. I think if you use the macro bare gcc won't 
cooperate, and if you cast it the name gets messed up. You'll have to declare a 
temporary variable and serialize/unserialize that. It's a pain, but it doesn't 
break encapsulation.



src/dev/x86/i8042.cc


Why aren't you using the SERIALIZE_SCALAR, etc., macros for these? Granted 
macros are gross generally, but using them would be consistent with all the 
other code and fit with what most or all of these lines are doing perfectly.



src/dev/x86/i8042.cc


This stuff that goes with the base PS2Device class should go in serialize 
functions declared there. Then you won't have two copies of it, and it makes 
the code more modular.



src/dev/x86/i8042.cc


Use new and delete.



src/dev/x86/i82094aa.cc


Hmm. Apparently you can cast inside a SERIALIZE_*? If that works that's 
what you should do with the bituions. Cast them to their underlying type (the 
type of __data) and it may all magically work. Or maybe not if it's using a 
reference or something.



src/dev/x86/speaker.cc


See above about bit unions.


- Gabe


On 2011-01-06 16:00:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/387/
> ---
> 
> (Updated 2011-01-06 16:00:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Add checkpointing capability to devices
> 
> Add checkpointing capability to the Intel 8254 timer, CMOS, I8042,
> PS2 Keyboard and Mouse, I82094AA, I8237, I8254, I8259, and speaker
> devices
> 
> 
> Diffs
> -
> 
>   src/dev/intel_8254_timer.cc 9f9e10967912 
>   src/dev/x86/cmos.hh 9f9e10967912 
>   src/dev/x86/cmos.cc 9f9e10967912 
>   src/dev/x86/i8042.hh 9f9e10967912 
>   src/dev/x86/i8042.cc 9f9e10967912 
>   src/dev/x86/i82094aa.hh 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/i8237.hh 9f9e10967912 
>   src/dev/x86/i8237.cc 9f9e10967912 
>   src/dev/x86/i8254.hh 9f9e10967912 
>   src/dev/x86/i8254.cc 9f9e10967912 
>   src/dev/x86/i8259.hh 9f9e10967912 
>   src/dev/x86/i8259.cc 9f9e10967912 
>   src/dev/x86/speaker.hh 9f9e10967912 
>   src/dev/x86/speaker.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/387/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/385/#review644
---


- Gabe


On 2011-01-06 15:59:24, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/385/
> ---
> 
> (Updated 2011-01-06 15:59:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: page table walker functional support
> 
> src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table
> src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table
> src/arch/x86/tlb.cc: Added method to return pointer to walker
> src/arch/x86/tlb.hh: Added method to return pointer to walker
> src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping
> 
> 
> Diffs
> -
> 
>   src/arch/x86/vtophys.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/385/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: Add checkpointing capability to arch components

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/386/#review643
---


I don't know all the ins and outs of checkpointing, but it looks ok to me. You 
aren't serializing all the state in the Interrupts object, but the things you 
left out I think are set up from configuration parameters directly or 
indirectly. Someone else will have to say whether that's ok, but I suspect it 
is.

- Gabe


On 2011-01-06 15:59:44, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/386/
> ---
> 
> (Updated 2011-01-06 15:59:44)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: Add checkpointing capability to arch components
> 
> Add checkpointing capability to the x86 interrupt device and the TLBs
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.hh 9f9e10967912 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/arch/x86/pagetable.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/386/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: page table walker functional support

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/385/#review642
---


I think you forgot some files so this I suppose this is only a partial review. 
It looks like this could be cleanly split into three different changes, and the 
fact that you have sub-commit messages for those independent parts suggests 
that you already knew that. It's best to split it up and commit it as separate 
changes.


src/arch/x86/vtophys.cc


Better wording might be "Need access to page tables."



src/arch/x86/vtophys.cc


Having a temporary variable here seems unnecessary unless it's to prevent 
having to wrap the next line. It's not a big deal, though.



src/arch/x86/vtophys.cc


This is very suspicious. The request size was set to 0 when you constructed 
the request object, so this is anding the original address with -1. That 
doesn't do anything, so you're really just oring the addresses together. The 
TLB will already have taken care of any page offset/page number munging that 
you need. Actually, this whole function is suspect (not because of your code) 
since there's no guarantee code/data and/or different forms of data will be 
translated the same, or that flags aren't important.


- Gabe


On 2011-01-06 15:59:24, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/385/
> ---
> 
> (Updated 2011-01-06 15:59:24)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: page table walker functional support
> 
> src/arch/x86/pagetable_walker.hh: Added method to functionally walk page table
> src/arch/x86/pagetable_walker.cc: Added method to functionally walk page table
> src/arch/x86/tlb.cc: Added method to return pointer to walker
> src/arch/x86/tlb.hh: Added method to return pointer to walker
> src/arch/x86/vtophys.cc: Calls walker to look up virt. to phys. page mapping
> 
> 
> Diffs
> -
> 
>   src/arch/x86/vtophys.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/385/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: latency fix

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/384/#review641
---


See review of the earlier IntDev patch. Basically this is displacing the 
latency value from the base class that uses it into the subclass that gets it 
from the config. I don't think it's necessary as described previously, but also 
that decentralizes a value that's always used in the same place for the same 
purpose.

- Gabe


On 2011-01-06 15:57:01, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/384/
> ---
> 
> (Updated 2011-01-06 15:57:01)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: latency fix
> 
> Since the device should be responsible for latency of packets, remove the
> latency field of the IntPort completely.
> 
> 
> Diffs
> -
> 
>   src/dev/x86/intdev.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/384/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: IntDev: packet latency fix

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/383/#review640
---


There are a few problems with this. First, this is an indirect way to get 
around the problem. It looks like IntDev can be constructed with no latency 
argument which defaults to 0 which is why the call to sendTiming/schedule would 
fail. A better fix would be to pass the right latency to the constructor of 
IntDev so it has the right value as apposed to passing it in when needed 
(ignoring the stored value as well). Second, the latency being used in 
interrupts.cc is actually the pio_latency which is the latency for accessing 
the memory mapped registers in the local APIC. This is not necessarily the 
latency that should be used for sending interrupts. It looks like the IO APIC 
has an int_latency parameter, and it would make sense to put one on the local 
APIC too. Bringing it all around, that would then go to the IntDev constructor.

- Gabe


On 2011-01-06 15:56:44, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/383/
> ---
> 
> (Updated 2011-01-06 15:56:44)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> IntDev: packet latency fix
> 
> The IntDev should be responsible for setting the latency of packets sent
> through it's port.  This patch fixes the packet scheduling failure when the
> IntPort latency is 0 in timing mode.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/i82094aa.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/dev/x86/intdev.cc 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/383/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: MessagePort: implemented virtual recvTiming avoiding double delete

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/382/#review639
---


I think there are two problems with this patch. First, if at all possible we 
should avoid the code duplication we'd now have for the recvTiming function. 
Second, while this probably does fix the legitimate problem of deleting packets 
twice, I think it creates a memory leak in the process. I suspect if you leave 
your other changes in place but get rid of your custom recvTiming function, 
things will still work. The packet won't be deleted by the device, won't be 
deleted after being received as a request in either atomic or timing mode, but 
will be deleted in both modes after being received as a response. The "virtual" 
you added in tport.hh could almost certainly go away then too.


src/mem/mport.cc


I really don't like how much code duplication there is now between these 
two classes.



src/mem/tport.hh


Marking this as explicitly virtual shouldn't really be necessary. Is there 
a reason you want to?


- Gabe


On 2011-01-06 15:56:19, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/382/
> ---
> 
> (Updated 2011-01-06 15:56:19)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> MessagePort: implemented virtual recvTiming avoiding double delete
> 
> Double packet delete problem is due to an interrupt device deleting a packet
> that the SimpleTimingPort also deletes. Since MessagePort descends from
> SimpleTimingPort, simply reimplement the failing code from SimpleTimingPort:
> recvTiming.
> 
> 
> Diffs
> -
> 
>   src/arch/x86/interrupts.cc 9f9e10967912 
>   src/dev/x86/intdev.hh 9f9e10967912 
>   src/mem/mport.hh 9f9e10967912 
>   src/mem/mport.cc 9f9e10967912 
>   src/mem/tport.hh 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/382/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: x86: set IsCondControl flag for the appropriate microops

[Gabe Black]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/377/#review638
---


There's one minor glitch, but otherwise it looks fine.


src/arch/x86/isa/microops/regop.isa


I think you meant to use imm_name here.


- Gabe


On 2011-01-06 15:51:09, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/377/
> ---
> 
> (Updated 2011-01-06 15:51:09)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> x86: set IsCondControl flag for the appropriate microops
> 
> 
> Diffs
> -
> 
>   src/arch/x86/isa/microops/regop.isa 9f9e10967912 
>   src/arch/x86/isa/microops/seqop.isa 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/377/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

Re: [m5-dev] Review Request: MOESI_hammer: trigge queue fix.

[Nilay Vaish]

---
This is an automatically generated e-mail. To reply, visit:
http://reviews.m5sim.org/r/381/#review637
---

Ship it!


- Nilay


On 2011-01-06 15:55:34, Brad Beckmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.m5sim.org/r/381/
> ---
> 
> (Updated 2011-01-06 15:55:34)
> 
> 
> Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and 
> Nathan Binkert.
> 
> 
> Summary
> ---
> 
> MOESI_hammer: trigge queue fix.
> 
> 
> Diffs
> -
> 
>   src/mem/protocol/MOESI_hammer-cache.sm 9f9e10967912 
> 
> Diff: http://reviews.m5sim.org/r/381/diff
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brad
> 
>

___
m5-dev mailing list
m5-dev@m5sim.org
http://m5sim.org/mailman/listinfo/m5-dev

[m5-dev] Cron /z/m5/regression/do-regression quick

[Cron Daemon]

* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/o3-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/linux/inorder-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-atomic passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing passed.
* build/ALPHA_SE/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/01.hello-2T-smt/alpha/linux/o3-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-atomic 
passed.
* build/ALPHA_SE/tests/fast/quick/20.eio-short/alpha/eio/simple-timing 
passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-atomic-mp 
passed.
* build/ALPHA_SE/tests/fast/quick/30.eio-mp/alpha/eio/simple-timing-mp 
passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest passed.
* build/ALPHA_SE/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby 
passed.
* build/ALPHA_SE/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby 
passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MOESI_hammer/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_hammer
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_directory/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_directory
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/linux/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/00.hello/alpha/tru64/simple-timing-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/50.memtest/alpha/linux/memtest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_SE_MOESI_CMP_token/tests/fast/quick/60.rubytest/alpha/linux/rubytest-ruby-MOESI_CMP_token
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic 
passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-atomic-dual
 passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing 
passed.
* 
build/ALPHA_FS/tests/fast/quick/10.linux-boot/alpha/linux/tsunami-simple-timing-dual
 passed.
* 
build/ALPHA_FS/tests/fast/quick/80.netperf-stream/alpha/linux/twosys-tsunami-simple-atomic
 passed.
* build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/inorder-timing passed.
* build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/o3-timing passed.
* build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-atomic passed.
* build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing passed.
* build/MIPS_SE/tests/fast/quick/00.hello/mips/linux/simple-timing-ruby 
passed.
* build/POWER_SE/tests/fast/quick/00.hello/power/linux/simple-atomic passed.
* build/POWER_SE/tests/fast/quick/00.hello/power/linux/o3-timing passed.
* build/SPARC_SE/tests/fast/quick/00.hello/sparc/linux/simple-atomic passed.
* build/SPARC_SE/tests/fast/quick/00.hello/sparc/linux/simple-timing passed.
* build/SPARC_SE/tests/fast/quick/00.hello/sparc/linux/simple-timing-ruby 
passed.
* build/SPARC_SE/tests/fast/quick/02.insttest/sparc/linux/o3-timing passed.
* build/SPARC_SE/tests/fast/quick/02.insttest/sparc/linux/simple-atomic 
passed.
* build/SPARC_SE/tests/fast/quick/02.insttest/sparc/linux/simple-timing 
passed.
* 
build/SPARC_SE/tests/fast/quick/40.m5threads-test-atomic/sparc/linux/simple-atomic-mp
 passed.
* 
build