[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm,arch-arm: correct arm kvm virtual time

2021-03-11 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42161 )


Change subject: cpu-kvm,arch-arm: correct arm kvm virtual time
..

cpu-kvm,arch-arm: correct arm kvm virtual time

According to the kernel code*1, the virtual time is related to physical
timer and cntvoff. When the simulator goes from KVM to gem5, the
physical timer is still ticking. After gem5 simulating models and going
back to KVM, the virtual time also goes away. We should update cntvoff
by ourselve to get correct virtual time.

Moreover, according to update_vtimer_cntvoff*2, setting cntvoff affacts
all vcpus. Instead of puting individual vtime on BaseArmKvmCPU, we
maintain a global vtime, restore it before the first vcpu going into
KVM, and save it after the last vcpu back from KVM.

1. https://code.woboq.org/linux/linux/virt/kvm/arm/arch_timer.c.html#826
2.  
https://code.woboq.org/linux/linux/virt/kvm/arm/arch_timer.c.html#update_vtimer_cntvoff


Change-Id: Ie054104642f2a6d5a0740f39b947f5f2c29c36f3
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42161
Reviewed-by: Earl Ou 
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/kvm/base_cpu.cc
M src/arch/arm/kvm/base_cpu.hh
2 files changed, 39 insertions(+), 0 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  Earl Ou: Looks good to me, but someone else must approve
  kokoro: Regressions pass



diff --git a/src/arch/arm/kvm/base_cpu.cc b/src/arch/arm/kvm/base_cpu.cc
index 2a948c7..ae53095 100644
--- a/src/arch/arm/kvm/base_cpu.cc
+++ b/src/arch/arm/kvm/base_cpu.cc
@@ -38,8 +38,10 @@
 #include "arch/arm/kvm/base_cpu.hh"

 #include 
+#include 

 #include "arch/arm/interrupts.hh"
+#include "base/uncontended_mutex.hh"
 #include "debug/KvmInt.hh"
 #include "dev/arm/generic_timer.hh"
 #include "params/BaseArmKvmCPU.hh"
@@ -58,6 +60,21 @@
 #define INTERRUPT_VCPU_FIQ(vcpu)\
 INTERRUPT_ID(KVM_ARM_IRQ_TYPE_CPU, vcpu, KVM_ARM_IRQ_CPU_FIQ)

+namespace {
+
+/**
+ * When the simulator returns from KVM for simulating other models, the
+ * in-kernel timer doesn't stop. We have to save the virtual time and
+ * restore before going into KVM next time. Moreover, setting virtual time
+ * affacts all vcpus according to the kvm implementation. We maintain a  
global
+ * virtual time here, restore it before the first vcpu going into KVM, and  
save

+ * it after the last vcpu back from KVM.
+ */
+uint64_t vtime = 0;
+uint64_t vtime_counter = 0;
+UncontendedMutex vtime_mutex;
+
+}  // namespace

 BaseArmKvmCPU::BaseArmKvmCPU(const BaseArmKvmCPUParams ¶ms)
 : BaseKvmCPU(params),
@@ -147,6 +164,26 @@
 return kvmRunTicks;
 }

+void
+BaseArmKvmCPU::ioctlRun()
+{
+// Check if it's the first vcpu going into KVM. If yes, it should  
restore

+// the virtual time.
+{
+std::lock_guard l(vtime_mutex);
+if (vtime_counter++ == 0)
+setOneReg(KVM_REG_ARM_TIMER_CNT, vtime);
+}
+BaseKvmCPU::ioctlRun();
+// Check if it's the last vcpu back from KVM. If yes, it should save  
the

+// virtual time.
+{
+std::lock_guard l(vtime_mutex);
+if (--vtime_counter == 0)
+getOneReg(KVM_REG_ARM_TIMER_CNT, &vtime);
+}
+}
+
 const BaseArmKvmCPU::RegIndexVector &
 BaseArmKvmCPU::getRegList() const
 {
diff --git a/src/arch/arm/kvm/base_cpu.hh b/src/arch/arm/kvm/base_cpu.hh
index 6419547..e4bc49f 100644
--- a/src/arch/arm/kvm/base_cpu.hh
+++ b/src/arch/arm/kvm/base_cpu.hh
@@ -56,6 +56,8 @@
   protected:
 Tick kvmRun(Tick ticks) override;

+/** Override for synchronizing state in kvm_run */
+void ioctlRun() override;

 /** Cached state of the IRQ line */
 bool irqAsserted;

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ie054104642f2a6d5a0740f39b947f5f2c29c36f3
Gerrit-Change-Number: 42161
Gerrit-PatchSet: 7
Gerrit-Owner: Yu-hsin Wang 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Earl Ou 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Yu-hsin Wang 
Gerrit-Reviewer: kokoro 
Gerrit-CC: Gabe Black 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: cpu-kvm: refactor x86 fixup before kvm_run

2021-03-11 Thread Yu-hsin Wang (Gerrit) via gem5-dev
Yu-hsin Wang has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42301 )


Change subject: cpu-kvm: refactor x86 fixup before kvm_run
..

cpu-kvm: refactor x86 fixup before kvm_run

Since kvmRun still does lots of thing before really going to KVM, to
make the fixup more precise, I change ioctlRun to a virtual function and
make the derived class overrides it if needed.

Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42301
Tested-by: kokoro 
Reviewed-by: Andreas Sandberg 
Reviewed-by: Jason Lowe-Power 
Maintainer: Andreas Sandberg 
---
M src/cpu/kvm/base.hh
M src/cpu/kvm/x86_cpu.cc
M src/cpu/kvm/x86_cpu.hh
3 files changed, 25 insertions(+), 28 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, but someone else must approve
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/kvm/base.hh b/src/cpu/kvm/base.hh
index d97845b..68eda46 100644
--- a/src/cpu/kvm/base.hh
+++ b/src/cpu/kvm/base.hh
@@ -569,6 +569,8 @@
 }
 /** @} */

+/** Execute the KVM_RUN ioctl */
+virtual void ioctlRun();

 /**
  * KVM memory port.  Uses default RequestPort behavior and provides an
@@ -678,9 +680,6 @@
 /** Try to drain the CPU if a drain is pending */
 bool tryDrain();

-/** Execute the KVM_RUN ioctl */
-void ioctlRun();
-
 /** KVM vCPU file descriptor */
 int vcpuFD;
 /** Size of MMAPed kvm_run area */
diff --git a/src/cpu/kvm/x86_cpu.cc b/src/cpu/kvm/x86_cpu.cc
index 4a7d21b..bae5a29 100644
--- a/src/cpu/kvm/x86_cpu.cc
+++ b/src/cpu/kvm/x86_cpu.cc
@@ -1225,7 +1225,7 @@
 if (_status == Idle)
 return 0;
 else
-return kvmRunWrapper(ticks);
+return BaseKvmCPU::kvmRun(ticks);
 }

 Tick
@@ -1245,33 +1245,14 @@
 // Limit the run to 1 millisecond. That is hopefully enough to
 // reach an interrupt window. Otherwise, we'll just try again
 // later.
-return kvmRunWrapper(1 * SimClock::Float::ms);
+return BaseKvmCPU::kvmRun(1 * SimClock::Float::ms);
 } else {
 DPRINTF(Drain, "kvmRunDrain: Delivering pending IO\n");

-return kvmRunWrapper(0);
+return BaseKvmCPU::kvmRun(0);
 }
 }

-Tick
-X86KvmCPU::kvmRunWrapper(Tick ticks)
-{
-struct kvm_run &kvm_run(*getKvmRunState());
-
-// Synchronize the APIC base and CR8 here since they are present
-// in the kvm_run struct, which makes the synchronization really
-// cheap.
-kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-const Tick run_ticks(BaseKvmCPU::kvmRun(ticks));
-
-tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
-kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
-
-return run_ticks;
-}
-
 uint64_t
 X86KvmCPU::getHostCycles() const
 {
@@ -1404,6 +1385,23 @@
 return !pending_events;
 }

+void
+X86KvmCPU::ioctlRun()
+{
+struct kvm_run &kvm_run(*getKvmRunState());
+
+// Synchronize the APIC base and CR8 here since they are present
+// in the kvm_run struct, which makes the synchronization really
+// cheap.
+kvm_run.apic_base = tc->readMiscReg(MISCREG_APIC_BASE);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+
+BaseKvmCPU::ioctlRun();
+
+tc->setMiscReg(MISCREG_APIC_BASE, kvm_run.apic_base);
+kvm_run.cr8 = tc->readMiscReg(MISCREG_CR8);
+}
+
 static struct kvm_cpuid_entry2
 makeKvmCpuid(uint32_t function, uint32_t index,
  CpuidResult &result)
diff --git a/src/cpu/kvm/x86_cpu.hh b/src/cpu/kvm/x86_cpu.hh
index a60d597..a114a8a 100644
--- a/src/cpu/kvm/x86_cpu.hh
+++ b/src/cpu/kvm/x86_cpu.hh
@@ -78,9 +78,6 @@
  */
 Tick kvmRunDrain() override;

-/** Wrapper that synchronizes state in kvm_run */
-Tick kvmRunWrapper(Tick ticks);
-
 uint64_t getHostCycles() const override;

 /**
@@ -158,6 +155,9 @@
  */
 bool archIsDrained() const override;

+/** Override for synchronizing state in kvm_run */
+void ioctlRun() override;
+
   private:
 /**
  * Support routines to update the state of the KVM CPU from gem5's

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ifd75decf0a5445a5266398caebd8aac1a5e80c50
Gerrit-Change-Number: 42301
Gerrit-PatchSet: 4
Gerrit-Owner: Yu-hsin Wang 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Earl Ou 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Yu-hsin Wang 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe

[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: scons,python: Always generate default create() methods.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42589 )


Change subject: scons,python: Always generate default create() methods.
..

scons,python: Always generate default create() methods.

We were originally generating default create() methods along side the
pybind definitions, but unfortunately those are only included when
python support is included. Since the SimObject Param structs are
unconditionally provided even if the thing calling their create()
methods is not, we need to also unconditionally provide the default
create() definitions. We do that by putting them in their own new .cc
files.

Change-Id: I29d1573d578794b3fe7ec2bc16ef5c8c58e56d0e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42589
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Earl Ou 
---
M src/SConscript
M src/python/m5/SimObject.py
2 files changed, 94 insertions(+), 76 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Earl Ou: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/SConscript b/src/SConscript
index 5fe0ab2..31fce0c 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -917,7 +917,7 @@
 # Create all of the SimObject param headers and enum headers
 #

-def createSimObjectParamStruct(target, source, env):
+def createSimObjectParamDecl(target, source, env):
 assert len(target) == 1 and len(source) == 1

 name = source[0].get_text_contents()
@@ -927,6 +927,16 @@
 obj.cxx_param_decl(code)
 code.write(target[0].abspath)

+def createSimObjectParamDef(target, source, env):
+assert len(target) == 1 and len(source) == 1
+
+name = source[0].get_text_contents()
+obj = sim_objects[name]
+
+code = code_formatter()
+obj.cxx_param_def(code)
+code.write(target[0].abspath)
+
 def createSimObjectCxxConfig(is_header):
 def body(target, source, env):
 assert len(target) == 1 and len(source) == 1
@@ -987,9 +997,16 @@
 hh_file = File('params/%s.hh' % name)
 params_hh_files.append(hh_file)
 env.Command(hh_file, Value(name),
-MakeAction(createSimObjectParamStruct, Transform("SO  
PARAM")))
+MakeAction(createSimObjectParamDecl,  
Transform("SOPARMHH")))

 env.Depends(hh_file, depends + extra_deps)

+if not getattr(simobj, 'abstract', False) and hasattr(simobj, 'type'):
+cc_file = File('params/%s.cc' % name)
+env.Command(cc_file, Value(name),
+MakeAction(createSimObjectParamDef,  
Transform("SOPARMCC")))

+env.Depends(cc_file, depends + extra_deps)
+Source(cc_file)
+
 # C++ parameter description files
 if GetOption('with_cxx_config'):
 for name,simobj in sorted(sim_objects.items()):
diff --git a/src/python/m5/SimObject.py b/src/python/m5/SimObject.py
index e604a20..bdce718 100644
--- a/src/python/m5/SimObject.py
+++ b/src/python/m5/SimObject.py
@@ -368,7 +368,7 @@

 if not is_header:
 code('{')
-if hasattr(simobj, 'abstract') and simobj.abstract:
+if getattr(simobj, 'abstract', False):
 code('return NULL;')
 else:
 code('return this->create();')
@@ -700,6 +700,80 @@
 def pybind_predecls(cls, code):
 code('#include "${{cls.cxx_header}}"')

+def cxx_param_def(cls, code):
+code('''
+#include 
+
+#include "base/compiler.hh"
+
+#include "${{cls.cxx_header}}"
+#include "params/${cls}.hh"
+
+''')
+code()
+code('namespace')
+code('{')
+code()
+# If we can't define a default create() method for this params  
struct

+# because the SimObject doesn't have the right constructor, use
+# template magic to make it so we're actually defining a create  
method

+# for this class instead.
+code('class Dummy${cls}ParamsClass')
+code('{')
+code('  public:')
+code('${{cls.cxx_class}} *create() const;')
+code('};')
+code()
+code('template ')
+code('class Dummy${cls}Shunt;')
+code()
+# This version directs to the real Params struct and the default
+# behavior of create if there's an appropriate constructor.
+code('template ')
+code('class Dummy${cls}Shunt::value>>')
+code('{')
+code('  public:')
+code('using Params = ${cls}Params;')
+code('static ${{cls.cxx_class}} *')
+code('create(const Params &p)')
+code('{')
+code('return new CxxClass(p);')
+code('}')
+code('};')
+code()
+# This version diverts to the DummyParamsClass and a dummy
+# implementation of create if the appropriate constructor does not
+# exist.
+code('template ')
+code('class Dummy${cls}S

[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: cpu: Delete unnecessary create() methods.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42743 )


Change subject: cpu: Delete unnecessary create() methods.
..

cpu: Delete unnecessary create() methods.

These were added in changes which were created before create() methods
were mostly automated, but were checked in after the then unnecessary
create() methods were purged.

Change-Id: I03da797ae8328fab6ef6b85dbc4ea86b34512fd5
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42743
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/cpu/testers/gpu_ruby_test/cpu_thread.cc
M src/cpu/testers/gpu_ruby_test/dma_thread.cc
M src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
M src/cpu/testers/gpu_ruby_test/protocol_tester.cc
4 files changed, 0 insertions(+), 24 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/cpu/testers/gpu_ruby_test/cpu_thread.cc  
b/src/cpu/testers/gpu_ruby_test/cpu_thread.cc

index fa801b3..d0ac10e 100644
--- a/src/cpu/testers/gpu_ruby_test/cpu_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/cpu_thread.cc
@@ -43,12 +43,6 @@
 assert(numLanes == 1);
 }

-CpuThread*
-CpuThreadParams::create() const
-{
-return new CpuThread(*this);
-}
-
 void
 CpuThread::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/dma_thread.cc  
b/src/cpu/testers/gpu_ruby_test/dma_thread.cc

index 254158d..e5f79c9 100644
--- a/src/cpu/testers/gpu_ruby_test/dma_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/dma_thread.cc
@@ -48,12 +48,6 @@

 }

-DmaThread*
-DmaThreadParams::create() const
-{
-return new DmaThread(*this);
-}
-
 void
 DmaThread::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc  
b/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc

index f2f1343..a90b204 100644
--- a/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
+++ b/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
@@ -48,12 +48,6 @@

 }

-GpuWavefront*
-GpuWavefrontParams::create() const
-{
-return new GpuWavefront(*this);
-}
-
 void
 GpuWavefront::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc  
b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc

index a8f8408..95e6035 100644
--- a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc
+++ b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc
@@ -357,9 +357,3 @@

 return true;
 }
-
-ProtocolTester*
-ProtocolTesterParams::create() const
-{
-return new ProtocolTester(*this);
-}

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


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-0
Gerrit-Change-Id: I03da797ae8328fab6ef6b85dbc4ea86b34512fd5
Gerrit-Change-Number: 42743
Gerrit-PatchSet: 2
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Matthew Poremba 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: base: Disable Death test for fast builds

2021-03-11 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42623 )


Change subject: base: Disable Death test for fast builds
..

base: Disable Death test for fast builds

The test would otherwise fail as the expected assertion
is stripped out of the build.

Change-Id: I7c1238b43ee86ad82ad89251754e4c804d8bf16d
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42623
Reviewed-by: Gabe Black 
Reviewed-by: Daniel Carvalho 
Tested-by: kokoro 
---
M src/base/intmath.test.cc
1 file changed, 6 insertions(+), 0 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  Giacomo Travaglini: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/intmath.test.cc b/src/base/intmath.test.cc
index b4bd2a5..3baa502 100644
--- a/src/base/intmath.test.cc
+++ b/src/base/intmath.test.cc
@@ -153,6 +153,12 @@
  */
 TEST(IntmathDeathTest, Log2iDeath)
 {
+
+#ifdef NDEBUG
+GTEST_SKIP() << "Skipping as assertions are "
+"stripped out of fast builds";
+#endif
+
 // 1) value = 0
 EXPECT_DEATH({
 const int value = 0;

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I7c1238b43ee86ad82ad89251754e4c804d8bf16d
Gerrit-Change-Number: 42623
Gerrit-PatchSet: 4
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Add missing transitions + wakes for Dma events

2021-03-11 Thread Matt Sinclair (Gerrit) via gem5-dev
Matt Sinclair has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42463 )


Change subject: mem-ruby: Add missing transitions + wakes for Dma events
..

mem-ruby: Add missing transitions + wakes for Dma events

This also changes one of the wakeUpDependents calls to a
wakeUpAllDependentsAddr call to prevent a hang.

Change-Id: Ia076414e5c6d9c8c0b2576d1f442195d75d275fc
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42463
Reviewed-by: Matt Sinclair 
Reviewed-by: Matthew Poremba 
Maintainer: Matt Sinclair 
Tested-by: kokoro 
---
M src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
1 file changed, 3 insertions(+), 2 deletions(-)

Approvals:
  Matthew Poremba: Looks good to me, approved
  Matt Sinclair: Looks good to me, but someone else must approve; Looks  
good to me, approved

  kokoro: Regressions pass



diff --git a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm  
b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm

index 684d03e..4d24891 100644
--- a/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
+++ b/src/mem/ruby/protocol/MOESI_AMD_Base-dir.sm
@@ -1119,7 +1119,7 @@

   // The exit state is always going to be U, so wakeUpDependents logic  
should be covered in all the

   // transitions which are flowing into U.
-  transition({BL, BS_M, BM_M, B_M, BP, BDW_P, BS_PM, BM_PM, B_PM, BS_Pm,  
BM_Pm, B_Pm, B}, {DmaRead,DmaWrite}){
+  transition({BL, BDR_M, BS_M, BM_M, B_M, BP, BDR_PM, BDW_P, BS_PM, BM_PM,  
B_PM, BDR_Pm, BS_Pm, BM_Pm, B_Pm, B}, {DmaRead,DmaWrite}){

 sd_stallAndWaitRequest;
   }

@@ -1280,6 +1280,7 @@
   transition(BDR_M, MemData, U) {
 mt_writeMemDataToTBE;
 dd_sendResponseDmaData;
+wa_wakeUpAllDependentsAddr;
 dt_deallocateTBE;
 pm_popMemQueue;
   }
@@ -1373,7 +1374,7 @@
 dd_sendResponseDmaData;
 // Check for pending requests from the core we put to sleep while  
waiting

 // for a response
-wa_wakeUpDependents;
+wa_wakeUpAllDependentsAddr;
 dt_deallocateTBE;
 pt_popTriggerQueue;
   }

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ia076414e5c6d9c8c0b2576d1f442195d75d275fc
Gerrit-Change-Number: 42463
Gerrit-PatchSet: 2
Gerrit-Owner: Kyle Roarty 
Gerrit-Reviewer: Alex Dutu 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Matt Sinclair 
Gerrit-Reviewer: Matthew Poremba 
Gerrit-Reviewer: kokoro 
Gerrit-CC: Bobby R. Bruce 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Implement ACPI root tables

2021-03-11 Thread Maximilian Stein (Gerrit) via gem5-dev
Maximilian Stein has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42824 )



Change subject: arch-x86: Implement ACPI root tables
..

arch-x86: Implement ACPI root tables

The RSDP points to the RSDT (32 bit) and/or the XSDT (64 bit), which are
both instances of the abstract System Description Table.
This commit implements the mechanism to write the three data structures
to memory based on the full system's configuration. The SysDescTable
class acts as base class for the RSDT and XSDT as well as any future
implementation of other System Description Tables.

Change-Id: I710279a72376c04f2a636ff2e96fa80228d03eaf
Signed-off-by: Maximilian Stein 
---
M src/arch/x86/SConscript
M src/arch/x86/bios/ACPI.py
M src/arch/x86/bios/acpi.cc
M src/arch/x86/bios/acpi.hh
M src/arch/x86/fs_workload.cc
5 files changed, 239 insertions(+), 34 deletions(-)



diff --git a/src/arch/x86/SConscript b/src/arch/x86/SConscript
index f790ec1..1fcf1dc 100644
--- a/src/arch/x86/SConscript
+++ b/src/arch/x86/SConscript
@@ -81,6 +81,7 @@
   "Page table walker state machine debugging")
 DebugFlag('Decoder', "Decoder debug output")
 DebugFlag('X86', "Generic X86 ISA debugging")
+DebugFlag('ACPI', "ACPI debugging")

 python_files = (
 '__init__.py',
diff --git a/src/arch/x86/bios/ACPI.py b/src/arch/x86/bios/ACPI.py
index 77de42f..19edac4 100644
--- a/src/arch/x86/bios/ACPI.py
+++ b/src/arch/x86/bios/ACPI.py
@@ -48,8 +48,8 @@
 oem_table_id = Param.String('', 'oem table ID')
 oem_revision = Param.UInt32(0, 'oem revision number for the table')

-creator_id = Param.String('',
-'string identifying the generator of the table')
+creator_id = Param.UInt32(0,
+'ID identifying the generator of the table')
 creator_revision = Param.UInt32(0,
 'revision number for the creator of the table')

@@ -78,6 +78,7 @@
 # here.
 revision = Param.UInt8(2, 'revision of ACPI being used, zero indexed')

-rsdt = Param.X86ACPIRSDT(NULL, 'root system description table')
+rsdt = Param.X86ACPIRSDT(X86ACPIRSDT(),
+'root system description table')
 xsdt = Param.X86ACPIXSDT(X86ACPIXSDT(),
 'extended system description table')
diff --git a/src/arch/x86/bios/acpi.cc b/src/arch/x86/bios/acpi.cc
index 8cdcdac..122ad31 100644
--- a/src/arch/x86/bios/acpi.cc
+++ b/src/arch/x86/bios/acpi.cc
@@ -37,6 +37,10 @@

 #include "arch/x86/bios/acpi.hh"

+#include 
+#include 
+
+#include "base/trace.hh"
 #include "mem/port.hh"
 #include "params/X86ACPIRSDP.hh"
 #include "params/X86ACPIRSDT.hh"
@@ -45,8 +49,25 @@
 #include "sim/byteswap.hh"
 #include "sim/sim_object.hh"

+namespace X86ISA
+{
+
+namespace ACPI
+{
+
+const char RSDP::signature[] = "RSD PTR ";
+
+static uint8_t
+apic_checksum(uint8_t* ptr, std::size_t size)
+{
+uint8_t sum = 0;
+for (unsigned i = 0; i < size; ++i)
+sum += ptr[i];
+return 0x100 - sum;
+}
+
 Addr
-X86ISA::ACPI::LinearAllocator::alloc(std::size_t size, unsigned align)
+LinearAllocator::alloc(std::size_t size, unsigned align)
 {
 if (align) {
 unsigned offset = next % align;
@@ -59,24 +80,130 @@
 return chunk;
 }

-const char X86ISA::ACPI::RSDP::signature[] = "RSD PTR ";
+RSDP::RSDP(const Params &p) :
+SimObject(p),
+rsdt(p.rsdt),
+xsdt(p.xsdt)
+{
+static_assert(sizeof(signature) - 1 == sizeof(d.Signature),
+"signature length mismatch");
+std::memcpy(d.Signature, signature, sizeof(d.Signature));
+std::strncpy(d.OEMID, p.oem_id.c_str(), sizeof(d.OEMID));
+d.Revision = p.revision;
+d.Length = sizeof(d);
+}

-X86ISA::ACPI::RSDP::RSDP(const Params &p) : SimObject(p), oemID(p.oem_id),
-revision(p.revision), rsdt(p.rsdt), xsdt(p.xsdt)
+Addr
+RSDP::write(PortProxy& physProxy, Allocator& alloc) const
+{
+std::vector mem(sizeof(d));
+Addr addr = alloc.alloc(sizeof(d), 16);
+
+assert(mem.size() >= sizeof(d));
+std::memcpy(mem.data(), &d, sizeof(d));
+Mem* desc = (Mem*)mem.data();
+
+if (rsdt) {
+desc->RsdtAddress = rsdt->write(physProxy, alloc);
+DPRINTF(ACPI, "Allocated RSDT @ %llx\n", desc->RsdtAddress);
+}
+if (xsdt) {
+desc->XsdtAddress = xsdt->write(physProxy, alloc);
+DPRINTF(ACPI, "Allocated XSDT @ %llx\n", desc->XsdtAddress);
+}
+
+// checksum calculation
+assert(d.Length == sizeof(d));
+desc->Checksum = apic_checksum(mem.data(), sizeof(MemR0));
+desc->ExtendedChecksum = apic_checksum(mem.data(), d.Length);
+
+// write the whole thing
+physProxy.writeBlob(addr, mem.data(), mem.size());
+
+return addr;
+}
+
+SysDescTable::Mem::Mem(const char* Signature, uint8_t Revision,
+const Params& p) :
+Revision(Revision)
+{
+std::strncpy(this->Signature, Signature, sizeof(this->Signature));
+std::strncpy(OEMID, p.oem_id.c_str(), sizeof(OEMID));
+std::strncpy(O

[gem5-dev] Change in gem5/gem5[develop]: configs: Use MADT in x86 full system simulation

2021-03-11 Thread Maximilian Stein (Gerrit) via gem5-dev
Maximilian Stein has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42825 )



Change subject: configs: Use MADT in x86 full system simulation
..

configs: Use MADT in x86 full system simulation

ACPI's MADT describes the interrupt system of a processor/system and
partially replaces the Intel MP tables. The config now simply adds the
ACPI variant, so an OS can use either Intel MP or ACPI for interrupt
configuration.

Change-Id: Ie3d293aac0925666f7661a03eab10218f04c8d0c
Signed-off-by: Maximilian Stein 
---
M configs/common/FSConfig.py
1 file changed, 30 insertions(+), 0 deletions(-)



diff --git a/configs/common/FSConfig.py b/configs/common/FSConfig.py
index 6665225..98df117 100644
--- a/configs/common/FSConfig.py
+++ b/configs/common/FSConfig.py
@@ -501,6 +501,7 @@
 # Set up the Intel MP table
 base_entries = []
 ext_entries = []
+madt_records = []
 for i in range(numCPUs):
 bp = X86IntelMPProcessor(
 local_apic_id = i,
@@ -508,6 +509,11 @@
 enable = True,
 bootstrap = (i == 0))
 base_entries.append(bp)
+lapic = X86ACPIMadtLAPIC(
+acpi_processor_id=i,
+apic_id=i,
+flags=1)
+madt_records.append(lapic)
 io_apic = X86IntelMPIOAPIC(
 id = numCPUs,
 version = 0x11,
@@ -515,6 +521,8 @@
 address = 0xfec0)
 self.pc.south_bridge.io_apic.apic_id = io_apic.id
 base_entries.append(io_apic)
+madt_records.append(X86ACPIMadtIOAPIC(id=io_apic.id,
+address=io_apic.address, int_base=0))
 # In gem5 Pc::calcPciConfigAddr(), it required "assert(bus==0)",
 # but linux kernel cannot config PCI device if it was not connected to
 # PCI bus, so we fix PCI bus id to 0, and ISA bus id to 1.
@@ -534,6 +542,13 @@
 dest_io_apic_id = io_apic.id,
 dest_io_apic_intin = 16)
 base_entries.append(pci_dev4_inta)
+pci_dev4_inta_madt = X86ACPIMadtIntSourceOverride(
+bus_source = pci_dev4_inta.source_bus_id,
+irq_source = pci_dev4_inta.source_bus_irq,
+sys_int = pci_dev4_inta.dest_io_apic_intin,
+flags = 0
+)
+madt_records.append(pci_dev4_inta_madt)
 def assignISAInt(irq, apicPin):
 assign_8259_to_apic = X86IntelMPIOIntAssignment(
 interrupt_type = 'ExtInt',
@@ -553,6 +568,14 @@
 dest_io_apic_id = io_apic.id,
 dest_io_apic_intin = apicPin)
 base_entries.append(assign_to_apic)
+# acpi
+assign_to_apic_acpi = X86ACPIMadtIntSourceOverride(
+bus_source = 1,
+irq_source = irq,
+sys_int = apicPin,
+flags = 0
+)
+madt_records.append(assign_to_apic_acpi)
 assignISAInt(0, 2)
 assignISAInt(1, 1)
 for i in range(3, 15):
@@ -560,6 +583,13 @@
 workload.intel_mp_table.base_entries = base_entries
 workload.intel_mp_table.ext_entries = ext_entries

+madt = X86ACPIMadt(local_apic_address=0,
+records=madt_records, oem_id='madt')
+workload.acpi_description_table_pointer.rsdt.entries.append(madt)
+workload.acpi_description_table_pointer.xsdt.entries.append(madt)
+workload.acpi_description_table_pointer.oem_id = 'gem5'
+workload.acpi_description_table_pointer.rsdt.oem_id='gem5'
+workload.acpi_description_table_pointer.xsdt.oem_id='gem5'
 return self

 def makeLinuxX86System(mem_mode, numCPUs=1, mdesc=None, Ruby=False,

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ie3d293aac0925666f7661a03eab10218f04c8d0c
Gerrit-Change-Number: 42825
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Stein 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-x86: Add allocator for ACPI tables

2021-03-11 Thread Maximilian Stein (Gerrit) via gem5-dev
Maximilian Stein has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42823 )



Change subject: arch-x86: Add allocator for ACPI tables
..

arch-x86: Add allocator for ACPI tables

This adds an allocator class to allocate memory linearly. It is intended
to be used by ACPI tables to dynamically request memory to write the
ACPI tables to.

Change-Id: I43c71d2b8e676f8ac0fd08b9468b00b6212d85b6
Signed-off-by: Maximilian Stein 
---
M src/arch/x86/bios/acpi.cc
M src/arch/x86/bios/acpi.hh
2 files changed, 32 insertions(+), 0 deletions(-)



diff --git a/src/arch/x86/bios/acpi.cc b/src/arch/x86/bios/acpi.cc
index 20cf088..8cdcdac 100644
--- a/src/arch/x86/bios/acpi.cc
+++ b/src/arch/x86/bios/acpi.cc
@@ -45,6 +45,20 @@
 #include "sim/byteswap.hh"
 #include "sim/sim_object.hh"

+Addr
+X86ISA::ACPI::LinearAllocator::alloc(std::size_t size, unsigned align)
+{
+if (align) {
+unsigned offset = next % align;
+if (offset)
+next += (align - offset);
+}
+Addr chunk = next;
+next += size;
+assert(0 == end || next <= end);
+return chunk;
+}
+
 const char X86ISA::ACPI::RSDP::signature[] = "RSD PTR ";

 X86ISA::ACPI::RSDP::RSDP(const Params &p) : SimObject(p), oemID(p.oem_id),
diff --git a/src/arch/x86/bios/acpi.hh b/src/arch/x86/bios/acpi.hh
index bc6e2cd..e314ab8 100644
--- a/src/arch/x86/bios/acpi.hh
+++ b/src/arch/x86/bios/acpi.hh
@@ -62,6 +62,24 @@
 class XSDT;
 class SysDescTable;

+struct Allocator
+{
+virtual Addr alloc(std::size_t size, unsigned align = 1) = 0;
+};
+struct LinearAllocator : public Allocator
+{
+LinearAllocator(Addr begin, Addr end = 0)
+: next(begin)
+, end(end)
+{}
+
+Addr alloc(std::size_t size, unsigned align) override;
+
+  protected:
+Addr next;
+Addr const end;
+};
+
 class RSDP : public SimObject
 {
   protected:

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I43c71d2b8e676f8ac0fd08b9468b00b6212d85b6
Gerrit-Change-Number: 42823
Gerrit-PatchSet: 1
Gerrit-Owner: Maximilian Stein 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: base: Stop using testing::internal:: in tests.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42723 )


Change subject: base: Stop using testing::internal:: in tests.
..

base: Stop using testing::internal:: in tests.

The name strongly suggests that this namespace isn't for our use.
Instead, use the new gtestLogOutput string stream, or in the debug test
add an optional parameter to debugDumpFlags which lets us direct the
output to a string stream we can inspect.

Change-Id: I51d3c0ec42981b70736e7b3bbedfb49f82dc7f95
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42723
Reviewed-by: Gabe Black 
Reviewed-by: Daniel Carvalho 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/debug.cc
M src/base/debug.hh
M src/base/debug.test.cc
M src/base/socket.test.cc
M src/base/stats/storage.test.cc
5 files changed, 18 insertions(+), 32 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, approved
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/debug.cc b/src/base/debug.cc
index 925c16f..38eb25f 100644
--- a/src/base/debug.cc
+++ b/src/base/debug.cc
@@ -181,7 +181,7 @@
 }

 void
-dumpDebugFlags()
+dumpDebugFlags(std::ostream &os)
 {
 using namespace Debug;
 FlagsMap::iterator i = allFlags().begin();
@@ -189,6 +189,6 @@
 for (; i != end; ++i) {
 SimpleFlag *f = dynamic_cast(i->second);
 if (f && f->enabled())
-cprintf("%s\n", f->name());
+ccprintf(os, "%s\n", f->name());
 }
 }
diff --git a/src/base/debug.hh b/src/base/debug.hh
index be5fa36..38d92f9 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -43,6 +43,7 @@
 #define __BASE_DEBUG_HH__

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -142,7 +143,7 @@

 void clearDebugFlag(const char *string);

-void dumpDebugFlags();
+void dumpDebugFlags(std::ostream &os=std::cout);

 /**
  * \def DTRACE(x)
diff --git a/src/base/debug.test.cc b/src/base/debug.test.cc
index f995a33..22320d1 100644
--- a/src/base/debug.test.cc
+++ b/src/base/debug.test.cc
@@ -29,6 +29,7 @@
 #include 

 #include "base/debug.hh"
+#include "base/gtest/logging.hh"

 /** Test assignment of names and descriptions. */
 TEST(DebugFlagTest, NameDesc)
@@ -51,12 +52,11 @@
 TEST(DebugFlagDeathTest, UniqueNames)
 {
 Debug::SimpleFlag flag("FlagUniqueNamesTest", "A");
-testing::internal::CaptureStderr();
+gtestLogOutput.str("");
 EXPECT_ANY_THROW(Debug::SimpleFlag("FlagUniqueNamesTest", "B"));
 const std::string expected = "panic: panic condition !result.second "
 "occurred: Flag FlagUniqueNamesTest already defined!\n";
-std::string actual = testing::internal::GetCapturedStderr().substr();
-actual = actual.substr(actual.find(":", actual.find(":") + 1) + 2);
+std::string actual = gtestLogOutput.str();
 EXPECT_EQ(expected, actual);
 }

@@ -266,9 +266,9 @@
 Debug::SimpleFlag flag("FlagDumpDebugFlagTest", "");

 // Verify that the names of the enabled flags are printed
-testing::internal::CaptureStdout();
+gtestLogOutput.str("");
 dumpDebugFlags();
-std::string output = testing::internal::GetCapturedStdout();
+std::string output = gtestLogOutput.str();
 EXPECT_EQ(output, "");
 ASSERT_FALSE(flag.enabled());
 }
@@ -298,9 +298,9 @@
 compound_flag_b.enable();

 // Verify that the names of the enabled flags are printed
-testing::internal::CaptureStdout();
-dumpDebugFlags();
-std::string output = testing::internal::GetCapturedStdout();
+std::ostringstream os;
+dumpDebugFlags(os);
+std::string output = os.str();
 EXPECT_EQ(output, "FlagDumpDebugFlagTestA\nFlagDumpDebugFlagTestC\n" \
 "FlagDumpDebugFlagTestE\n");
 }
diff --git a/src/base/socket.test.cc b/src/base/socket.test.cc
index 7262a02..7372911 100644
--- a/src/base/socket.test.cc
+++ b/src/base/socket.test.cc
@@ -28,6 +28,7 @@

 #include 

+#include "base/gtest/logging.hh"
 #include "base/socket.hh"

 #define TEST_PORT_1 7893
@@ -103,20 +104,10 @@
 /*
  * You cannot listen to another port if you are already listening to  
one.

  */
-testing::internal::CaptureStderr();
+gtestLogOutput.str("");
 EXPECT_ANY_THROW(listen_socket.listen(TEST_PORT_1));
 std::string expected = "panic: Socket already listening!\n";
-std::string actual = testing::internal::GetCapturedStderr().substr();
-
-/*
- * The GoogleExitLogger will output using the following:
- * `std::cerr << loc.file << ":" << loc.line << ": " << s;`
- * As we do not care about the file and line where the error originated
- * (this may change, and it shouldn't break the test when this  
happens),
- * we strip out the leading `:: ` (we simply remove  
everything

- * prior to two characters after the second colon in the string).
- */
-actual = actual.substr(actual.find(":", actual.find(":") 

[gem5-dev] Change in gem5/gem5[stable]: python: Fix incorrect prefixes is m5.utils.convert

2021-03-11 Thread Andreas Sandberg (Gerrit) via gem5-dev
Andreas Sandberg has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42783 )



Change subject: python: Fix incorrect prefixes is m5.utils.convert
..

python: Fix incorrect prefixes is m5.utils.convert

The conversion functions incorrectly assumed that kibibytes are 'kiB'
rather than 'KiB' (correct).

Change-Id: I7ef9e54546fdb3379435b40af6d9f619ad9b37a5
Signed-off-by: Andreas Sandberg 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/39375
Reviewed-by: Daniel Carvalho 
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
(cherry picked from commit b67b917345e85d6c02aa8c37dc40524eac5622c6)
---
M src/python/m5/util/convert.py
1 file changed, 2 insertions(+), 2 deletions(-)



diff --git a/src/python/m5/util/convert.py b/src/python/m5/util/convert.py
index 077b6b4..ae667b3 100644
--- a/src/python/m5/util/convert.py
+++ b/src/python/m5/util/convert.py
@@ -62,7 +62,7 @@
 'Gi': gibi,
 'G': giga,
 'M': mega,
-'ki': kibi,
+'Ki': kibi,
 'k': kilo,
 'Mi': mebi,
 'm': milli,
@@ -84,7 +84,7 @@
 'G' : gibi,
 'Mi': mebi,
 'M' : mebi,
-'ki': kibi,
+'Ki': kibi,
 'k' : kibi,
 }


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


Gerrit-Project: public/gem5
Gerrit-Branch: stable
Gerrit-Change-Id: I7ef9e54546fdb3379435b40af6d9f619ad9b37a5
Gerrit-Change-Number: 42783
Gerrit-PatchSet: 1
Gerrit-Owner: Andreas Sandberg 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch: Make some internal decode methods protected.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40102 )


Change subject: arch: Make some internal decode methods protected.
..

arch: Make some internal decode methods protected.

These methods aren't used outside of the decoder and the decode cache,
so they don't need to be public.

Change-Id: Ifdaf318995f1bb0a75b390bd1c5fde1211c66e62
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40102
Tested-by: kokoro 
Reviewed-by: Daniel Carvalho 
Reviewed-by: Bobby R. Bruce 
Maintainer: Bobby R. Bruce 
---
M src/arch/arm/decoder.hh
M src/arch/mips/decoder.hh
M src/arch/power/decoder.hh
M src/arch/riscv/decoder.hh
M src/arch/sparc/decoder.hh
M src/arch/x86/decoder.hh
6 files changed, 53 insertions(+), 48 deletions(-)

Approvals:
  Daniel Carvalho: Looks good to me, but someone else must approve
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/decoder.hh b/src/arch/arm/decoder.hh
index 1f14328..53e3f76 100644
--- a/src/arch/arm/decoder.hh
+++ b/src/arch/arm/decoder.hh
@@ -82,6 +82,7 @@

 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

 /**
  * Pre-decode an instruction from the current state of the
@@ -95,6 +96,38 @@
  */
 void consumeBytes(int numBytes);

+/**
+ * Decode a machine instruction without calling the cache.
+ *
+ * @note The implementation of this method is generated by the ISA
+ * parser script.
+ *
+ * @warn This method takes a pre-decoded instruction as its
+ * argument. It should typically not be called directly.
+ *
+ * @param mach_inst The binary instruction to decode.
+ * @retval A pointer to the corresponding StaticInst object.
+ */
+StaticInstPtr decodeInst(ExtMachInst mach_inst);
+
+/**
+ * Decode a pre-decoded machine instruction.
+ *
+ * @warn This method takes a pre-decoded instruction as its
+ * argument. It should typically not be called directly.
+ *
+ * @param mach_inst A pre-decoded instruction
+ * @retval A pointer to the corresponding StaticInst object.
+ */
+StaticInstPtr
+decode(ExtMachInst mach_inst, Addr addr)
+{
+StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
+DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
+si->getName(), mach_inst);
+return si;
+}
+
   public: // Decoder API
 Decoder(ISA* isa = nullptr);

@@ -162,38 +195,6 @@
 StaticInstPtr decode(ArmISA::PCState &pc);

 /**
- * Decode a pre-decoded machine instruction.
- *
- * @warn This method takes a pre-decoded instruction as its
- * argument. It should typically not be called directly.
- *
- * @param mach_inst A pre-decoded instruction
- * @retval A pointer to the corresponding StaticInst object.
- */
-StaticInstPtr
-decode(ExtMachInst mach_inst, Addr addr)
-{
-StaticInstPtr si = defaultCache.decode(this, mach_inst, addr);
-DPRINTF(Decode, "Decode: Decoded %s instruction: %#x\n",
-si->getName(), mach_inst);
-return si;
-}
-
-/**
- * Decode a machine instruction without calling the cache.
- *
- * @note The implementation of this method is generated by the ISA
- * parser script.
- *
- * @warn This method takes a pre-decoded instruction as its
- * argument. It should typically not be called directly.
- *
- * @param mach_inst The binary instruction to decode.
- * @retval A pointer to the corresponding StaticInst object.
- */
-StaticInstPtr decodeInst(ExtMachInst mach_inst);
-
-/**
  * Take over the state from an old decoder when switching CPUs.
  *
  * @param old Decoder used in old CPU
diff --git a/src/arch/mips/decoder.hh b/src/arch/mips/decoder.hh
index 6e00bc3..969c152 100644
--- a/src/arch/mips/decoder.hh
+++ b/src/arch/mips/decoder.hh
@@ -89,8 +89,8 @@
   protected:
 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

-  public:
 StaticInstPtr decodeInst(ExtMachInst mach_inst);

 /// Decode a machine instruction.
@@ -105,6 +105,7 @@
 return si;
 }

+  public:
 StaticInstPtr
 decode(MipsISA::PCState &nextPC)
 {
diff --git a/src/arch/power/decoder.hh b/src/arch/power/decoder.hh
index 4e02ef7..bd32614 100644
--- a/src/arch/power/decoder.hh
+++ b/src/arch/power/decoder.hh
@@ -96,8 +96,8 @@
   protected:
 /// A cache of decoded instruction objects.
 static GenericISA::BasicDecodeCache defaultCache;
+friend class GenericISA::BasicDecodeCache;

-  public:
 StaticInstPtr decodeInst(ExtMachInst mach_inst);

 /// Decode a ma

[gem5-dev] Change in gem5/gem5[develop]: arch,cpu: Move machInst into the arch specific StaticInst classes.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/40101 )


Change subject: arch,cpu: Move machInst into the arch specific StaticInst  
classes.

..

arch,cpu: Move machInst into the arch specific StaticInst classes.

This type is ISA specific. By moving it into the subclasses, it's still
available to everybody that needs it but avoids that ISA dependence in
the base StaticInst class.

Change-Id: I87ac4c6eded42287ef9ebaa4c4a5738482a2fc13
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/40101
Reviewed-by: Giacomo Travaglini 
Reviewed-by: Daniel Carvalho 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/insts/static_inst.hh
M src/arch/mips/isa/base.isa
M src/arch/power/insts/static_inst.hh
M src/arch/riscv/faults.cc
M src/arch/riscv/insts/static_inst.hh
M src/arch/sparc/insts/static_inst.hh
M src/arch/x86/faults.cc
M src/arch/x86/insts/static_inst.hh
M src/cpu/static_inst.cc
M src/cpu/static_inst.hh
10 files changed, 39 insertions(+), 27 deletions(-)

Approvals:
  Giacomo Travaglini: Looks good to me, approved; Looks good to me, approved
  Daniel Carvalho: Looks good to me, but someone else must approve
  kokoro: Regressions pass



diff --git a/src/arch/arm/insts/static_inst.hh  
b/src/arch/arm/insts/static_inst.hh

index 896523e..b78f64a 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -143,10 +143,12 @@
 }
 }

+ExtMachInst machInst;
+
 // Constructor
 ArmStaticInst(const char *mnem, ExtMachInst _machInst,
   OpClass __opClass)
-: StaticInst(mnem, _machInst, __opClass)
+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 aarch64 = machInst.aarch64;
 if (bits(machInst, 28, 24) == 0x10)
diff --git a/src/arch/mips/isa/base.isa b/src/arch/mips/isa/base.isa
index cdd950c..0175bcc 100644
--- a/src/arch/mips/isa/base.isa
+++ b/src/arch/mips/isa/base.isa
@@ -42,10 +42,9 @@
 class MipsStaticInst : public StaticInst
 {
   protected:
-
 // Constructor
 MipsStaticInst(const char *mnem, MachInst _machInst, OpClass  
__opClass)

-: StaticInst(mnem, _machInst, __opClass)
+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 }

@@ -57,6 +56,8 @@
 Addr pc, const Loader::SymbolTable *symtab) const override;

   public:
+ExtMachInst machInst;
+
 void
 advancePC(MipsISA::PCState &pc) const override
 {
diff --git a/src/arch/power/insts/static_inst.hh  
b/src/arch/power/insts/static_inst.hh

index dc53bc1..403d358 100644
--- a/src/arch/power/insts/static_inst.hh
+++ b/src/arch/power/insts/static_inst.hh
@@ -38,10 +38,11 @@
 class PowerStaticInst : public StaticInst
 {
   protected:
+ExtMachInst machInst;

 // Constructor
-PowerStaticInst(const char *mnem, MachInst _machInst, OpClass  
__opClass)

-: StaticInst(mnem, _machInst, __opClass)
+PowerStaticInst(const char *mnem, ExtMachInst _machInst, OpClass  
__opClass)

+: StaticInst(mnem, __opClass), machInst(_machInst)
 {
 }

diff --git a/src/arch/riscv/faults.cc b/src/arch/riscv/faults.cc
index 5ac2a3c..8c273f2 100644
--- a/src/arch/riscv/faults.cc
+++ b/src/arch/riscv/faults.cc
@@ -32,6 +32,7 @@
 #include "arch/riscv/faults.hh"

 #include "arch/riscv/fs_workload.hh"
+#include "arch/riscv/insts/static_inst.hh"
 #include "arch/riscv/isa.hh"
 #include "arch/riscv/registers.hh"
 #include "arch/riscv/utility.hh"
@@ -163,14 +164,16 @@
 void
 UnknownInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 {
-panic("Unknown instruction 0x%08x at pc 0x%016llx", inst->machInst,
+auto *rsi = static_cast(inst.get());
+panic("Unknown instruction 0x%08x at pc 0x%016llx", rsi->machInst,
 tc->pcState().pc());
 }

 void
 IllegalInstFault::invokeSE(ThreadContext *tc, const StaticInstPtr &inst)
 {
-panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", inst->machInst,
+auto *rsi = static_cast(inst.get());
+panic("Illegal instruction 0x%08x at pc 0x%016llx: %s", rsi->machInst,
 tc->pcState().pc(), reason.c_str());
 }

diff --git a/src/arch/riscv/insts/static_inst.hh  
b/src/arch/riscv/insts/static_inst.hh

index 149e847..1c57cc7 100644
--- a/src/arch/riscv/insts/static_inst.hh
+++ b/src/arch/riscv/insts/static_inst.hh
@@ -46,9 +46,14 @@
 class RiscvStaticInst : public StaticInst
 {
   protected:
-using StaticInst::StaticInst;
+RiscvStaticInst(const char *_mnemonic, ExtMachInst _machInst,
+OpClass __opClass) :
+StaticInst(_mnemonic, __opClass), machInst(_machInst)
+{}

   public:
+ExtMachInst machInst;
+
 void advancePC(PCState &pc) const override { pc.advance(); }

 size_t
diff --git a/src/arch/sparc/insts/static_inst.hh  
b/src/arch/sparc/insts/static_inst.hh

index 0237c98..43d

[gem5-dev] Change in gem5/gem5[develop]: base: Add fatal tests to sat_counter

2021-03-11 Thread Daniel Carvalho (Gerrit) via gem5-dev
Daniel Carvalho has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42763 )



Change subject: base: Add fatal tests to sat_counter
..

base: Add fatal tests to sat_counter

Add tests to make sure that the fatal conditions are
triggered when met.

This commit also moves shift-related death tests to
their own DeathTests.

Change-Id: Ia610636fe8cf636401e2b2ed623bf20b41147ea4
Signed-off-by: Daniel R. Carvalho 
---
M src/base/sat_counter.hh
M src/base/sat_counter.test.cc
2 files changed, 63 insertions(+), 7 deletions(-)



diff --git a/src/base/sat_counter.hh b/src/base/sat_counter.hh
index 4849c2a..cf60fdc 100644
--- a/src/base/sat_counter.hh
+++ b/src/base/sat_counter.hh
@@ -78,7 +78,7 @@
 fatal_if(bits > 8*sizeof(T),
  "Number of bits exceeds counter size");
 fatal_if(initial_val > maxVal,
- "Saturating counter's Initial value exceeds max value.");
+ "Saturating counter's initial value exceeds max value.");
 }

 /**
diff --git a/src/base/sat_counter.test.cc b/src/base/sat_counter.test.cc
index 19f792e..02b6a87 100644
--- a/src/base/sat_counter.test.cc
+++ b/src/base/sat_counter.test.cc
@@ -31,9 +31,44 @@

 #include 

+#include "base/gtest/logging.hh"
 #include "base/sat_counter.hh"

 /**
+ * Test that an error is triggered when the number of bits exceeds the
+ * counter's capacity.
+ */
+TEST(SatCounterDeathTest, BitCountExceeds)
+{
+#ifdef NDEBUG
+GTEST_SKIP() << "Skipping as assertions are "
+"stripped out of fast builds";
+#endif
+
+gtestLogOutput.str("");
+EXPECT_ANY_THROW(SatCounter8 counter(9));
+ASSERT_NE(gtestLogOutput.str().find("Number of bits exceeds counter  
size"),

+std::string::npos);
+}
+
+/**
+ * Test that an error is triggered when the initial value is higher than  
the

+ * maximum possible value.
+ */
+TEST(SatCounterDeathTest, InitialValueExceeds)
+{
+#ifdef NDEBUG
+GTEST_SKIP() << "Skipping as assertions are "
+"stripped out of fast builds";
+#endif
+
+gtestLogOutput.str("");
+EXPECT_ANY_THROW(SatCounter8 counter(7, 128));
+ASSERT_NE(gtestLogOutput.str().find("initial value exceeds max value"),
+std::string::npos);
+}
+
+/**
  * Test if the maximum value is indeed the maximum value reachable.
  */
 TEST(SatCounterTest, MaximumValue)
@@ -183,15 +218,36 @@
 ASSERT_EQ(counter, value);
 counter >>= saturated_counter;
 ASSERT_EQ(counter, 0);
+}

-// Make sure the counters cannot be shifted by negative numbers, since
-// that is undefined behaviour. As these tests depend on asserts  
failing,

-// these tests are only functional if `TRACING_ON == 1`, when gem5 is
-// compiled as `debug` or `opt`.
-#if TRACING_ON
+/**
+ * Make sure the counters cannot be right-shifted by negative numbers,  
since

+ * that is undefined behaviour
+ */
+TEST(SatCounterDeathTest, RightShiftNegative)
+{
+#ifdef NDEBUG
+GTEST_SKIP() << "Skipping as assertions are "
+"stripped out of fast builds";
+#endif
+
+SatCounter8 counter(8);
 ASSERT_DEATH(counter >>= -1, "");
+}
+
+/**
+ * Make sure the counters cannot be left-shifted by negative numbers, since
+ * that is undefined behaviour
+ */
+TEST(SatCounterDeathTest, LeftShiftNegative)
+{
+#ifdef NDEBUG
+GTEST_SKIP() << "Skipping as assertions are "
+"stripped out of fast builds";
+#endif
+
+SatCounter8 counter(8);
 ASSERT_DEATH(counter <<= -1, "");
-#endif
 }

 /**

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


Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ia610636fe8cf636401e2b2ed623bf20b41147ea4
Gerrit-Change-Number: 42763
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Carvalho 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[release-staging-v21-0]: cpu: Delete unnecessary create() methods.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42743 )



Change subject: cpu: Delete unnecessary create() methods.
..

cpu: Delete unnecessary create() methods.

These were added in changes which were created before create() methods
were mostly automated, but were checked in after the then unnecessary
create() methods were purged.

Change-Id: I03da797ae8328fab6ef6b85dbc4ea86b34512fd5
---
M src/cpu/testers/gpu_ruby_test/cpu_thread.cc
M src/cpu/testers/gpu_ruby_test/dma_thread.cc
M src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
M src/cpu/testers/gpu_ruby_test/protocol_tester.cc
4 files changed, 0 insertions(+), 24 deletions(-)



diff --git a/src/cpu/testers/gpu_ruby_test/cpu_thread.cc  
b/src/cpu/testers/gpu_ruby_test/cpu_thread.cc

index fa801b3..d0ac10e 100644
--- a/src/cpu/testers/gpu_ruby_test/cpu_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/cpu_thread.cc
@@ -43,12 +43,6 @@
 assert(numLanes == 1);
 }

-CpuThread*
-CpuThreadParams::create() const
-{
-return new CpuThread(*this);
-}
-
 void
 CpuThread::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/dma_thread.cc  
b/src/cpu/testers/gpu_ruby_test/dma_thread.cc

index 254158d..e5f79c9 100644
--- a/src/cpu/testers/gpu_ruby_test/dma_thread.cc
+++ b/src/cpu/testers/gpu_ruby_test/dma_thread.cc
@@ -48,12 +48,6 @@

 }

-DmaThread*
-DmaThreadParams::create() const
-{
-return new DmaThread(*this);
-}
-
 void
 DmaThread::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc  
b/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc

index f2f1343..a90b204 100644
--- a/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
+++ b/src/cpu/testers/gpu_ruby_test/gpu_wavefront.cc
@@ -48,12 +48,6 @@

 }

-GpuWavefront*
-GpuWavefrontParams::create() const
-{
-return new GpuWavefront(*this);
-}
-
 void
 GpuWavefront::issueLoadOps()
 {
diff --git a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc  
b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc

index a8f8408..95e6035 100644
--- a/src/cpu/testers/gpu_ruby_test/protocol_tester.cc
+++ b/src/cpu/testers/gpu_ruby_test/protocol_tester.cc
@@ -357,9 +357,3 @@

 return true;
 }
-
-ProtocolTester*
-ProtocolTesterParams::create() const
-{
-return new ProtocolTester(*this);
-}

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


Gerrit-Project: public/gem5
Gerrit-Branch: release-staging-v21-0
Gerrit-Change-Id: I03da797ae8328fab6ef6b85dbc4ea86b34512fd5
Gerrit-Change-Number: 42743
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Build failed in Jenkins: Nightly #244

2021-03-11 Thread jenkins-no-reply--- via gem5-dev
See 

Changes:

[Jason Lowe-Power] python: Add search functions to pystats groups

[Giacomo Travaglini] arch-arm: Fix atomics permission checks in TLB

[mattdsinclair] gpu-compute: Fix accidental execution when stopped at barrier

[Giacomo Travaglini] configs: NVM missing the xor_low_bit argument in 
create_mem_intf

[yuhsingw] system-arm: update armv8 cpu-release-addr

[Jason Lowe-Power] python: Use Pattern from typing

[gabe.black] arch-x86: Clean up some style issues in regop.isa.

[gabe.black] arch-x86: Fix the "index" value for SSrcReg2.

[gabe.black] base,misc: Collapse and eliminate the ULL and LL macros.

[gabe.black] arch,cpu: Create register class descriptors.


--
[...truncated 333.74 KB...]
[--] 10 tests from MatchTest
[ RUN  ] MatchTest.Add
[   OK ] MatchTest.Add (0 ms)
[ RUN  ] MatchTest.SetExpression
[   OK ] MatchTest.SetExpression (0 ms)
[ RUN  ] MatchTest.SetExpressionVector
[   OK ] MatchTest.SetExpressionVector (0 ms)
[ RUN  ] MatchTest.SimpleMatch
[   OK ] MatchTest.SimpleMatch (0 ms)
[ RUN  ] MatchTest.SimpleMismatch
[   OK ] MatchTest.SimpleMismatch (0 ms)
[ RUN  ] MatchTest.MultipleExpressionsMatch
[   OK ] MatchTest.MultipleExpressionsMatch (0 ms)
[ RUN  ] MatchTest.MultipleExpressionsMismatch
[   OK ] MatchTest.MultipleExpressionsMismatch (0 ms)
[ RUN  ] MatchTest.WildCardMatch
[   OK ] MatchTest.WildCardMatch (0 ms)
[ RUN  ] MatchTest.WildCardMismatch
[   OK ] MatchTest.WildCardMismatch (0 ms)
[ RUN  ] MatchTest.TokensEmptyNoMatch
[   OK ] MatchTest.TokensEmptyNoMatch (0 ms)
[--] 10 tests from MatchTest (0 ms total)

[--] Global test environment tear-down
[==] 10 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 10 tests.
 [ CXX] NULL/base/pixel.test.cc -> .fo
 [LINK]  -> NULL/base/intmath.test.fast.unstripped
 [   STRIP] NULL/base/addr_range.test.fast.unstripped -> .fast
build/NULL/base/addr_range.test.fast 
--gtest_output=xml:build/NULL/unittests.fast/base/addr_range.test.xml
Running main() from build/googletest/googletest/src/gtest_main.cc
[==] Running 54 tests from 1 test suite.
[--] Global test environment set-up.
[--] 54 tests from AddrRangeTest
[ RUN  ] AddrRangeTest.ValidRange
[   OK ] AddrRangeTest.ValidRange (0 ms)
[ RUN  ] AddrRangeTest.EmptyRange
[   OK ] AddrRangeTest.EmptyRange (0 ms)
[ RUN  ] AddrRangeTest.RangeSizeOfOne
[   OK ] AddrRangeTest.RangeSizeOfOne (0 ms)
[ RUN  ] AddrRangeTest.Range16Bit
[   OK ] AddrRangeTest.Range16Bit (0 ms)
[ RUN  ] AddrRangeTest.InvalidRange
[   OK ] AddrRangeTest.InvalidRange (0 ms)
[ RUN  ] AddrRangeTest.LessThan
[   OK ] AddrRangeTest.LessThan (0 ms)
[ RUN  ] AddrRangeTest.EqualToNotEqualTo
[   OK ] AddrRangeTest.EqualToNotEqualTo (0 ms)
[ RUN  ] AddrRangeTest.MergesWith
[   OK ] AddrRangeTest.MergesWith (0 ms)
[ RUN  ] AddrRangeTest.DoesNotMergeWith
[   OK ] AddrRangeTest.DoesNotMergeWith (0 ms)
[ RUN  ] AddrRangeTest.IntersectsCompleteOverlap
[   OK ] AddrRangeTest.IntersectsCompleteOverlap (0 ms)
[ RUN  ] AddrRangeTest.IntersectsAddressWithin
[   OK ] AddrRangeTest.IntersectsAddressWithin (0 ms)
[ RUN  ] AddrRangeTest.IntersectsPartialOverlap
[   OK ] AddrRangeTest.IntersectsPartialOverlap (0 ms)
[ RUN  ] AddrRangeTest.IntersectsNoOverlap
[   OK ] AddrRangeTest.IntersectsNoOverlap (0 ms)
[ RUN  ] AddrRangeTest.IntersectsFirstLastAddressOverlap
[   OK ] AddrRangeTest.IntersectsFirstLastAddressOverlap (0 ms)
[ RUN  ] AddrRangeTest.isSubsetCompleteOverlap
[   OK ] AddrRangeTest.isSubsetCompleteOverlap (0 ms)
[ RUN  ] AddrRangeTest.isSubsetNoOverlap
[   OK ] AddrRangeTest.isSubsetNoOverlap (0 ms)
[ RUN  ] AddrRangeTest.isSubsetTrueSubset
[   OK ] AddrRangeTest.isSubsetTrueSubset (0 ms)
[ RUN  ] AddrRangeTest.isSubsetPartialSubset
[   OK ] AddrRangeTest.isSubsetPartialSubset (0 ms)
[ RUN  ] AddrRangeTest.isSubsetInterleavedCompleteOverlap
[   OK ] AddrRangeTest.isSubsetInterleavedCompleteOverlap (0 ms)
[ RUN  ] AddrRangeTest.isSubsetInterleavedNoOverlap
[   OK ] AddrRangeTest.isSubsetInterleavedNoOverlap (0 ms)
[ RUN  ] AddrRangeTest.isSubsetInterleavedPartialOverlap
[   OK ] AddrRangeTest.isSubsetInterleavedPartialOverlap (0 ms)
[ RUN  ] AddrRangeTest.Contains
[   OK ] AddrRangeTest.Contains (0 ms)
[ RUN  ] AddrRangeTest.ContainsInAnEmptyRange
[   OK ] AddrRangeTest.ContainsInAnEmptyRange (0 ms)
[ RUN  ] AddrRangeTest.RemoveIntlvBits
[   OK ] AddrRangeTest.RemoveIntlvBits (0 ms)
[ RUN  ] AddrRangeTest.addIntlvBits
[   OK ] AddrRangeTest.addIntlvBits (0 ms)
[ RUN  ] AddrRangeTest.OffsetInRange
[   OK ] AddrRangeTest.OffsetInRange (0 ms)
[ RUN  ] AddrRangeTest.OffsetOu

[gem5-dev] Change in gem5/gem5[develop]: arch: Move setting up RegClassInfos into the arches.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/41734 )


Change subject: arch: Move setting up RegClassInfos into the arches.
..

arch: Move setting up RegClassInfos into the arches.

Also remove no longer global constants from arch/registers.hh if they
are no longer used locally.

Change-Id: I1d1589db3dd4c51a5ec11e32348d394261e36d17
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/41734
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/arch/arm/freebsd/se_workload.hh
M src/arch/arm/htm.cc
M src/arch/arm/htm.hh
M src/arch/arm/isa.cc
M src/arch/arm/isa.hh
M src/arch/arm/isa_device.cc
M src/arch/arm/nativetrace.cc
M src/arch/arm/process.cc
M src/arch/arm/registers.hh
M src/arch/arm/tracers/tarmac_base.cc
M src/arch/arm/tracers/tarmac_record.hh
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
M src/arch/generic/isa.hh
M src/arch/mips/isa.cc
M src/arch/mips/isa.hh
M src/arch/mips/registers.hh
M src/arch/mips/utility.cc
M src/arch/power/isa.cc
M src/arch/power/isa.hh
M src/arch/power/isa/includes.isa
M src/arch/power/registers.hh
M src/arch/power/se_workload.hh
M src/arch/power/utility.cc
M src/arch/riscv/isa.cc
M src/arch/riscv/registers.hh
M src/arch/sparc/insts/static_inst.cc
M src/arch/sparc/interrupts.hh
M src/arch/sparc/isa.cc
M src/arch/sparc/isa.hh
M src/arch/sparc/isa/includes.isa
M src/arch/sparc/process.cc
M src/arch/sparc/registers.hh
M src/arch/sparc/remote_gdb.cc
M src/arch/sparc/tlb.cc
M src/arch/sparc/utility.cc
M src/arch/sparc/utility.hh
M src/arch/x86/isa.cc
M src/arch/x86/registers.hh
39 files changed, 126 insertions(+), 107 deletions(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/freebsd/se_workload.hh  
b/src/arch/arm/freebsd/se_workload.hh

index a228ee0..6f13201 100644
--- a/src/arch/arm/freebsd/se_workload.hh
+++ b/src/arch/arm/freebsd/se_workload.hh
@@ -34,8 +34,8 @@
 #ifndef __ARCH_ARM_FREEBSD_SE_WORKLOAD_HH__
 #define __ARCH_ARM_FREEBSD_SE_WORKLOAD_HH__

+#include "arch/arm/ccregs.hh"
 #include "arch/arm/freebsd/freebsd.hh"
-#include "arch/arm/registers.hh"
 #include "arch/arm/se_workload.hh"
 #include "params/ArmEmuFreebsd.hh"
 #include "sim/syscall_desc.hh"
diff --git a/src/arch/arm/htm.cc b/src/arch/arm/htm.cc
index 276406a..3129b3f 100644
--- a/src/arch/arm/htm.cc
+++ b/src/arch/arm/htm.cc
@@ -36,6 +36,9 @@
  */

 #include "arch/arm/htm.hh"
+
+#include "arch/arm/intregs.hh"
+#include "arch/arm/miscregs.hh"
 #include "cpu/thread_context.hh"

 void
@@ -70,7 +73,7 @@
 //tme_checkpoint->iccPmrEl1 = tc->readMiscReg(MISCREG_ICC_PMR_EL1);
 nzcv = tc->readMiscReg(MISCREG_NZCV);
 daif = tc->readMiscReg(MISCREG_DAIF);
-for (auto n = 0; n < NumIntArchRegs; n++) {
+for (auto n = 0; n < NUM_ARCH_INTREGS; n++) {
 x[n] = tc->readIntReg(n);
 }
 // TODO first detect if FP is enabled at this EL
@@ -97,7 +100,7 @@
 //tc->setMiscReg(MISCREG_ICC_PMR_EL1, tme_checkpoint->iccPmrEl1);
 tc->setMiscReg(MISCREG_NZCV, nzcv);
 tc->setMiscReg(MISCREG_DAIF, daif);
-for (auto n = 0; n < NumIntArchRegs; n++) {
+for (auto n = 0; n < NUM_ARCH_INTREGS; n++) {
 tc->setIntReg(n, x[n]);
 }
 // TODO first detect if FP is enabled at this EL
diff --git a/src/arch/arm/htm.hh b/src/arch/arm/htm.hh
index 3fa7c1d..d32c58e 100644
--- a/src/arch/arm/htm.hh
+++ b/src/arch/arm/htm.hh
@@ -44,6 +44,7 @@
  * ISA-specific types for hardware transactional memory.
  */

+#include "arch/arm/intregs.hh"
 #include "arch/arm/registers.hh"
 #include "arch/generic/htm.hh"
 #include "base/types.hh"
@@ -70,7 +71,7 @@
   private:
 uint8_t rt; // TSTART destination register
 Addr nPc; // Fallback instruction address
-std::array x; // General purpose registers
+std::array x; // General purpose registers
 std::array z; // Vector registers
 std::array p; // Predicate registers
 Addr sp; // Stack Pointer at current EL
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index c7f82e0..039224f 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -47,6 +47,7 @@
 #include "arch/arm/tlbi_op.hh"
 #include "cpu/base.hh"
 #include "cpu/checker/cpu.hh"
+#include "cpu/reg_class.hh"
 #include "debug/Arm.hh"
 #include "debug/MiscRegs.hh"
 #include "dev/arm/generic_timer.hh"
@@ -65,6 +66,16 @@
 pmu(p.pmu), impdefAsNop(p.impdef_nop),
 afterStartup(false)
 {
+_regClasses.insert(_regClasses.end(), {
+{ NUM_INTREGS },
+{ 0 },
+{ NumVecRegs },
+{ NumVecRegs * TheISA::NumVecElemPerVecReg },
+{ NumVecPredRegs },
+{ NUM_CCREGS },
+{ NUM_MISCREGS }
+});
+
 miscRegs[MISCREG_SCTLR_RST] = 0;

 // Hook up a dummy device if we haven't been configured with a
@@ -484,7 +495,7 @@
 RegVal
 ISA::readMiscRegNoEff

[gem5-dev] Change in gem5/gem5[develop]: base,tests: Add a stringstream which tracks all log output.

2021-03-11 Thread Gabe Black (Gerrit) via gem5-dev
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/42181 )


Change subject: base,tests: Add a stringstream which tracks all log output.
..

base,tests: Add a stringstream which tracks all log output.

Since gtest's SUCCEED() just throws away output sent to it, there needs
to be an alternative way to capture non-fatal warn, hack, or inform
messages. This change adds a stringstream called gtestLogOutput which
will accumulate all log messages so they can be inspected later. If you
want to see what output occurs as a result of a specific action, you can
flush out the stringstream with .str(""), perform that action, and then
check the stream's contents.

The stream also records the output of exiting logs like fatal and panic.
It's not 100% clear that these messages would be retrievable or useful,
but this at least maintains consistency between the two classes of
messages.

To avoid threading/locking issues, the stream is thread local. To
prevent tests from affecting each other and to make the output more
predictable for test assertions, it also automatically installs an event
hook which will be called each time a test starts which will clear out
the contents of the stream.

Change-Id: I9d6650feb77b676a5b2b1fc2542cdebf3c60ed34
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/42181
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/base/gtest/logging.cc
A src/base/gtest/logging.hh
2 files changed, 84 insertions(+), 1 deletion(-)

Approvals:
  Gabe Black: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/gtest/logging.cc b/src/base/gtest/logging.cc
index d9cb9eb..e1ef373 100644
--- a/src/base/gtest/logging.cc
+++ b/src/base/gtest/logging.cc
@@ -25,6 +25,8 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */

+#include "base/gtest/logging.hh"
+
 #include 

 #include 
@@ -49,7 +51,12 @@
 using Logger::Logger;

   protected:
-void log(const Loc &loc, std::string s) override { SUCCEED() << s; }
+void
+log(const Loc &loc, std::string s) override
+{
+gtestLogOutput << s;
+SUCCEED() << s;
+}
 };

 class GTestExitLogger : public Logger
@@ -61,6 +68,7 @@
 void
 log(const Loc &loc, std::string s) override
 {
+gtestLogOutput << s;
 std::cerr << loc.file << ":" << loc.line << ": " << s;
 }
 // Throw an exception to escape down to the gtest framework.
@@ -75,6 +83,25 @@

 } // anonymous namespace

+thread_local GTestLogOutput gtestLogOutput;
+
+GTestLogOutput::EventHook::EventHook(GTestLogOutput &_stream) :  
stream(_stream)

+{
+::testing::UnitTest::GetInstance()->listeners().Append(this);
+}
+
+GTestLogOutput::EventHook::~EventHook()
+{
+::testing::UnitTest::GetInstance()->listeners().Release(this);
+}
+
+void
+GTestLogOutput::EventHook::OnTestStart(const ::testing::TestInfo  
&test_info)

+{
+// Clear out the stream at the start of each test.
+stream.str("");
+}
+
 Logger &Logger::getPanic() { return panicLogger; }
 Logger &Logger::getFatal() { return fatalLogger; }
 Logger &Logger::getWarn() { return warnLogger; }
diff --git a/src/base/gtest/logging.hh b/src/base/gtest/logging.hh
new file mode 100644
index 000..3889e30
--- /dev/null
+++ b/src/base/gtest/logging.hh
@@ -0,0 +1,56 @@
+/*
+ * Copyright 2021 Google Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met: redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer;
+ * redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution;
+ * neither the name of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE