[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Fix fast-forward when using the backing store.

2020-03-12 Thread Yuan Yao (Gerrit)
Yuan Yao has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26343 )


Change subject: mem-ruby: Fix fast-forward when using the backing store.
..

mem-ruby: Fix fast-forward when using the backing store.

While the up-to-date data may reside in any agent of Ruby's memory
hierarchy, there's an optional backing store in Ruby that provides
a 'correct' view of the physical memory. When it is enabled by the
user, every Ruby memory access will update this global memory view
as well upon finishing.

The issue is that Ruby's atomic access, used in fast-forward, does
not currently access the backing store, leading to data
incorrectness. More specifically, at the very beginning stage of the
simulation, a loader loads the program into the backing store using
functional accesses. Then the program starts execution with
fast-forward enabled, using atomic accesses for faster simulation. But
because atomic access only accesses the real memory hierarchy, the CPU
fetches incorrect instructions.

The fix is simple. Just make Ruby's atomic access update the backing
store as well as the real physical memory.

Change-Id: I2541d923e18ea488d383097ca7abd4124e47e59b
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26343
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Onur Kayıran 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/mem/ruby/system/RubyPort.cc
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  Onur Kayıran: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/mem/ruby/system/RubyPort.cc  
b/src/mem/ruby/system/RubyPort.cc

index 800046e..a6cee05 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -339,7 +339,10 @@
 RubySystem *rs = ruby_port->m_ruby_system;
 AbstractController *directory =
 rs->m_abstract_controls[id.getType()][id.getNum()];
-return directory->recvAtomic(pkt);
+Tick latency = directory->recvAtomic(pkt);
+if (access_backing_store)
+rs->getPhysMem()->access(pkt);
+return latency;
 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26343
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: I2541d923e18ea488d383097ca7abd4124e47e59b
Gerrit-Change-Number: 26343
Gerrit-PatchSet: 3
Gerrit-Owner: Yuan Yao 
Gerrit-Reviewer: Bradford Beckmann 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: John Alsop 
Gerrit-Reviewer: Onur Kayıran 
Gerrit-Reviewer: Tuan Ta 
Gerrit-Reviewer: Yuan Yao 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arm: Eliminate the MustBeOne ARM specific request flag.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26546 )


Change subject: arm: Eliminate the MustBeOne ARM specific request flag.
..

arm: Eliminate the MustBeOne ARM specific request flag.

This flag makes constructing a generic Request object for ARM
impossible, since generic consumers won't know to set that flag to one.

As a principle, Request flags should change the behavior of a request
away from whatever the default/typical behavior is in the current
context, so flags set to zero means no special behavior.

Change-Id: Id606dc0bf42210218e1745585327671a98a8dba4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26546
Reviewed-by: Nikos Nikoleris 
Maintainer: Gabe Black 
Maintainer: Giacomo Travaglini 
Tested-by: kokoro 
---
M src/arch/arm/insts/macromem.cc
M src/arch/arm/insts/macromem.hh
M src/arch/arm/insts/sve_mem.hh
M src/arch/arm/isa.cc
M src/arch/arm/isa/formats/fp.isa
M src/arch/arm/isa/insts/amo64.isa
M src/arch/arm/isa/insts/branch.isa
M src/arch/arm/isa/insts/data64.isa
M src/arch/arm/isa/insts/ldr.isa
M src/arch/arm/isa/insts/ldr64.isa
M src/arch/arm/isa/insts/misc.isa
M src/arch/arm/isa/insts/str.isa
M src/arch/arm/isa/insts/str64.isa
M src/arch/arm/isa/templates/sve_mem.isa
M src/arch/arm/table_walker.cc
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
M src/arch/arm/tracers/tarmac_parser.cc
18 files changed, 47 insertions(+), 87 deletions(-)

Approvals:
  Nikos Nikoleris: 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/arch/arm/insts/macromem.cc b/src/arch/arm/insts/macromem.cc
index 125e0f8..211a176 100644
--- a/src/arch/arm/insts/macromem.cc
+++ b/src/arch/arm/insts/macromem.cc
@@ -472,7 +472,7 @@

 RegIndex rMid = deinterleave ? VecSpecialElem : vd * 2;

-uint32_t noAlign = TLB::MustBeOne;
+uint32_t noAlign = 0;

 unsigned uopIdx = 0;
 switch (regs) {
@@ -833,7 +833,7 @@
 if (interleave) numMicroops += (regs / elems);
 microOps = new StaticInstPtr[numMicroops];

-uint32_t noAlign = TLB::MustBeOne;
+uint32_t noAlign = 0;

 RegIndex rMid = interleave ? VecSpecialElem : vd * 2;

@@ -1143,8 +1143,7 @@

 microOps = new StaticInstPtr[numMicroops];
 unsigned uopIdx = 0;
-uint32_t memaccessFlags = TLB::MustBeOne | (TLB::ArmFlags) eSize |
-TLB::AllowUnaligned;
+uint32_t memaccessFlags = (TLB::ArmFlags)eSize | TLB::AllowUnaligned;

 int i = 0;
 for (; i < numMemMicroops - 1; ++i) {
@@ -1251,8 +1250,7 @@
 }
 }

-uint32_t memaccessFlags = TLB::MustBeOne | (TLB::ArmFlags) eSize |
-TLB::AllowUnaligned;
+uint32_t memaccessFlags = (TLB::ArmFlags)eSize | TLB::AllowUnaligned;

 int i = 0;
 for (; i < numMemMicroops - 1; ++i) {
@@ -1319,8 +1317,7 @@
 microOps = new StaticInstPtr[numMicroops];
 unsigned uopIdx = 0;

-uint32_t memaccessFlags = TLB::MustBeOne | (TLB::ArmFlags) eSize |
-TLB::AllowUnaligned;
+uint32_t memaccessFlags = (TLB::ArmFlags)eSize | TLB::AllowUnaligned;

 int i = 0;
 for (; i < numMemMicroops - 1; ++i) {
@@ -1398,8 +1395,7 @@
 numStructElems, index, i /* step */, replicate);
 }

-uint32_t memaccessFlags = TLB::MustBeOne | (TLB::ArmFlags) eSize |
-TLB::AllowUnaligned;
+uint32_t memaccessFlags = (TLB::ArmFlags)eSize | TLB::AllowUnaligned;

 int i = 0;
 for (; i < numMemMicroops - 1; ++i) {
diff --git a/src/arch/arm/insts/macromem.hh b/src/arch/arm/insts/macromem.hh
index b3ba76d..94a3e0f 100644
--- a/src/arch/arm/insts/macromem.hh
+++ b/src/arch/arm/insts/macromem.hh
@@ -118,8 +118,7 @@
 MicroNeonMemOp(const char *mnem, ExtMachInst machInst, OpClass  
__opClass,

RegIndex _dest, RegIndex _ura, uint32_t _imm)
 : MicroOp(mnem, machInst, __opClass),
-  dest(_dest), ura(_ura), imm(_imm),
-  memAccessFlags(TLB::MustBeOne)
+  dest(_dest), ura(_ura), imm(_imm), memAccessFlags()
 {
 }
 };
@@ -393,7 +392,7 @@
 MicroMemOp(const char *mnem, ExtMachInst machInst, OpClass __opClass,
RegIndex _ura, RegIndex _urb, bool _up, uint8_t _imm)
 : MicroIntImmOp(mnem, machInst, __opClass, _ura, _urb, _imm),
-  up(_up), memAccessFlags(TLB::MustBeOne | TLB::AlignWord)
+  up(_up), memAccessFlags(TLB::AlignWord)
 {
 }

@@ -414,7 +413,7 @@
 bool _up, uint8_t _imm)
 : MicroOp(mnem, machInst, __opClass),
 dest(_dreg1), dest2(_dreg2), urb(_base), up(_up), imm(_imm),
-memAccessFlags(TLB::MustBeOne | TLB::AlignWord)
+memAccessFlags(TLB::AlignWord)
 {
 }

diff --git a/src/arch/arm/insts/sve_mem.hh b/src/arch/arm/insts/sve_mem.hh
index 7625374..4865113 100644
--- a/src/arch/arm/insts/sve_mem.hh
+++ 

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Checkpoint from MOESI_hammer Ruby hangs

2020-03-12 Thread Hussein Elnawawy (Gerrit)
Hussein Elnawawy has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26663 )



Change subject: mem-ruby: Checkpoint from MOESI_hammer Ruby hangs
..

mem-ruby: Checkpoint from MOESI_hammer Ruby hangs

Fix MOESI_hammer checkpoint hanging.
The function markRemoved() should be called before hitCallback().
The reason is that hitCallback() is responsible for checking if
draining is complete based on the value of "m_outstanding_count"
which gets decremented in markRemoved().

Reported by: Timothy Hayes
Jira: https://gem5.atlassian.net/browse/GEM5-331
Change-Id: Ibc8df80f0d5feedf824925b216c91e9289c22c77
---
1 file changed, 0 insertions(+), 0 deletions(-)




--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26663
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: Ibc8df80f0d5feedf824925b216c91e9289c22c77
Gerrit-Change-Number: 26663
Gerrit-PatchSet: 1
Gerrit-Owner: Hussein Elnawawy 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: power: Fix regStats for PowerModel and PowerModelState

2020-03-12 Thread Giacomo Travaglini (Gerrit)

Hello Nikos Nikoleris,

I'd like you to do a code review. Please visit

https://gem5-review.googlesource.com/c/public/gem5/+/26643

to review the following change.


Change subject: power: Fix regStats for PowerModel and PowerModelState
..

power: Fix regStats for PowerModel and PowerModelState

Every Stats::Group need to call the parent regStats to
make sure that the base Stats::Group::regStats() gets
called

JIRA: https://gem5.atlassian.net/projects/GEM5/issues/GEM5-319

Change-Id: I931941d8ec5f375f7e51e719d43ae31af095f661
Signed-off-by: Giacomo Travaglini 
Reviewed-by: Nikos Nikoleris 
---
M src/sim/power/power_model.hh
1 file changed, 4 insertions(+), 0 deletions(-)



diff --git a/src/sim/power/power_model.hh b/src/sim/power/power_model.hh
index 36e0a3e..918b2d2 100644
--- a/src/sim/power/power_model.hh
+++ b/src/sim/power/power_model.hh
@@ -84,6 +84,8 @@
 }

 void regStats() {
+SimObject::regStats();
+
 dynamicPower
   .method(this, ::getDynamicPower)
   .name(params()->name + ".dynamic_power")
@@ -135,6 +137,8 @@
 double getStaticPower() const;

 void regStats() {
+SimObject::regStats();
+
 dynamicPower
   .method(this, ::getDynamicPower)
   .name(params()->name + ".dynamic_power")

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26643
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: I931941d8ec5f375f7e51e719d43ae31af095f661
Gerrit-Change-Number: 26643
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Fix Arch detection in FS if there is not bootloader

2020-03-12 Thread Giacomo Travaglini (Gerrit)
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26606 )



Change subject: arch-arm: Fix Arch detection in FS if there is not  
bootloader

..

arch-arm: Fix Arch detection in FS if there is not bootloader

In case a workload is run with no bootloader we still want to be able
to provide the simulation with the correct arch version.
Without this patch every baremetal simulation will default to AArch64, which
is the default value of ArmFsWorkload.

Change-Id: I0f766167d8983cafc1fd30d054862339eb21f73f
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/fs_workload.cc
1 file changed, 2 insertions(+), 0 deletions(-)



diff --git a/src/arch/arm/fs_workload.cc b/src/arch/arm/fs_workload.cc
index befba3a..ce9c464 100644
--- a/src/arch/arm/fs_workload.cc
+++ b/src/arch/arm/fs_workload.cc
@@ -77,6 +77,8 @@

 entry = bootldr->entryPoint();
 _highestELIs64 = (bootldr->getArch() == ObjectFile::Arm64);
+} else {
+_highestELIs64 = (obj->getArch() == ObjectFile::Arm64);
 }
 }


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26606
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: I0f766167d8983cafc1fd30d054862339eb21f73f
Gerrit-Change-Number: 26606
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: dev-arm: add FVP Base Power Controller model

2020-03-12 Thread Giacomo Travaglini (Gerrit)
Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26463 )


Change subject: dev-arm: add FVP Base Power Controller model
..

dev-arm: add FVP Base Power Controller model

This is a reduced model of the FVP Base platforms Power Controller.
As of now it allows the following functions from software:
- Checking for core presence
- Reporting the power state of a core / cluster
- Explicitly powering off a core on WFI
- Explicitly powering off cores in a CPU cluster on WFI
- Explicitly powering on a core through writes to PPONR register

Change-Id: Ia1d4d3ae8e4bfb2d23b2c6077396a4d8500e2e30
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26463
Maintainer: Giacomo Travaglini 
Reviewed-by: Nikos Nikoleris 
Tested-by: kokoro 
---
M src/arch/arm/isa/insts/misc.isa
M src/arch/arm/system.cc
M src/arch/arm/system.hh
M src/dev/arm/RealView.py
M src/dev/arm/SConscript
A src/dev/arm/fvp_base_pwr_ctrl.cc
A src/dev/arm/fvp_base_pwr_ctrl.hh
M src/dev/arm/gic_v3.cc
8 files changed, 518 insertions(+), 4 deletions(-)

Approvals:
  Nikos Nikoleris: Looks good to me, approved
  Giacomo Travaglini: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/isa/insts/misc.isa  
b/src/arch/arm/isa/insts/misc.isa

index 0c6fdc3..03401ca 100644
--- a/src/arch/arm/isa/insts/misc.isa
+++ b/src/arch/arm/isa/insts/misc.isa
@@ -762,6 +762,7 @@
 fault = trapWFx(tc, cpsr, scr, false);
 if (fault == NoFault) {
 PseudoInst::quiesce(tc);
+ArmSystem::callSetStandByWfi(tc);
 } else {
 PseudoInst::quiesceSkip(tc);
 }
diff --git a/src/arch/arm/system.cc b/src/arch/arm/system.cc
index 6ecc75a..9053a5c 100644
--- a/src/arch/arm/system.cc
+++ b/src/arch/arm/system.cc
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2012-2013, 2015,2017-2019 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2015,2017-2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -47,6 +47,7 @@
 #include "base/loader/object_file.hh"
 #include "base/loader/symtab.hh"
 #include "cpu/thread_context.hh"
+#include "dev/arm/fvp_base_pwr_ctrl.hh"
 #include "dev/arm/gic_v2.hh"
 #include "mem/fs_translating_port_proxy.hh"
 #include "mem/physical.hh"
@@ -62,6 +63,7 @@
   _haveCrypto(p->have_crypto),
   _genericTimer(nullptr),
   _gic(nullptr),
+  _pwrCtrl(nullptr),
   _highestELIs64(p->highest_el_is_64),
   _physAddrRange64(p->phys_addr_range_64),
   _haveLargeAsid64(p->have_large_asid_64),
@@ -197,6 +199,20 @@
 return sys->semihosting->call32(tc, op, param);
 }

+void
+ArmSystem::callSetStandByWfi(ThreadContext *tc)
+{
+if (FVPBasePwrCtrl *pwr_ctrl = getArmSystem(tc)->getPowerController())
+pwr_ctrl->setStandByWfi(tc);
+}
+
+void
+ArmSystem::callClearStandByWfi(ThreadContext *tc)
+{
+if (FVPBasePwrCtrl *pwr_ctrl = getArmSystem(tc)->getPowerController())
+pwr_ctrl->clearStandByWfi(tc);
+}
+
 ArmSystem *
 ArmSystemParams::create()
 {
diff --git a/src/arch/arm/system.hh b/src/arch/arm/system.hh
index 0c75ad5..d9268f0 100644
--- a/src/arch/arm/system.hh
+++ b/src/arch/arm/system.hh
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2012-2013, 2015-2019 ARM Limited
+ * Copyright (c) 2010, 2012-2013, 2015-2020 ARM Limited
  * All rights reserved
  *
  * The license below extends only to copyright in the software and shall
@@ -53,6 +53,7 @@

 class GenericTimer;
 class BaseGic;
+class FVPBasePwrCtrl;
 class ThreadContext;

 class ArmSystem : public System
@@ -85,6 +86,11 @@
 BaseGic *_gic;

 /**
+ * Pointer to the Power Controller (if any)
+ */
+FVPBasePwrCtrl *_pwrCtrl;
+
+/**
  * Reset address (ARMv8)
  */
 Addr _resetAddr;
@@ -175,12 +181,21 @@
 /** Sets the pointer to the GIC. */
 void setGIC(BaseGic *gic) { _gic = gic; }

+/** Sets the pointer to the Power Controller */
+void setPowerController(FVPBasePwrCtrl *pwr_ctrl)
+{
+_pwrCtrl = pwr_ctrl;
+}
+
 /** Get a pointer to the system's generic timer model */
 GenericTimer *getGenericTimer() const { return _genericTimer; }

 /** Get a pointer to the system's GIC */
 BaseGic *getGIC() const { return _gic; }

+/** Get a pointer to the system's power controller */
+FVPBasePwrCtrl *getPowerController() const { return _pwrCtrl; }
+
 /** Returns true if the register width of the highest implemented  
exception

  * level is 64 bits (ARMv8) */
 bool highestELIs64() const { return _highestELIs64; }
@@ -305,6 +320,12 @@
 /** Make a Semihosting call from aarch32 */
 static uint32_t callSemihosting32(ThreadContext *tc,
   uint32_t op, uint32_t param);
+
+/** Make a call to notify the power controller of STANDBYWFI assertion  
*/

+static void 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Rewrite getMPIDR

2020-03-12 Thread Giacomo Travaglini (Gerrit)
Giacomo Travaglini has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26304 )


Change subject: arch-arm: Rewrite getMPIDR
..

arch-arm: Rewrite getMPIDR

This patch is rewriting getMPIDR to have a more canonical form:

* Using threadId() instead of contextId()

It is also splitting the helper so that a client can get an affinity
number given a specific thread context.

Change-Id: I727e4b96ada345fd548cd3ff9423bf27347812c4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/26304
Reviewed-by: Nikos Nikoleris 
Reviewed-by: Ciro Santilli 
Tested-by: kokoro 
Maintainer: Giacomo Travaglini 
---
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
2 files changed, 40 insertions(+), 13 deletions(-)

Approvals:
  Nikos Nikoleris: Looks good to me, approved
  Ciro Santilli: Looks good to me, but someone else must approve
  Giacomo Travaglini: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 373508c..8f81a34 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -263,18 +263,42 @@
 // for simulation of larger systems
 assert((0 <= tc->cpuId()) && (tc->cpuId() < 256));
 assert(tc->socketId() < 65536);
-if (arm_sys->multiThread) {
-   return 0x8000 | // multiprocessor extensions available
-  0x0100 | // multi-threaded cores
-  tc->contextId();
-} else if (arm_sys->multiProc) {
-   return 0x8000 | // multiprocessor extensions available
-  tc->cpuId() | tc->socketId() << 8;
-} else {
-   return 0x8000 |  // multiprocessor extensions available
-  0x4000 |  // in up system
-  tc->cpuId() | tc->socketId() << 8;
-}
+
+RegVal mpidr = 0x8000;
+
+if (!arm_sys->multiProc)
+replaceBits(mpidr, 30, 1);
+
+if (arm_sys->multiThread)
+replaceBits(mpidr, 24, 1);
+
+// Get Affinity numbers
+mpidr |= getAffinity(arm_sys, tc);
+return mpidr;
+}
+
+static RegVal
+getAff2(ArmSystem *arm_sys, ThreadContext *tc)
+{
+return arm_sys->multiThread ? tc->socketId() << 16 : 0;
+}
+
+static RegVal
+getAff1(ArmSystem *arm_sys, ThreadContext *tc)
+{
+return arm_sys->multiThread ? tc->cpuId() << 8 : tc->socketId() << 8;
+}
+
+static RegVal
+getAff0(ArmSystem *arm_sys, ThreadContext *tc)
+{
+return arm_sys->multiThread ? tc->threadId() : tc->cpuId();
+}
+
+RegVal
+getAffinity(ArmSystem *arm_sys, ThreadContext *tc)
+{
+return getAff2(arm_sys, tc) | getAff1(arm_sys, tc) | getAff0(arm_sys,  
tc);

 }

 bool
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 409ceed..3d64bdd 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -256,9 +256,12 @@
  * to VMPIDR_EL2 (as it happens in virtualized systems) */
 RegVal readMPIDR(ArmSystem *arm_sys, ThreadContext *tc);

-/** This helper function is returing the value of MPIDR_EL1 */
+/** This helper function is returning the value of MPIDR_EL1 */
 RegVal getMPIDR(ArmSystem *arm_sys, ThreadContext *tc);

+/** Retrieves MPIDR_EL1.{Aff2,Aff1,Aff0} affinity numbers */
+RegVal getAffinity(ArmSystem *arm_sys, ThreadContext *tc);
+
 static inline uint32_t
 mcrMrcIssBuild(bool isRead, uint32_t crm, IntRegIndex rt, uint32_t crn,
uint32_t opc1, uint32_t opc2)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26304
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: I727e4b96ada345fd548cd3ff9423bf27347812c4
Gerrit-Change-Number: 26304
Gerrit-PatchSet: 4
Gerrit-Owner: Giacomo Travaglini 
Gerrit-Reviewer: Ciro Santilli 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Nikos Nikoleris 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: base: Do not treat addresses < 10 specially

2020-03-12 Thread Boris Shingarov (Gerrit)
Boris Shingarov has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26605 )



Change subject: base: Do not treat addresses < 10 specially
..

base: Do not treat addresses < 10 specially

The RSP stub (base/remote_gdb.cc) treats virtual addresses below 0x000A as
meaning "the address used in the previous m-packet".  This leads to nasty
surprises, and is not justified by neither the RSP protocol documentation
nor other existing RSP implementations.

Jira Issue: https://gem5.atlassian.net/browse/GEM5-407

Change-Id: I5fccc10a58d9af8566d45418905c0f47ffab
---
M src/base/remote_gdb.cc
1 file changed, 0 insertions(+), 16 deletions(-)



diff --git a/src/base/remote_gdb.cc b/src/base/remote_gdb.cc
index add903e..ada5e27 100644
--- a/src/base/remote_gdb.cc
+++ b/src/base/remote_gdb.cc
@@ -604,14 +604,6 @@
 bool
 BaseRemoteGDB::read(Addr vaddr, size_t size, char *data)
 {
-static Addr lastaddr = 0;
-static size_t lastsize = 0;
-
-if (vaddr < 10) {
-  DPRINTF(GDBRead, "read:  reading memory location zero!\n");
-  vaddr = lastaddr + lastsize;
-}
-
 DPRINTF(GDBRead, "read:  addr=%#x, size=%d", vaddr, size);

 PortProxy  = tc->getVirtProxy();
@@ -635,14 +627,6 @@
 bool
 BaseRemoteGDB::write(Addr vaddr, size_t size, const char *data)
 {
-static Addr lastaddr = 0;
-static size_t lastsize = 0;
-
-if (vaddr < 10) {
-  DPRINTF(GDBWrite, "write: writing memory location zero!\n");
-  vaddr = lastaddr + lastsize;
-}
-
 if (DTRACE(GDBWrite)) {
 DPRINTFN("write: addr=%#x, size=%d", vaddr, size);
 if (DTRACE(GDBExtra)) {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26605
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: I5fccc10a58d9af8566d45418905c0f47ffab
Gerrit-Change-Number: 26605
Gerrit-PatchSet: 1
Gerrit-Owner: Boris Shingarov 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim: Rename allocate in GuestABI to prepare.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24106 )


Change subject: sim: Rename allocate in GuestABI to prepare.
..

sim: Rename allocate in GuestABI to prepare.

This method can be used for allocating resources for registers and/or
arguments, but it can also be used for generic preparation for them
as well. For instance, it can be used to detect some sort of property
of the function signature as a whole (if it's variadic for instance)
which can be stored in position and used to change ABI behavior.

Change-Id: I8a090be65dc4987e35cd115562114cd1b748155f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24106
Reviewed-by: Gabe Black 
Maintainer: Gabe Black 
Tested-by: kokoro 
---
M src/sim/guest_abi.hh
M src/sim/guest_abi.test.cc
M src/sim/guest_abi/definition.hh
M src/sim/guest_abi/layout.hh
4 files changed, 39 insertions(+), 38 deletions(-)

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



diff --git a/src/sim/guest_abi.hh b/src/sim/guest_abi.hh
index 97d3e21..310fc00 100644
--- a/src/sim/guest_abi.hh
+++ b/src/sim/guest_abi.hh
@@ -50,7 +50,7 @@
 // Default construct a Position to track consumed resources. Built in
 // types will be zero initialized.
 auto position = GuestABI::initializePosition(tc);
-GuestABI::allocateSignature(tc, position);
+GuestABI::prepareForFunction(tc, position);
 return GuestABI::callFrom(tc, position, target);
 }

@@ -70,7 +70,7 @@
 // Default construct a Position to track consumed resources. Built in
 // types will be zero initialized.
 auto position = GuestABI::initializePosition(tc);
-GuestABI::allocateArguments(tc, position);
+GuestABI::prepareForArguments(tc, position);
 GuestABI::callFrom(tc, position, target);
 }

@@ -96,7 +96,7 @@
 auto position = GuestABI::initializePosition(tc);
 std::ostringstream ss;

-GuestABI::allocateSignature(tc, position);
+GuestABI::prepareForFunction(tc, position);
 ss << name;
 GuestABI::dumpArgsFrom(0, ss, tc, position);
 return ss.str();
diff --git a/src/sim/guest_abi.test.cc b/src/sim/guest_abi.test.cc
index 9b1fe61..9ac7def 100644
--- a/src/sim/guest_abi.test.cc
+++ b/src/sim/guest_abi.test.cc
@@ -66,8 +66,8 @@
 using Position = int;
 };

-// ABI anchor for an ABI which uses the allocate() hook.
-struct TestABI_Allocate
+// ABI anchor for an ABI which uses the prepare() hook.
+struct TestABI_Prepare
 {
 using Position = int;
 };
@@ -136,31 +136,31 @@
 }
 };

-// Hooks for the ABI which uses allocate(). It uses the same rules as the
+// Hooks for the ABI which uses prepare(). It uses the same rules as the
 // 1D ABI for arguments, but allocates space for and discards return values
 // and returns integer arguments in reverse order.
 template <>
-struct Argument
+struct Argument
 {
 static int
-get(ThreadContext *tc, TestABI_Allocate::Position )
+get(ThreadContext *tc, TestABI_Prepare::Position )
 {
 return tc->ints[--position];
 }

 static void
-allocate(ThreadContext *tc, TestABI_Allocate::Position )
+prepare(ThreadContext *tc, TestABI_Prepare::Position )
 {
 position++;
 }
 };

 template 
-struct Result
+struct Result
 {
 static void store(ThreadContext *tc, const Ret ) {}
 static void
-allocate(ThreadContext *tc, TestABI_Allocate::Position )
+prepare(ThreadContext *tc, TestABI_Prepare::Position )
 {
 position++;
 }
@@ -243,14 +243,14 @@
 // Test functions which verify that the return allocating ABI allocates  
space

 // for its return value successfully.
 void
-testAllocateVoid(ThreadContext *tc, int a, int b)
+testPrepareVoid(ThreadContext *tc, int a, int b)
 {
 EXPECT_EQ(a, tc->ints[1]);
 EXPECT_EQ(b, tc->ints[0]);
 }

 int
-testAllocateInt(ThreadContext *tc, int a, int b)
+testPrepareInt(ThreadContext *tc, int a, int b)
 {
 EXPECT_EQ(a, tc->ints[2]);
 EXPECT_EQ(b, tc->ints[1]);
@@ -299,11 +299,11 @@
 EXPECT_EQ(tc.floatResult, tc.DefaultFloatResult);
 }

-TEST(GuestABI, ABI_Allocate)
+TEST(GuestABI, ABI_Prepare)
 {
 ThreadContext tc;
-invokeSimcall(, testAllocateVoid);
-invokeSimcall(, testAllocateInt);
+invokeSimcall(, testPrepareVoid);
+invokeSimcall(, testPrepareInt);
 }

 TEST(GuestABI, ABI_2D_args)
diff --git a/src/sim/guest_abi/definition.hh  
b/src/sim/guest_abi/definition.hh

index c61d1ae..b43a482 100644
--- a/src/sim/guest_abi/definition.hh
+++ b/src/sim/guest_abi/definition.hh
@@ -72,11 +72,12 @@
   typename ABI::Position );

 /*
- * Adjust the position of arguments based on the return type, if  
necessary.

+ * Prepare for a result of type Ret. This might mean, for instance,
+ * allocating an argument register for a result pointer.
  *
- * This method can be excluded if no 

[gem5-dev] Change in gem5/gem5[develop]: kern: Minor style cleanup in the base SkipFuncEvent class.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24110 )


Change subject: kern: Minor style cleanup in the base SkipFuncEvent class.
..

kern: Minor style cleanup in the base SkipFuncEvent class.

Change-Id: I0f4efb825b0611d3bab5a429fd36d28a178f86b9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24110
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/kern/system_events.cc
M src/kern/system_events.hh
2 files changed, 1 insertion(+), 4 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/kern/system_events.cc b/src/kern/system_events.cc
index d4dd792..e865be5 100644
--- a/src/kern/system_events.cc
+++ b/src/kern/system_events.cc
@@ -28,15 +28,12 @@

 #include "kern/system_events.hh"

-#include "arch/isa_traits.hh"
 #include "arch/utility.hh"
 #include "base/trace.hh"
 #include "config/the_isa.hh"
 #include "cpu/thread_context.hh"
 #include "debug/PCEvent.hh"

-using namespace TheISA;
-
 void
 SkipFuncEvent::process(ThreadContext *tc)
 {
diff --git a/src/kern/system_events.hh b/src/kern/system_events.hh
index 912a505..83ed37f 100644
--- a/src/kern/system_events.hh
+++ b/src/kern/system_events.hh
@@ -37,7 +37,7 @@
 SkipFuncEvent(PCEventScope *s, const std::string , Addr addr)
 : PCEvent(s, desc, addr)
 {}
-virtual void process(ThreadContext *tc);
+void process(ThreadContext *tc) override;
 };

 #endif // __SYSTEM_EVENTS_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/24110
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: I0f4efb825b0611d3bab5a429fd36d28a178f86b9
Gerrit-Change-Number: 24110
Gerrit-PatchSet: 18
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arm: Implement the AAPCS32 ABI.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24108 )


Change subject: arm: Implement the AAPCS32 ABI.
..

arm: Implement the AAPCS32 ABI.

Change-Id: I63b2ec586146163642392f5164fb01335d811471
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24108
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
A src/arch/arm/aapcs32.hh
1 file changed, 611 insertions(+), 0 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/aapcs32.hh b/src/arch/arm/aapcs32.hh
new file mode 100644
index 000..3e9ad54
--- /dev/null
+++ b/src/arch/arm/aapcs32.hh
@@ -0,0 +1,611 @@
+/*
+ * Copyright 2019 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.
+ */
+
+#ifndef __ARCH_ARM_AAPCS32_HH__
+#define __ARCH_ARM_AAPCS32_HH__
+
+#include 
+#include 
+#include 
+#include 
+
+#include "arch/arm/intregs.hh"
+#include "arch/arm/utility.hh"
+#include "base/intmath.hh"
+#include "cpu/thread_context.hh"
+#include "sim/guest_abi.hh"
+#include "sim/syscall_emul_buf.hh"
+
+class ThreadContext;
+
+struct Aapcs32
+{
+struct State
+{
+bool stackUsed=false; // Whether anything has been put on the  
stack.

+
+int ncrn=0; // Next general purpose register number.
+Addr nsaa; // Next stacked argument address.
+
+// The maximum allowed general purpose register number.
+static const int MAX_CRN = 3;
+
+Addr retAddr=0;
+
+explicit State(const ThreadContext *tc) :
+nsaa(tc->readIntReg(ArmISA::INTREG_SPX))
+{}
+};
+};
+
+namespace GuestABI
+{
+
+/*
+ * Composite Types
+ */
+
+template 
+struct IsAapcs32Composite : public std::false_type {};
+
+template 
+struct IsAapcs32Composite::value ||
+ std::is_class::value ||
+ std::is_union::value) &&
+// VarArgs is technically a composite type, but it's not a normal  
argument.

+!IsVarArgs::value
+>::type> : public std::true_type
+{};
+
+// Homogeneous Aggregates
+// These *should* be any aggregate type which has only one type of member,  
but
+// we can't actually detect that or manipulate that with templates.  
Instead,

+// we approximate that by detecting only arrays with that property.
+
+template 
+using Aapcs32HomogeneousAggregate = T[count];
+
+template 
+struct IsAapcs32HomogeneousAggregate : public std::false_type {};
+
+template 
+struct IsAapcs32HomogeneousAggregate : public std::true_type {};
+
+struct Aapcs32ArgumentBase
+{
+template 
+static T
+loadFromStack(ThreadContext *tc, Aapcs32::State )
+{
+state.stackUsed = true;
+
+// The alignment is the larger of 4 or the natural alignment of T.
+size_t align = std::max(4, alignof(T));
+// Increase the size to the next multiple of 4.
+size_t size = roundUp(sizeof(T), 4);
+
+// Align the stack.
+state.nsaa = roundUp(state.nsaa, align);
+
+// Extract the value from it.
+TypedBufferArg val(state.nsaa);
+val.copyIn(tc->getVirtProxy());
+
+// Move the nsaa past this argument.
+state.nsaa += size;
+
+// Return the value we extracted.
+return gtoh(*val, ArmISA::byteOrder(tc));
+}
+};
+
+
+/*
+ * Integer arguments and return values.
+ */
+
+template 
+struct Result::value>::type>
+{
+static void
+

[gem5-dev] Change in gem5/gem5[develop]: mem: Get rid of the now unused SecurePortProxy class.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26625 )



Change subject: mem: Get rid of the now unused SecurePortProxy class.
..

mem: Get rid of the now unused SecurePortProxy class.

This proxy was only used by the ARM semihosting interface which can now
use a tweaked regular TranslatingPortProxy or SETranslatingPortProxy
instead of this special purpose class.

This sort of class would still be necessary if you wanted to use
physical addresses and not virtual addresses, but presently there is no
such use. This code can be retrieved from history if it's needed in the
future.

Change-Id: Ie47a8b4bb173cba1a06bd3ca60391081987936b8
---
M src/mem/SConscript
D src/mem/secure_port_proxy.cc
D src/mem/secure_port_proxy.hh
3 files changed, 0 insertions(+), 138 deletions(-)



diff --git a/src/mem/SConscript b/src/mem/SConscript
index f7636b2..2eb64e0 100644
--- a/src/mem/SConscript
+++ b/src/mem/SConscript
@@ -70,7 +70,6 @@
 Source('packet_queue.cc')
 Source('port_proxy.cc')
 Source('physical.cc')
-Source('secure_port_proxy.cc')
 Source('simple_mem.cc')
 Source('snoop_filter.cc')
 Source('stack_dist_calc.cc')
diff --git a/src/mem/secure_port_proxy.cc b/src/mem/secure_port_proxy.cc
deleted file mode 100644
index 9b822c7..000
--- a/src/mem/secure_port_proxy.cc
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * Copyright (c) 2012, 2018 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder.  You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * 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.
- */
-
-#include "mem/secure_port_proxy.hh"
-
-bool
-SecurePortProxy::tryReadBlob(Addr addr, void *p, int size) const
-{
-readBlobPhys(addr, Request::SECURE, p, size);
-return true;
-}
-
-bool
-SecurePortProxy::tryWriteBlob(Addr addr, const void *p, int size) const
-{
-writeBlobPhys(addr, Request::SECURE, p, size);
-return true;
-}
-
-bool
-SecurePortProxy::tryMemsetBlob(Addr addr, uint8_t v, int size) const
-{
-memsetBlobPhys(addr, Request::SECURE, v, size);
-return true;
-}
diff --git a/src/mem/secure_port_proxy.hh b/src/mem/secure_port_proxy.hh
deleted file mode 100644
index 8336531..000
--- a/src/mem/secure_port_proxy.hh
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * Copyright (c) 2011-2013, 2018 ARM Limited
- * All rights reserved
- *
- * The license below extends only to copyright in the software and shall
- * not be construed as granting a license to any other intellectual
- * property including but not limited to intellectual property relating
- * to a hardware implementation of the functionality of the software
- * licensed hereunder.  You may use the software subject to the license
- * terms below provided that you ensure that this notice is replicated
- * unmodified and in its entirety in all distributions of the software,
- * modified or unmodified, in source code or in binary form.
- *
- * Redistribution and use in source and binary forms, with or without
- * 

[gem5-dev] Change in gem5/gem5[develop]: arm: Make semihosting use virtual addresses.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26624 )



Change subject: arm: Make semihosting use virtual addresses.
..

arm: Make semihosting use virtual addresses.

This is in accordance with the spec. To successfully translate requests
which need their secure flag set, build a translating port proxy with
that flag enabled.

Change-Id: I6ceec12aed297c57831a368a74d8b4e41f86f4c9
---
M src/arch/arm/semihosting.cc
M src/arch/arm/semihosting.hh
2 files changed, 29 insertions(+), 22 deletions(-)



diff --git a/src/arch/arm/semihosting.cc b/src/arch/arm/semihosting.cc
index 4f897e2..00d1bbc 100644
--- a/src/arch/arm/semihosting.cc
+++ b/src/arch/arm/semihosting.cc
@@ -45,9 +45,11 @@
 #include "debug/Semihosting.hh"
 #include "dev/serial/serial.hh"
 #include "mem/physical.hh"
-#include "mem/secure_port_proxy.hh"
+#include "mem/se_translating_port_proxy.hh"
+#include "mem/translating_port_proxy.hh"
 #include "params/ArmSemihosting.hh"
 #include "sim/byteswap.hh"
+#include "sim/full_system.hh"
 #include "sim/sim_exit.hh"
 #include "sim/system.hh"

@@ -173,7 +175,7 @@
 }

 std::vector argv(call->argc64 + 1);
-PortProxy  = physProxy(tc);
+PortProxy  = portProxy(tc);
 ByteOrder endian = ArmISA::byteOrder(tc);

 DPRINTF(Semihosting, "Semihosting call64: %s(0x%x)\n", call->name,  
param);

@@ -208,7 +210,7 @@
 }

 std::vector argv(call->argc32 + 1);
-PortProxy  = physProxy(tc);
+PortProxy  = portProxy(tc);
 ByteOrder endian = ArmISA::byteOrder(tc);

 DPRINTF(Semihosting, "Semihosting call32: %s(0x%x)\n", call->name,  
param);

@@ -253,18 +255,23 @@
 }

 PortProxy &
-ArmSemihosting::physProxy(ThreadContext *tc)
+ArmSemihosting::portProxy(ThreadContext *tc)
 {
 if (ArmISA::inSecureState(tc)) {
-if (!physProxyS) {
-System *sys = tc->getSystemPtr();
-physProxyS.reset(new SecurePortProxy(
- sys->getSystemPort(),
- sys->cacheLineSize()));
+if (!portProxyS) {
+if (FullSystem) {
+portProxyS.reset(
+new TranslatingPortProxy(tc, Request::SECURE));
+} else {
+portProxyS.reset(
+new SETranslatingPortProxy(
+tc, SETranslatingPortProxy::NextPage,
+Request::SECURE));
+}
 }
-return *physProxyS;
+return *portProxyS;
 } else {
-return tc->getPhysProxy();
+return tc->getVirtProxy();
 }
 }

@@ -275,7 +282,7 @@
 std::vector buf(len + 1);

 buf[len] = '\0';
-physProxy(tc).readBlob(ptr, buf.data(), len);
+portProxy(tc).readBlob(ptr, buf.data(), len);

 return std::string(buf.data());
 }
@@ -337,7 +344,7 @@
 ArmSemihosting::callWriteC(ThreadContext *tc, bool aarch64,
std::vector )
 {
-const char c = physProxy(tc).read(argv[0]);
+const char c = portProxy(tc).read(argv[0]);

 DPRINTF(Semihosting, "Semihosting SYS_WRITEC('%c')\n", c);
 std::cout.put(c);
@@ -350,7 +357,7 @@
std::vector )
 {
 DPRINTF(Semihosting, "Semihosting SYS_WRITE0(...)\n");
-PortProxy  = physProxy(tc);
+PortProxy  = portProxy(tc);
 for (Addr addr = (Addr)argv[0]; ; ++addr) {
 char data = proxy.read(addr);
 if (data == 0)
@@ -370,7 +377,7 @@
 return RetErrno(argv[3], EBADF);

 std::vector buffer(argv[3]);
-physProxy(tc).readBlob(argv[2], buffer.data(), buffer.size());
+portProxy(tc).readBlob(argv[2], buffer.data(), buffer.size());

 int64_t ret = files[argv[1]]->write(buffer.data(), buffer.size());
 if (ret < 0) {
@@ -397,7 +404,7 @@
 } else {
 panic_if(ret > buffer.size(), "Read longer than buffer size.");

-physProxy(tc).writeBlob(argv[2], buffer.data(), ret);
+portProxy(tc).writeBlob(argv[2], buffer.data(), ret);

 // Return the number of bytes not written
 return retOK(argv[3] - ret);
@@ -484,7 +491,7 @@
 if (path_len >= max_len)
 return retError(ENOSPC);

-physProxy(tc).writeBlob(guest_buf, path, path_len + 1);
+portProxy(tc).writeBlob(guest_buf, path, path_len + 1);
 return retOK(0);
 }

@@ -553,7 +560,7 @@
std::vector )
 {
 if (cmdLine.size() + 1 < argv[2]) {
-PortProxy  = physProxy(tc);
+PortProxy  = portProxy(tc);
 ByteOrder endian = ArmISA::byteOrder(tc);
 proxy.writeBlob((Addr)argv[1], cmdLine.c_str(), cmdLine.size() +  
1);


@@ -608,7 +615,7 @@
heap_base, heap_limit, stack_base, stack_limit);

 Addr base = argv[1];
-PortProxy  = physProxy(tc);
+PortProxy  = portProxy(tc);
 ByteOrder endian = ArmISA::byteOrder(tc);
 if (aarch64) {
 

[gem5-dev] Change in gem5/gem5[develop]: mem: Add a Request::Flags parameter to the translating port proxies.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/26623 )



Change subject: mem: Add a Request::Flags parameter to the translating port  
proxies.

..

mem: Add a Request::Flags parameter to the translating port proxies.

These flags will be given to the Request object which is used to do the
translation.

Change-Id: I21755f5f9369311e2f2d5be73ebd4f5865f73265
---
M src/mem/se_translating_port_proxy.cc
M src/mem/se_translating_port_proxy.hh
M src/mem/translating_port_proxy.cc
M src/mem/translating_port_proxy.hh
4 files changed, 14 insertions(+), 9 deletions(-)



diff --git a/src/mem/se_translating_port_proxy.cc  
b/src/mem/se_translating_port_proxy.cc

index 458b2fa..8af628f 100644
--- a/src/mem/se_translating_port_proxy.cc
+++ b/src/mem/se_translating_port_proxy.cc
@@ -44,8 +44,8 @@
 #include "sim/system.hh"

 SETranslatingPortProxy::SETranslatingPortProxy(
-ThreadContext *tc, AllocType alloc) :
-TranslatingPortProxy(tc), allocating(alloc)
+ThreadContext *tc, AllocType alloc, Request::Flags _flags) :
+TranslatingPortProxy(tc, _flags), allocating(alloc)
 {}

 bool
diff --git a/src/mem/se_translating_port_proxy.hh  
b/src/mem/se_translating_port_proxy.hh

index 1b6be8a..0fe3212 100644
--- a/src/mem/se_translating_port_proxy.hh
+++ b/src/mem/se_translating_port_proxy.hh
@@ -60,7 +60,8 @@
 bool fixupAddr(Addr addr, BaseTLB::Mode mode) const override;

   public:
-SETranslatingPortProxy(ThreadContext *tc, AllocType alloc);
+SETranslatingPortProxy(ThreadContext *tc, AllocType alloc,
+   Request::Flags _flags=0);
 };

 #endif // __MEM_SE_TRANSLATING_PORT_PROXY_HH__
diff --git a/src/mem/translating_port_proxy.cc  
b/src/mem/translating_port_proxy.cc

index c44ff5f..8bb93cc 100644
--- a/src/mem/translating_port_proxy.cc
+++ b/src/mem/translating_port_proxy.cc
@@ -50,10 +50,12 @@
 #include "cpu/thread_context.hh"
 #include "sim/system.hh"

-TranslatingPortProxy::TranslatingPortProxy(ThreadContext *tc) :
+TranslatingPortProxy::TranslatingPortProxy(
+ThreadContext *tc, Request::Flags _flags) :
 PortProxy(tc->getCpuPtr()->getSendFunctional(),
   tc->getSystemPtr()->cacheLineSize()), _tc(tc),
-  pageBytes(tc->getSystemPtr()->getPageBytes())
+  pageBytes(tc->getSystemPtr()->getPageBytes()),
+  flags(_flags)
 {}

 bool
@@ -81,7 +83,7 @@
  gen.next())
 {
 auto req = std::make_shared(
-gen.addr(), gen.size(), 0, Request::funcMasterId, 0,
+gen.addr(), gen.size(), flags, Request::funcMasterId, 0,
 _tc->contextId());

 if (!tryTLBs(req, BaseTLB::Read))
@@ -103,7 +105,7 @@
  gen.next())
 {
 auto req = std::make_shared(
-gen.addr(), gen.size(), 0, Request::funcMasterId, 0,
+gen.addr(), gen.size(), flags, Request::funcMasterId, 0,
 _tc->contextId());

 if (!tryTLBs(req, BaseTLB::Write))
@@ -123,7 +125,7 @@
  gen.next())
 {
 auto req = std::make_shared(
-gen.addr(), gen.size(), 0, Request::funcMasterId, 0,
+gen.addr(), gen.size(), flags, Request::funcMasterId, 0,
 _tc->contextId());

 if (!tryTLBs(req, BaseTLB::Write))
diff --git a/src/mem/translating_port_proxy.hh  
b/src/mem/translating_port_proxy.hh

index 1e17c64..a5dfe7f 100644
--- a/src/mem/translating_port_proxy.hh
+++ b/src/mem/translating_port_proxy.hh
@@ -62,6 +62,8 @@
 ThreadContext* _tc;
 const Addr pageBytes;

+Request::Flags flags;
+
 virtual bool
 fixupAddr(Addr addr, BaseTLB::Mode mode) const
 {
@@ -70,7 +72,7 @@

   public:

-TranslatingPortProxy(ThreadContext* tc);
+TranslatingPortProxy(ThreadContext *tc, Request::Flags _flags=0);

 /** Version of tryReadblob that translates virt->phys and deals
   * with page boundries. */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/26623
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: I21755f5f9369311e2f2d5be73ebd4f5865f73265
Gerrit-Change-Number: 26623
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: arm: Delete the unused onKvmExitHypercall method.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23746 )


Change subject: arm: Delete the unused onKvmExitHypercall method.
..

arm: Delete the unused onKvmExitHypercall method.

The KVM_EXIT_HYPERCALL KVM exit is now unused, and so even if this
exit handler was plumbed to receive these exits, they would probably
never come.

Change-Id: Ic3ecc789102e761a6dbe80caaf57d61dd95f70a6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23746
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/arch/arm/kvm/arm_cpu.cc
M src/arch/arm/kvm/arm_cpu.hh
2 files changed, 0 insertions(+), 22 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/kvm/arm_cpu.cc b/src/arch/arm/kvm/arm_cpu.cc
index 02e6240..a8b07b9 100644
--- a/src/arch/arm/kvm/arm_cpu.cc
+++ b/src/arch/arm/kvm/arm_cpu.cc
@@ -312,26 +312,6 @@
 updateTCStateMisc();
 }

-Tick
-ArmKvmCPU::onKvmExitHypercall()
-{
-ThreadContext *tc(getContext(0));
-const uint32_t reg_ip(tc->readIntRegFlat(INTREG_R12));
-const uint8_t func((reg_ip >> 8) & 0xFF);
-
-DPRINTF(Kvm, "KVM Hypercall: %#x/%#x\n", func, subfunc);
-const uint64_t ret =
-PseudoInst::pseudoInst(getContext(0), func);
-
-// Just set the return value using the KVM API instead of messing
-// with the context. We could have used the context, but that
-// would have required us to request a full context sync.
-setOneReg(REG_CORE32(usr_regs.ARM_r0), ret & 0x);
-setOneReg(REG_CORE32(usr_regs.ARM_r1), (ret >> 32) & 0x);
-
-return 0;
-}
-
 const ArmKvmCPU::RegIndexVector &
 ArmKvmCPU::getRegList() const
 {
diff --git a/src/arch/arm/kvm/arm_cpu.hh b/src/arch/arm/kvm/arm_cpu.hh
index 16725de..28453d7 100644
--- a/src/arch/arm/kvm/arm_cpu.hh
+++ b/src/arch/arm/kvm/arm_cpu.hh
@@ -92,8 +92,6 @@
 void updateKvmState();
 void updateThreadContext();

-Tick onKvmExitHypercall();
-
 /**
  * Get a list of registers supported by getOneReg() and setOneReg().
  */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23746
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: Ic3ecc789102e761a6dbe80caaf57d61dd95f70a6
Gerrit-Change-Number: 23746
Gerrit-PatchSet: 19
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim: Rework the SyscallDesc to use the dumpSimcall mechanism.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23461 )


Change subject: sim: Rework the SyscallDesc to use the dumpSimcall  
mechanism.

..

sim: Rework the SyscallDesc to use the dumpSimcall mechanism.

This greatly simplifies the doSyscall method, removes a use of
getSyscallArg, and will only print arguments the target syscall is
going to use.

Change-Id: Id8c9c995a2506468fd99fd865f2eb31c40db8b55
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23461
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_desc.cc
M src/sim/syscall_desc.hh
2 files changed, 40 insertions(+), 58 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_desc.cc b/src/sim/syscall_desc.cc
index d0c96b9..6c7fae5 100644
--- a/src/sim/syscall_desc.cc
+++ b/src/sim/syscall_desc.cc
@@ -29,54 +29,22 @@

 #include "sim/syscall_desc.hh"

-#include 
-
-#include "base/trace.hh"
 #include "base/types.hh"
-#include "config/the_isa.hh"
-#include "cpu/base.hh"
-#include "cpu/thread_context.hh"
-#include "sim/faults.hh"
-#include "sim/process.hh"
 #include "sim/syscall_debug_macros.hh"

+class ThreadContext;
+
 void
 SyscallDesc::doSyscall(int callnum, ThreadContext *tc, Fault *fault)
 {
-RegVal arg[6] M5_VAR_USED;
-auto process = tc->getProcessPtr();
+DPRINTF_SYSCALL(Base, "Calling %s...\n", dumper(name(), tc));

-/**
- * Step through the first six parameters for the system call and
- * retrieve their values. Note that index is incremented as a
- * side-effect of the getSyscallArg method.
- */
-int index = 0;
-for (int i = 0; i < 6; i++)
-arg[i] = process->getSyscallArg(tc, index);
-
-/**
- * Linux supports up to six system call arguments through registers
- * so we want to print all six. Check to the relevant man page to
- * verify how many are actually used by a given system call.
- */
-DPRINTF_SYSCALL(Base, "%s called w/arguments %d, %d, %d, %d, %d, %d\n",
-_name, arg[0], arg[1], arg[2], arg[3], arg[4], arg[5]);
-
-/** Invoke the system call */
 SyscallReturn retval = executor(this, callnum, tc);

-/**
- * If the system call needs to be restarted, most likely due to
- * blocking behavior, warn that the system call will retry;
- * alternatively, print the return value.
- */
-if (retval.needsRetry()) {
-*fault = std::make_shared();
-DPRINTF_SYSCALL(Base, "%s needs retry\n", _name);
-} else
-DPRINTF_SYSCALL(Base, "%s returns %d\n", _name,  
retval.encodedValue());

-
-if (!retval.suppressed() && !retval.needsRetry())
-process->setSyscallReturn(tc, retval);
+if (retval.needsRetry())
+DPRINTF_SYSCALL(Base, "Needs retry.\n", name());
+else if (retval.suppressed())
+DPRINTF_SYSCALL(Base, "No return value.\n", name());
+else
+DPRINTF_SYSCALL(Base, "Returned %d.\n", retval.encodedValue());
 }
diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh
index a00b1f3..4f1aa18 100644
--- a/src/sim/syscall_desc.hh
+++ b/src/sim/syscall_desc.hh
@@ -64,13 +64,6 @@
  */
 class SyscallDesc {
   public:
-using SyscallExecutor =
-std::function*)>;

-
-SyscallDesc(const char *name, SyscallExecutor  
sys_exec=unimplementedFunc)

-: _name(name), executor(sys_exec)
-{}
-
 /**
  * Interface for invoking the system call funcion pointer. Note that
  * this acts as a gateway for all system calls and serves a good point
@@ -83,12 +76,22 @@

 std::string name() { return _name; }

+  protected:
+using Executor =
+std::function*)>;
+using Dumper = std::function*)>;

+
+SyscallDesc(const char *name, Executor exec, Dumper dump) :
+_name(name), executor(exec), dumper(dump)
+{}
+
   private:
 /** System call name (e.g., open, mmap, clone, socket, etc.) */
 std::string _name;

 /** Mechanism for ISAs to connect to the emul function definitions */
-SyscallExecutor executor;
+Executor executor;
+Dumper dumper;
 };

 /*
@@ -102,20 +105,20 @@
   private:
 // Aliases to make the code below a little more concise.
 template 
-using SyscallABIExecutor =
+using ABIExecutor =
 std::function;

 template 
-using SyscallABIExecutorPtr =
+using ABIExecutorPtr =
 SyscallReturn (*)(SyscallDesc *, int, ThreadContext *, Args...);


 // Wrap an executor with guest arguments with a normal executor that  
gets

 // those additional arguments from the guest context.
 template 
-static inline SyscallExecutor
-buildExecutor(SyscallABIExecutor target)
+static inline Executor
+buildExecutor(ABIExecutor target)
 {
 return 

[gem5-dev] Change in gem5/gem5[develop]: sim: Use the new returnInto method in cloneFunc.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23504 )


Change subject: sim: Use the new returnInto method in cloneFunc.
..

sim: Use the new returnInto method in cloneFunc.

This gets rid of the final use of setSyscallReturn.

Change-Id: I1108df0c5c72b5dec60128dced48ac0fd0356d24
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23504
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_emul.hh
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index cc07953..2503635 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1529,7 +1529,7 @@

 OS::archClone(flags, p, cp, tc, ctc, newStack, tlsPtr);

-cp->setSyscallReturn(ctc, 0);
+desc->returnInto(ctc, 0);

 #if THE_ISA == SPARC_ISA
 tc->setIntReg(TheISA::SyscallPseudoReturnReg, 0);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23504
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: I1108df0c5c72b5dec60128dced48ac0fd0356d24
Gerrit-Change-Number: 23504
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Gabrielli 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim: Generalize the GuestABI Result::allocate() mechanism.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24103 )


Change subject: sim: Generalize the GuestABI Result::allocate() mechanism.
..

sim: Generalize the GuestABI Result::allocate() mechanism.

This change generalizes the GuestABI Result template family's
allocate() mechanism so that it can also be used with arguments. This
is useful in cases like the 32 bit ARM ISA which says that variadic
functions treat their floating point arguments differently than
non-variadic functions do. This mechanism will give the ABI a chance
to detect that the function is variadic (by checking for VarArgs) and
then to behave differently in that case.

This also makes the GuestABI mechanism a little more regular by
supporting allocate() on both arguments and return values instead of
just return values.

Change-Id: I67050a0ad7ccf15ee75b800dca4eae4fdc0e564e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24103
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/guest_abi.hh
M src/sim/guest_abi.test.cc
2 files changed, 95 insertions(+), 28 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/guest_abi.hh b/src/sim/guest_abi.hh
index ec4e358..99b47b7 100644
--- a/src/sim/guest_abi.hh
+++ b/src/sim/guest_abi.hh
@@ -127,16 +127,25 @@
  * the expected method signature.
  */
 static Arg get(ThreadContext *tc, typename ABI::Position );
-};

+/*
+ * Adjust the position of arguments based on the argument type, if
+ * necessary.
+ *
+ * This method can be excluded if no adjustment is necessary.
+ */
+static void allocate(ThreadContext *tc, typename ABI::Position  
);

+};

 /*
  * This struct template provides a default allocate() method in case the
- * Result template doesn't provide one. This is the default in cases where  
the

- * return type doesn't affect how arguments are laid out.
+ * Result or Argument template doesn't provide one. This is the default in
+ * cases where the return or argument type doesn't affect where things are
+ * stored.
  */
-template 
-struct ResultAllocator
+template  class Role,
+  typename Type, typename Enabled=void>
+struct Allocator
 {
 static void
 allocate(ThreadContext *tc, typename ABI::Position )
@@ -144,20 +153,61 @@
 };

 /*
- * If the return type *does* affect how the arguments are laid out, the ABI
- * can implement an allocate() method for the various return types, and  
this

- * specialization will call into it.
+ * If the return type is void, don't allocate anything.
  */
-template 
-struct ResultAllocatorRet>::allocate)>

+template  class Role>
+struct Allocator
+{
+static void
+allocate(ThreadContext *tc, typename ABI::Position )
+{}
+};
+
+/*
+ * If the return or argument type isn't void and does affect where things
+ * are stored, the ABI can implement an allocate() method for the various
+ * argument and/or return types, and this specialization will call into it.
+ */
+template  class Role, typename Type>
+struct AllocatorType>::allocate)>

 {
 static void
 allocate(ThreadContext *tc, typename ABI::Position )
 {
-Result::allocate(tc, position);
+Role::allocate(tc, position);
 }
 };

+template 
+static void
+allocateResult(ThreadContext *tc, typename ABI::Position )
+{
+Allocator::allocate(tc, position);
+}
+
+template 
+static void
+allocateArguments(ThreadContext *tc, typename ABI::Position )
+{
+return;
+}
+
+template 
+static void
+allocateArguments(ThreadContext *tc, typename ABI::Position )
+{
+Allocator::allocate(tc, position);
+allocateArguments(tc, position);
+}
+
+template 
+static void
+allocateSignature(ThreadContext *tc, typename ABI::Position )
+{
+allocateResult(tc, position);
+allocateArguments(tc, position);
+}
+

 /*
  * These templates implement a variadic argument mechanism for guest ABI
@@ -438,7 +488,7 @@
 // Default construct a Position to track consumed resources. Built in
 // types will be zero initialized.
 auto position = GuestABI::PositionInitializer::init(tc);
-GuestABI::ResultAllocator::allocate(tc, position);
+GuestABI::allocateSignature(tc, position);
 return GuestABI::callFrom(tc, position, target);
 }

@@ -458,6 +508,7 @@
 // Default construct a Position to track consumed resources. Built in
 // types will be zero initialized.
 auto position = GuestABI::PositionInitializer::init(tc);
+GuestABI::allocateArguments(tc, position);
 GuestABI::callFrom(tc, position, target);
 }

@@ -483,7 +534,7 @@
 auto position = GuestABI::PositionInitializer::init(tc);
 std::ostringstream ss;

-GuestABI::ResultAllocator::allocate(tc, position);
+GuestABI::allocateSignature(tc, position);
 ss << name;
 

[gem5-dev] Change in gem5/gem5[develop]: sim: Split up the guest_abi.hh header.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24104 )


Change subject: sim: Split up the guest_abi.hh header.
..

sim: Split up the guest_abi.hh header.

This header was getting pretty long, and could be broken up into a few
headers which logically grouped related definitions and concepts.

To maintain compatibility and keep things simple for users of the
mechanism, there is still a top level header with the original name
which defines the interface for using ABIs. It includes all the other
new headers, and so can also be used when defining ABIs.

Change-Id: I62a051b9bd982e0fcecfceeb3d658d1ff4d30c5e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24104
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/guest_abi.hh
A src/sim/guest_abi/definition.hh
A src/sim/guest_abi/dispatch.hh
A src/sim/guest_abi/layout.hh
A src/sim/guest_abi/varargs.hh
5 files changed, 604 insertions(+), 446 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/guest_abi.hh b/src/sim/guest_abi.hh
index 99b47b7..97d3e21 100644
--- a/src/sim/guest_abi.hh
+++ b/src/sim/guest_abi.hh
@@ -29,452 +29,14 @@
 #define __SIM_GUEST_ABI_HH__

 #include 
-#include 
-#include 
-#include 
+
+#include "sim/guest_abi/definition.hh"
+#include "sim/guest_abi/dispatch.hh"
+#include "sim/guest_abi/layout.hh"
+#include "sim/guest_abi/varargs.hh"

 class ThreadContext;

-namespace GuestABI
-{
-
-/*
- * To implement an ABI, a subclass needs to implement a system of
- * specializations of these two templates Result and Argument, and define a
- * "Position" type.
- *
- * The Position type carries information about, for instance, how many
- * integer registers have been consumed gathering earlier arguments. It
- * may contain multiple elements if there are multiple dimensions to track,
- * for instance the number of integer and floating point registers used so  
far.

- *
- * Result and Argument are class templates instead of function templates so
- * that they can be partially specialized if necessary. C++ doesn't let you
- * partially specialize function templates because that conflicts with
- * template resolution using the function's arguments. Since we already  
know
- * what type we want and we don't need argument based resolution, we can  
just

- * wrap the desired functionality in classes and sidestep the problem.
- *
- * Also note that these templates have an "Enabled" parameter to support
- * std::enable_if style conditional specializations.
- */
-
-/*
- * Position may need to be initialized based on the ThreadContext, for  
instance

- * to find out where the stack pointer is initially.
- */
-template 
-struct PositionInitializer
-{
-static typename ABI::Position
-init(const ThreadContext *tc)
-{
-return typename ABI::Position();
-}
-};
-
-template 
-struct PositionInitializer-std::is_constructible*>::value

->::type>
-{
-static typename ABI::Position
-init(const ThreadContext *tc)
-{
-return typename ABI::Position(tc);
-}
-};
-
-template 
-struct Result
-{
-  private:
-/*
- * Store result "ret" into the state accessible through tc.
- *
- * Note that the declaration below is only to document the expected
- * signature and is private so it won't be used by accident.
- * Specializations of this Result class should define their own version
- * of this method which actually does something and is public.
- */
-static void store(ThreadContext *tc, const Ret );
-
-/*
- * Adjust the position of arguments based on the return type, if  
necessary.

- *
- * This method can be excluded if no adjustment is necessary.
- */
-static void allocate(ThreadContext *tc, typename ABI::Position  
);

-};
-
-/*
- * This partial specialization prevents having to special case 'void' when
- * working with return types.
- */
-template 
-struct Result
-{};
-
-template 
-struct Argument
-{
-/*
- * Retrieve an argument of type Arg from the state accessible through  
tc,

- * assuming the state represented by "position" has already been used.
- * Also update position to account for this argument as well.
- *
- * Like Result::store above, the declaration below is only to document
- * the expected method signature.
- */
-static Arg get(ThreadContext *tc, typename ABI::Position );
-
-/*
- * Adjust the position of arguments based on the argument type, if
- * necessary.
- *
- * This method can be excluded if no adjustment is necessary.
- */
-static void allocate(ThreadContext *tc, typename ABI::Position  
);

-};
-
-/*
- * This struct template provides a default allocate() method in case the
- * Result or Argument template doesn't 

[gem5-dev] Change in gem5/gem5[develop]: sim: Get rid of the now unused getSyscallArg method.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23462 )


Change subject: sim: Get rid of the now unused getSyscallArg method.
..

sim: Get rid of the now unused getSyscallArg method.

Change-Id: I2f78420d8687da7530feb66784fe3e6d2357baf8
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23462
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/arch/arm/freebsd/process.hh
M src/arch/arm/linux/process.hh
M src/arch/arm/process.cc
M src/arch/arm/process.hh
M src/arch/mips/process.cc
M src/arch/mips/process.hh
M src/arch/power/linux/process.cc
M src/arch/power/linux/process.hh
M src/arch/power/process.cc
M src/arch/power/process.hh
M src/arch/riscv/process.cc
M src/arch/riscv/process.hh
M src/arch/sparc/process.cc
M src/arch/sparc/process.hh
M src/arch/x86/process.cc
M src/arch/x86/process.hh
M src/sim/process.cc
M src/sim/process.hh
18 files changed, 0 insertions(+), 157 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



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

index 835acc5..d731b69 100644
--- a/src/arch/arm/freebsd/process.hh
+++ b/src/arch/arm/freebsd/process.hh
@@ -97,9 +97,6 @@

 void syscall(ThreadContext *tc, Fault *fault) override;

-/// Explicitly import the otherwise hidden getSyscallArg
-using ArmProcess::getSyscallArg;
-
 /// A page to hold "kernel" provided functions. The name might be  
wrong.

 static const Addr commPage;

diff --git a/src/arch/arm/linux/process.hh b/src/arch/arm/linux/process.hh
index 8204f43..2edd31e 100644
--- a/src/arch/arm/linux/process.hh
+++ b/src/arch/arm/linux/process.hh
@@ -97,9 +97,6 @@

 void syscall(ThreadContext *tc, Fault *fault) override;

-/// Explicitly import the otherwise hidden getSyscallArg
-using ArmProcess::getSyscallArg;
-
 /// A page to hold "kernel" provided functions. The name might be  
wrong.

 static const Addr commPage;

diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index 7eb263d..b997d6c 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -478,45 +478,6 @@
 0, 1, 2, 3, 4, 5, 6
 };

-RegVal
-ArmProcess32::getSyscallArg(ThreadContext *tc, int )
-{
-assert(i < 6);
-return tc->readIntReg(ArgumentReg0 + i++);
-}
-
-RegVal
-ArmProcess64::getSyscallArg(ThreadContext *tc, int )
-{
-assert(i < 8);
-return tc->readIntReg(ArgumentReg0 + i++);
-}
-
-RegVal
-ArmProcess32::getSyscallArg(ThreadContext *tc, int , int width)
-{
-assert(width == 32 || width == 64);
-if (width == 32)
-return getSyscallArg(tc, i);
-
-// 64 bit arguments are passed starting in an even register
-if (i % 2 != 0)
-   i++;
-
-// Registers r0-r6 can be used
-assert(i < 5);
-uint64_t val;
-val = tc->readIntReg(ArgumentReg0 + i++);
-val |= ((uint64_t)tc->readIntReg(ArgumentReg0 + i++) << 32);
-return val;
-}
-
-RegVal
-ArmProcess64::getSyscallArg(ThreadContext *tc, int , int width)
-{
-return getSyscallArg(tc, i);
-}
-
 void
 ArmProcess32::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
 {
diff --git a/src/arch/arm/process.hh b/src/arch/arm/process.hh
index 1650ff6..4ebe614 100644
--- a/src/arch/arm/process.hh
+++ b/src/arch/arm/process.hh
@@ -86,8 +86,6 @@

   public:

-RegVal getSyscallArg(ThreadContext *tc, int , int width) override;
-RegVal getSyscallArg(ThreadContext *tc, int ) override;
 void setSyscallReturn(ThreadContext *tc,
 SyscallReturn return_value) override;

@@ -135,8 +133,6 @@

   public:

-RegVal getSyscallArg(ThreadContext *tc, int , int width) override;
-RegVal getSyscallArg(ThreadContext *tc, int ) override;
 void setSyscallReturn(ThreadContext *tc,
 SyscallReturn return_value) override;

diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index 93b64ff..f3bb387 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -189,13 +189,6 @@
 }


-RegVal
-MipsProcess::getSyscallArg(ThreadContext *tc, int )
-{
-assert(i < 6);
-return tc->readIntReg(FirstArgumentReg + i++);
-}
-
 void
 MipsProcess::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
 {
diff --git a/src/arch/mips/process.hh b/src/arch/mips/process.hh
index c153979..124dd56 100644
--- a/src/arch/mips/process.hh
+++ b/src/arch/mips/process.hh
@@ -49,9 +49,6 @@
 void argsInit(int pageSize);

   public:
-RegVal getSyscallArg(ThreadContext *tc, int );
-/// Explicitly import the otherwise hidden getSyscallArg
-using Process::getSyscallArg;
 void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);

 struct SyscallABI : public GenericSyscallABI64
diff --git a/src/arch/power/linux/process.cc  
b/src/arch/power/linux/process.cc

index 0653c31..3c74ac4 

[gem5-dev] Change in gem5/gem5[develop]: arch, sim: Get rid of the now unused setSyscallReturn method.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23505 )


Change subject: arch,sim: Get rid of the now unused setSyscallReturn method.
..

arch,sim: Get rid of the now unused setSyscallReturn method.

Change-Id: I61741ab2eca4c77a2c8884e2b5c328479e2b3c90
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23505
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/arch/arm/process.cc
M src/arch/arm/process.hh
M src/arch/mips/process.cc
M src/arch/mips/process.hh
M src/arch/power/process.cc
M src/arch/power/process.hh
M src/arch/riscv/process.cc
M src/arch/riscv/process.hh
M src/arch/sparc/process.cc
M src/arch/sparc/process.hh
M src/arch/x86/process.cc
M src/arch/x86/process.hh
M src/sim/process.hh
13 files changed, 0 insertions(+), 133 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index b997d6c..9cfa431 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -477,41 +477,3 @@
 const std::vector ArmProcess64::SyscallABI::ArgumentRegs = {
 0, 1, 2, 3, 4, 5, 6
 };
-
-void
-ArmProcess32::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
-{
-
-if (objFile->getOpSys() == ObjectFile::FreeBSD) {
-// Decode return value
-if (sysret.encodedValue() >= 0)
-// FreeBSD checks the carry bit to determine if syscall is  
succeeded

-tc->setCCReg(CCREG_C, 0);
-else {
-sysret = -sysret.encodedValue();
-}
-}
-
-tc->setIntReg(ReturnValueReg, sysret.encodedValue());
-if (sysret.count() > 1)
-tc->setIntReg(SyscallPseudoReturnReg, sysret.value2());
-}
-
-void
-ArmProcess64::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
-{
-
-if (objFile->getOpSys() == ObjectFile::FreeBSD) {
-// Decode return value
-if (sysret.encodedValue() >= 0)
-// FreeBSD checks the carry bit to determine if syscall is  
succeeded

-tc->setCCReg(CCREG_C, 0);
-else {
-sysret = -sysret.encodedValue();
-}
-}
-
-tc->setIntReg(ReturnValueReg, sysret.encodedValue());
-if (sysret.count() > 1)
-tc->setIntReg(SyscallPseudoReturnReg, sysret.value2());
-}
diff --git a/src/arch/arm/process.hh b/src/arch/arm/process.hh
index 4ebe614..08100fc 100644
--- a/src/arch/arm/process.hh
+++ b/src/arch/arm/process.hh
@@ -85,10 +85,6 @@
 uint32_t armHwcapImpl() const override;

   public:
-
-void setSyscallReturn(ThreadContext *tc,
-SyscallReturn return_value) override;
-
 struct SyscallABI : public GenericSyscallABI32
 {
 static const std::vector ArgumentRegs;
@@ -132,10 +128,6 @@
 uint32_t armHwcapImpl() const override;

   public:
-
-void setSyscallReturn(ThreadContext *tc,
-SyscallReturn return_value) override;
-
 struct SyscallABI : public GenericSyscallABI64
 {
 static const std::vector ArgumentRegs;
diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index f3bb387..afeef8b 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -188,23 +188,6 @@
 tc->pcState(getStartPC());
 }

-
-void
-MipsProcess::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
-{
-if (sysret.successful()) {
-// no error
-tc->setIntReg(SyscallSuccessReg, 0);
-tc->setIntReg(ReturnValueReg, sysret.returnValue());
-} else {
-// got an error, return details
-tc->setIntReg(SyscallSuccessReg, (uint32_t)(-1));
-tc->setIntReg(ReturnValueReg, sysret.errnoValue());
-}
-if (sysret.count() > 1)
-tc->setIntReg(SyscallPseudoReturnReg, sysret.value2());
-}
-
 const std::vector MipsProcess::SyscallABI::ArgumentRegs = {
 4, 5, 6, 7, 8, 9
 };
diff --git a/src/arch/mips/process.hh b/src/arch/mips/process.hh
index 124dd56..d7b233f 100644
--- a/src/arch/mips/process.hh
+++ b/src/arch/mips/process.hh
@@ -49,8 +49,6 @@
 void argsInit(int pageSize);

   public:
-void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
-
 struct SyscallABI : public GenericSyscallABI64
 {
 static const std::vector ArgumentRegs;
diff --git a/src/arch/power/process.cc b/src/arch/power/process.cc
index dd161fd..3af8788 100644
--- a/src/arch/power/process.cc
+++ b/src/arch/power/process.cc
@@ -269,19 +269,6 @@
 memState->setStackMin(roundDown(stack_min, pageSize));
 }

-void
-PowerProcess::setSyscallReturn(ThreadContext *tc, SyscallReturn sysret)
-{
-Cr cr = tc->readIntReg(INTREG_CR);
-if (sysret.successful()) {
-cr.cr0.so = 0;
-} else {
-cr.cr0.so = 1;
-}
-tc->setIntReg(INTREG_CR, cr);
-tc->setIntReg(ReturnValueReg, sysret.encodedValue());
-}
-
 const 

[gem5-dev] Change in gem5/gem5[develop]: sim: Optionally pass "position" to GuestABI::Result::store.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/24105 )


Change subject: sim: Optionally pass "position" to GuestABI::Result::store.
..

sim: Optionally pass "position" to GuestABI::Result::store.

This will let it get at information about the signature as a whole.

Also, put result storing and argument getting behind functions to hide
some of the templating involved in those mechanisms.

Change-Id: Ib9f26ff69495f8891435f68d3d2f9dfa761a0274
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/24105
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/guest_abi/definition.hh
M src/sim/guest_abi/dispatch.hh
M src/sim/guest_abi/layout.hh
3 files changed, 62 insertions(+), 5 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/guest_abi/definition.hh  
b/src/sim/guest_abi/definition.hh

index ccebcc5..c61d1ae 100644
--- a/src/sim/guest_abi/definition.hh
+++ b/src/sim/guest_abi/definition.hh
@@ -59,7 +59,8 @@
 {
   private:
 /*
- * Store result "ret" into the state accessible through tc.
+ * Store result "ret" into the state accessible through tc. Optionally
+ * accept "position" in case it holds some signature wide information.
  *
  * Note that the declaration below is only to document the expected
  * signature and is private so it won't be used by accident.
@@ -67,6 +68,8 @@
  * of this method which actually does something and is public.
  */
 static void store(ThreadContext *tc, const Ret );
+static void store(ThreadContext *tc, const Ret ,
+  typename ABI::Position );

 /*
  * Adjust the position of arguments based on the return type, if  
necessary.

diff --git a/src/sim/guest_abi/dispatch.hh b/src/sim/guest_abi/dispatch.hh
index c817f63..eb703a5 100644
--- a/src/sim/guest_abi/dispatch.hh
+++ b/src/sim/guest_abi/dispatch.hh
@@ -33,6 +33,7 @@
 #include 

 #include "sim/guest_abi/definition.hh"
+#include "sim/guest_abi/layout.hh"

 class ThreadContext;

@@ -57,7 +58,7 @@
 std::function target)
 {
 Ret ret = target(tc);
-Result::store(tc, ret);
+storeResult(tc, ret, position);
 return ret;
 }

@@ -78,7 +79,7 @@
 std::function target)
 {
 // Extract the next argument from the thread context.
-NextArg next = Argument::get(tc, position);
+NextArg next = getArgument(tc, position);

 // Build a partial function which adds the next argument to the call.
 std::function partial =
@@ -98,7 +99,7 @@
 std::function target)
 {
 // Extract the next argument from the thread context.
-NextArg next = Argument::get(tc, position);
+NextArg next = getArgument(tc, position);

 // Build a partial function which adds the next argument to the call.
 std::function partial =
@@ -139,7 +140,7 @@
 os << (count ? ", " : "(");

 // Extract the next argument from the thread context.
-NextArg next = Argument::get(tc, position);
+NextArg next = getArgument(tc, position);

 // Add this argument to the list.
 os << next;
diff --git a/src/sim/guest_abi/layout.hh b/src/sim/guest_abi/layout.hh
index e7941f8..562f3ee 100644
--- a/src/sim/guest_abi/layout.hh
+++ b/src/sim/guest_abi/layout.hh
@@ -130,6 +130,59 @@
 allocateArguments(tc, position);
 }

+/*
+ * This struct template provides a way to call the Result store method and
+ * optionally pass it the position.
+ */
+
+template 
+struct ResultStorer
+{
+static void
+store(ThreadContext *tc, const Ret , typename ABI::Position  
)

+{
+Result::store(tc, ret);
+}
+};
+
+template 
+std::true_type foo(void (*)(ThreadContext *, const Ret , State  
));

+
+template 
+std::false_type foo(void (*)(ThreadContext *, const Ret ));
+
+template 
+struct ResultStorer::store)>::value>::type>
+{
+static void
+store(ThreadContext *tc, const Ret , typename ABI::Position  
)

+{
+Result::store(tc, ret, position);
+}
+};
+
+/*
+ * Function templates to wrap the Result::store and Argument::get methods.
+ */
+
+template 
+static void
+storeResult(ThreadContext *tc, const Ret ,
+typename ABI::Position )
+{
+ResultStorer::store(tc, ret, position);
+}
+
+template 
+static Arg
+getArgument(ThreadContext *tc, typename ABI::Position )
+{
+return Argument::get(tc, position);
+}
+
 } // namespace GuestABI

 #endif // __SIM_GUEST_ABI_LAYOUT_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/24105
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: Ib9f26ff69495f8891435f68d3d2f9dfa761a0274
Gerrit-Change-Number: 24105
Gerrit-PatchSet: 18
Gerrit-Owner: Gabe Black 

[gem5-dev] Change in gem5/gem5[develop]: arm: Implement the AAPCS64 ABI.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23751 )


Change subject: arm: Implement the AAPCS64 ABI.
..

arm: Implement the AAPCS64 ABI.

This implementation has been tested a tiny bit by intercepting a call
which passed an argument of this type to a function.

struct Test
{
int32_t a;
float *b;
};

The gem5 intercept printed out the value of a, the value of b, and the
value of the float it pointed to.

I was able to get things to work by commenting out the panic in
fixFuncEventAddr and making it return its argument unmodified, and by
calling addFuncEvent instead of addKernelFuncEvent which injects the
kernel symbol table. I substitured the Process's debugSymbolTable which
had the right symbols.

Note that this implementation is not completely correct. First of all,
I used a dummy type in place of the Short Vector type which is just
a byte array with the appropriate alignment forced on it. It sounds
like this type would be something the compiler would need an intrinsic
and architecture specific type for to behave correctly, and so in
gem5 we'd have to define our own type for ARM which could feed in here.

Also, strictly speaking, it sounds like HVA and HFA category of types,
the Homogeneous Short-Vector Aggregates and Homogeneous Floating-point
Aggregates, are supposed to apply to any type which is an aggregate of
all the same type (short vector for one, floating point for the other)
with 4 or fewer members.

In this implementation, I capture any *array* of 4 or fewer elements of
the appropriate type as an HVA or HFA, but I believe these structures
would also count and are not included in my implementation.

struct {
float a;
float b;
float c;
};

struct {
ShortVector a;
ShortVector b;
};

This only matters if those sorts of structures are passed by value as
top level arguments to a function, ie they are not included in some
larger structure.

Also, rule B.6 talks about what to do with an "aignment adjusted type",
and I have no idea what that's supposed to be. Those may not be handled
correctly either.

Change-Id: I5a599a03d38075d7c0a06988c05e7fb5423c68c0
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23751
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
A src/arch/arm/aapcs64.hh
1 file changed, 417 insertions(+), 0 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/arch/arm/aapcs64.hh b/src/arch/arm/aapcs64.hh
new file mode 100644
index 000..16edcb3
--- /dev/null
+++ b/src/arch/arm/aapcs64.hh
@@ -0,0 +1,417 @@
+/*
+ * Copyright 2019 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.
+ */
+
+#ifndef __ARCH_ARM_AAPCS64_HH__
+#define __ARCH_ARM_AAPCS64_HH__
+
+#include 
+#include 
+#include 
+#include 
+
+#include "arch/arm/intregs.hh"
+#include "arch/arm/utility.hh"
+#include "base/intmath.hh"
+#include "cpu/thread_context.hh"
+#include "sim/guest_abi.hh"
+#include "sim/syscall_emul_buf.hh"
+
+class ThreadContext;
+
+struct Aapcs64
+{
+struct Position
+{
+int ngrn=0; // Next general purpose register number.
+int nsrn=0; // Next SIMD and floating point register number.
+Addr nsaa; // Next stacked argument address.
+
+// The maximum allowed general purpose register number.
+static const int MAX_GRN = 7;
+// The 

[gem5-dev] Change in gem5/gem5[develop]: sim: Get rid of the no longer needed DefaultSyscallABI.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23458 )


Change subject: sim: Get rid of the no longer needed DefaultSyscallABI.
..

sim: Get rid of the no longer needed DefaultSyscallABI.

All ISAs now have their own ABI definitions.

Change-Id: I20484b024227658bed7093c232ebf7d64f29bdb6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23458
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_desc.hh
1 file changed, 0 insertions(+), 46 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh
index 8bc3929..a00b1f3 100644
--- a/src/sim/syscall_desc.hh
+++ b/src/sim/syscall_desc.hh
@@ -150,50 +150,4 @@
 using SyscallDesc::SyscallDesc;
 };

-struct DefaultSyscallABI
-{
-using Position = int;
-};
-
-namespace GuestABI
-{
-
-template <>
-struct Result
-{
-static void
-store(ThreadContext *tc, const SyscallReturn )
-{
-auto *process = tc->getProcessPtr();
-process->setSyscallReturn(tc, ret);
-}
-};
-
-template 
-struct Argument::value>::type>
-{
-static Arg
-get(ThreadContext *tc, DefaultSyscallABI::Position )
-{
-auto *process = tc->getProcessPtr();
-return process->getSyscallArg(tc, position);
-}
-};
-
-template 
-struct Argument::value>::type>
-{
-static Arg
-get(ThreadContext *tc, DefaultSyscallABI::Position )
-{
-auto *process = tc->getProcessPtr();
-RegVal reg = process->getSyscallArg(tc, position);
-return (Arg)(uintptr_t)(reg);
-}
-};
-
-} // namespace GuestABI
-
 #endif // __SIM_SYSCALL_DESC_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23458
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: I20484b024227658bed7093c232ebf7d64f29bdb6
Gerrit-Change-Number: 23458
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: kern: Stop using a pseudo instruction to quiesce the ThreadContext.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23748 )


Change subject: kern: Stop using a pseudo instruction to quiesce the  
ThreadContext.

..

kern: Stop using a pseudo instruction to quiesce the ThreadContext.

The pseudo instruction implementation is very short, and so doing the
work it was doing directly doesn't really add much to the
implementation of the udelay events. By not calling the pseudo
instruction we also uncouple these unrelated mechanisms and don't,
for instance, cause pseudo instruction debug output every time udelay
executes.

Change-Id: I5c9b32509562487e53b2acfa1a3f6226d33d1cfd
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23748
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/kern/freebsd/events.cc
M src/kern/linux/events.cc
2 files changed, 5 insertions(+), 8 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/kern/freebsd/events.cc b/src/kern/freebsd/events.cc
index 15954d6..a9e2854 100644
--- a/src/kern/freebsd/events.cc
+++ b/src/kern/freebsd/events.cc
@@ -40,7 +40,6 @@
 #include "debug/DebugPrintf.hh"
 #include "kern/system_events.hh"
 #include "sim/arguments.hh"
-#include "sim/pseudo_inst.hh"
 #include "sim/system.hh"

 namespace FreeBSD {
@@ -69,9 +68,8 @@
 // __delay and __loop_delay functions. One form involves setting  
quiesce

 // time to 0 with the assumption that quiesce will not happen. To avoid
 // the quiesce handling in this case, only execute the quiesce if time  

0.

-if (time > 0) {
-PseudoInst::quiesceNs(tc, time);
-}
+if (time > 0)
+tc->quiesceTick(curTick() + SimClock::Int::ns * time);
 }

 } // namespace FreeBSD
diff --git a/src/kern/linux/events.cc b/src/kern/linux/events.cc
index 199dfc6..3c0cbbf 100644
--- a/src/kern/linux/events.cc
+++ b/src/kern/linux/events.cc
@@ -52,7 +52,7 @@
 #include "kern/linux/printk.hh"
 #include "kern/system_events.hh"
 #include "sim/arguments.hh"
-#include "sim/pseudo_inst.hh"
+#include "sim/core.hh"
 #include "sim/system.hh"

 namespace Linux {
@@ -90,9 +90,8 @@
 // __delay and __loop_delay functions. One form involves setting  
quiesce

 // time to 0 with the assumption that quiesce will not happen. To avoid
 // the quiesce handling in this case, only execute the quiesce if time  

0.

-if (time > 0) {
-PseudoInst::quiesceNs(tc, time);
-}
+if (time > 0)
+tc->quiesceTick(curTick() + SimClock::Int::ns * time);
 }

 void

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23748
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: I5c9b32509562487e53b2acfa1a3f6226d33d1cfd
Gerrit-Change-Number: 23748
Gerrit-PatchSet: 19
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim: Add a returnInto function to the SyscallDesc class.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23503 )


Change subject: sim: Add a returnInto function to the SyscallDesc class.
..

sim: Add a returnInto function to the SyscallDesc class.

This method lets system call implementations return values into
ThreadContexts other than the one they were called from. That's useful
for, for instance, clone() which creates new ThreadContexts.

By making it a virtual function in the SyscallDesc, we can delegate the
actual implementation to the SyscallDescABI subclass which knows the
ABI and how to use it to set the return value.

Change-Id: I61c6e60e4c2a8863c885cd818e4ff053fc3312ee
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23503
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_desc.hh
1 file changed, 12 insertions(+), 0 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_desc.hh b/src/sim/syscall_desc.hh
index 4f1aa18..b8d993b 100644
--- a/src/sim/syscall_desc.hh
+++ b/src/sim/syscall_desc.hh
@@ -76,6 +76,12 @@

 std::string name() { return _name; }

+/**
+ * For use within the system call executor if new threads are created  
and

+ * need something returned into them.
+ */
+virtual void returnInto(ThreadContext *tc, const SyscallReturn ) =  
0;

+
   protected:
 using Executor =
 std::function*)>;

@@ -162,6 +168,12 @@
 SyscallDescABI(const char *name) :
 SyscallDescABI(name, ABIExecutor<>(unimplementedFunc))
 {}
+
+void
+returnInto(ThreadContext *tc, const SyscallReturn ) override
+{
+GuestABI::Result::store(tc, ret);
+}
 };

 #endif // __SIM_SYSCALL_DESC_HH__

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23503
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: I61c6e60e4c2a8863c885cd818e4ff053fc3312ee
Gerrit-Change-Number: 23503
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim: Convert the various flavors of pipe to GuestABI.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23457 )


Change subject: sim: Convert the various flavors of pipe to GuestABI.
..

sim: Convert the various flavors of pipe to GuestABI.

Change-Id: I44aaff417ea6a3ce311208b084fe4013bb93a48e
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23457
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_emul.cc
M src/sim/syscall_emul.hh
2 files changed, 24 insertions(+), 45 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_emul.cc b/src/sim/syscall_emul.cc
index 731cf24..2dbd9d5 100644
--- a/src/sim/syscall_emul.cc
+++ b/src/sim/syscall_emul.cc
@@ -810,19 +810,22 @@
 }

 SyscallReturn
-pipeImpl(SyscallDesc *desc, int callnum, ThreadContext *tc, bool  
pseudo_pipe,

- bool is_pipe2)
+pipePseudoFunc(SyscallDesc *desc, int callnum, ThreadContext *tc)
 {
-Addr tgt_addr = 0;
-int flags = 0;
+return pipe2Func(desc, callnum, tc, 0, 0);
+}
+
+SyscallReturn
+pipeFunc(SyscallDesc *desc, int callnum, ThreadContext *tc, Addr tgt_addr)
+{
+return pipe2Func(desc, callnum, tc, tgt_addr, 0);
+}
+
+SyscallReturn
+pipe2Func(SyscallDesc *desc, int callnum, ThreadContext *tc,
+  Addr tgt_addr, int flags)
+{
 auto p = tc->getProcessPtr();
-if (!pseudo_pipe) {
-int index = 0;
-tgt_addr = p->getSyscallArg(tc, index);
-if (is_pipe2) {
-flags = p->getSyscallArg(tc, index);
-}
-}

 int sim_fds[2], tgt_fds[2];

@@ -847,13 +850,12 @@
 rpfd->setPipeReadSource(tgt_fds[1]);

 /**
- * Alpha Linux convention for pipe() is that fd[0] is returned as
- * the return value of the function, and fd[1] is returned in r20.
+ * On some architectures, it's possible to use more than one register  
for

+ * a return value. In those cases, pipe returns its values rather than
+ * write them into a buffer.
  */
-if (pseudo_pipe) {
-tc->setIntReg(SyscallPseudoReturnReg, tgt_fds[1]);
-return tgt_fds[0];
-}
+if (tgt_addr == 0)
+return SyscallReturn(tgt_fds[0], tgt_fds[1]);

 /**
  * Copy the target file descriptors into buffer space and then copy
@@ -865,8 +867,7 @@
 buf_ptr[1] = tgt_fds[1];
 tgt_handle.copyOut(tc->getVirtProxy());

-// pipe2 has additional behavior if flags != 0
-if (is_pipe2 && flags) {
+if (flags) {
 // pipe2 only uses O_NONBLOCK, O_CLOEXEC, and (O_NONBLOCK |  
O_CLOEXEC)

 // if flags set to anything else, return EINVAL
 if ((flags != O_CLOEXEC) && (flags != O_NONBLOCK) &&
@@ -907,26 +908,6 @@
 }

 SyscallReturn
-pipePseudoFunc(SyscallDesc *desc, int callnum, ThreadContext *tc)
-{
-return pipeImpl(desc, callnum, tc, true);
-}
-
-SyscallReturn
-pipeFunc(SyscallDesc *desc, int callnum, ThreadContext *tc)
-{
-return pipeImpl(desc, callnum, tc, false);
-}
-
-SyscallReturn
-pipe2Func(SyscallDesc *desc, int callnum, ThreadContext *tc)
-{
-// call pipeImpl since the only difference between pipe and pipe2 is
-// the flags values and what they do (at the end of pipeImpl)
-return pipeImpl(desc, callnum, tc, false, true);
-}
-
-SyscallReturn
 getpgrpFunc(SyscallDesc *desc, int callnum, ThreadContext *tc)
 {
 auto process = tc->getProcessPtr();
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 2df5579..cc07953 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -276,14 +276,12 @@
   int tgt_fd, int cmd);

 /// Target pipe() handler.
-SyscallReturn pipeFunc(SyscallDesc *desc, int num, ThreadContext *tc);
-
-/// Internal pipe() handler.
-SyscallReturn pipeImpl(SyscallDesc *desc, int num, ThreadContext *tc,
-   bool pseudo_pipe, bool is_pipe2=false);
+SyscallReturn pipeFunc(SyscallDesc *desc, int num, ThreadContext *tc,
+   Addr tgt_addr);

 /// Target pipe() handler.
-SyscallReturn pipe2Func(SyscallDesc *desc, int num, ThreadContext *tc);
+SyscallReturn pipe2Func(SyscallDesc *desc, int num, ThreadContext *tc,
+Addr tgt_addr, int flags);

 /// Target getpid() handler.
 SyscallReturn getpidFunc(SyscallDesc *desc, int num, ThreadContext *tc);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23457
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: I44aaff417ea6a3ce311208b084fe4013bb93a48e
Gerrit-Change-Number: 23457
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 

[gem5-dev] Change in gem5/gem5[develop]: sim: Clean up some constants used in some syscalls.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23455 )


Change subject: sim: Clean up some constants used in some syscalls.
..

sim: Clean up some constants used in some syscalls.

Having readable constants for these large numbers is good, but they
used incorrect style, were at global scope, and were only used in one
place.

This change centralizes them where they're used, fixes their style, and
rewrites the actual constants in a way that makes it clear what they're
values are.

Change-Id: Ib89c46fce133d4180296d384a61d51d1fe1f8d20
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23455
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/sim/syscall_emul.hh
1 file changed, 9 insertions(+), 10 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 74ffd76..0bc846a 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -508,15 +508,10 @@
 SyscallReturn pipePseudoFunc(SyscallDesc *desc, int num, ThreadContext  
*tc);



-/// A readable name for 1,000,000, for converting microseconds to seconds.
-const int one_million = 100;
-/// A readable name for 1,000,000,000, for converting nanoseconds to  
seconds.

-const int one_billion = 10;
-
 /// Approximate seconds since the epoch (1/1/1970).  About a billion,
 /// by my reckoning.  We want to keep this a constant (not use the
 /// real-world time) to keep simulations repeatable.
-const unsigned seconds_since_epoch = 10;
+const unsigned seconds_since_epoch = 1000 * 1000 * 1000;

 /// Helper function to convert current elapsed time to seconds and
 /// microseconds.
@@ -524,9 +519,11 @@
 void
 getElapsedTimeMicro(T1 , T2 )
 {
+static const int OneMillion = 1000 * 1000;
+
 uint64_t elapsed_usecs = curTick() / SimClock::Int::us;
-sec = elapsed_usecs / one_million;
-usec = elapsed_usecs % one_million;
+sec = elapsed_usecs / OneMillion;
+usec = elapsed_usecs % OneMillion;
 }

 /// Helper function to convert current elapsed time to seconds and
@@ -535,9 +532,11 @@
 void
 getElapsedTimeNano(T1 , T2 )
 {
+static const int OneBillion = 1000 * 1000 * 1000;
+
 uint64_t elapsed_nsecs = curTick() / SimClock::Int::ns;
-sec = elapsed_nsecs / one_billion;
-nsec = elapsed_nsecs % one_billion;
+sec = elapsed_nsecs / OneBillion;
+nsec = elapsed_nsecs % OneBillion;
 }

 //

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23455
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: Ib89c46fce133d4180296d384a61d51d1fe1f8d20
Gerrit-Change-Number: 23455
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Change in gem5/gem5[develop]: sim, gpu: Make ioctl unconditionally take an address parameter.

2020-03-12 Thread Gabe Black (Gerrit)
Gabe Black has submitted this change. (  
https://gem5-review.googlesource.com/c/public/gem5/+/23456 )


Change subject: sim,gpu: Make ioctl unconditionally take an address  
parameter.

..

sim,gpu: Make ioctl unconditionally take an address parameter.

The definition of ioctl is not actually variadic, it just doesn't
specify what the type of the pointer is that it takes as its third
argument. The man page says that that's because it predates void *
being valid C.

By passing this address around (even if it's unused), we avoid having
to extract system call arguments further down the call stack.

Change-Id: I62541237baafaec30bbe3df06b3284dd286a4051
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/23456
Tested-by: kokoro 
Reviewed-by: Bobby R. Bruce 
Maintainer: Gabe Black 
---
M src/gpu-compute/cl_driver.cc
M src/gpu-compute/cl_driver.hh
M src/sim/emul_driver.hh
M src/sim/syscall_emul.hh
4 files changed, 7 insertions(+), 10 deletions(-)

Approvals:
  Bobby R. Bruce: Looks good to me, approved
  Gabe Black: Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/gpu-compute/cl_driver.cc b/src/gpu-compute/cl_driver.cc
index c63856a..d8a4618 100644
--- a/src/gpu-compute/cl_driver.cc
+++ b/src/gpu-compute/cl_driver.cc
@@ -103,11 +103,10 @@
 }

 int
-ClDriver::ioctl(ThreadContext *tc, unsigned req)
+ClDriver::ioctl(ThreadContext *tc, unsigned req, Addr buf_addr)
 {
 int index = 2;
 auto process = tc->getProcessPtr();
-Addr buf_addr = process->getSyscallArg(tc, index);

 switch (req) {
   case HSA_GET_SIZES:
diff --git a/src/gpu-compute/cl_driver.hh b/src/gpu-compute/cl_driver.hh
index 5dbb27d..bc7b749 100644
--- a/src/gpu-compute/cl_driver.hh
+++ b/src/gpu-compute/cl_driver.hh
@@ -54,7 +54,7 @@
 ClDriver(ClDriverParams *p);
 void handshake(GpuDispatcher *_dispatcher);
 int open(ThreadContext *tc, int mode, int flags);
-int ioctl(ThreadContext *tc, unsigned req);
+int ioctl(ThreadContext *tc, unsigned req, Addr buf);
 const char* codeOffToKernelName(uint64_t code_ptr);

   private:
diff --git a/src/sim/emul_driver.hh b/src/sim/emul_driver.hh
index fe13d90..9921d15 100644
--- a/src/sim/emul_driver.hh
+++ b/src/sim/emul_driver.hh
@@ -83,7 +83,7 @@
  * @return The return code for the ioctl, or the negation of the errno
  * (see the SyscallReturn class).
  */
-virtual int ioctl(ThreadContext *tc, unsigned req) = 0;
+virtual int ioctl(ThreadContext *tc, unsigned req, Addr buf) = 0;

 /**
  * Virtual method, invoked when the user program calls mmap() on
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 0bc846a..2df5579 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -702,7 +702,7 @@
 template 
 SyscallReturn
 ioctlFunc(SyscallDesc *desc, int callnum, ThreadContext *tc,
-int tgt_fd, unsigned req, GuestABI::VarArgs varargs)
+int tgt_fd, unsigned req, Addr addr)
 {
 auto p = tc->getProcessPtr();

@@ -715,7 +715,7 @@
 if (dfdp) {
 EmulatedDriver *emul_driver = dfdp->getDriver();
 if (emul_driver)
-return emul_driver->ioctl(tc, req);
+return emul_driver->ioctl(tc, req, addr);
 }

 auto sfdp =  
std::dynamic_pointer_cast((*p->fds)[tgt_fd]);

@@ -724,8 +724,7 @@

 switch (req) {
   case SIOCGIFCONF: {
-Addr conf_addr = varargs.get();
-BufferArg conf_arg(conf_addr, sizeof(ifconf));
+BufferArg conf_arg(addr, sizeof(ifconf));
 conf_arg.copyIn(tc->getVirtProxy());

 ifconf *conf = (ifconf*)conf_arg.bufferPtr();
@@ -754,8 +753,7 @@
   case SIOCGIFHWADDR:
 #endif
   case SIOCGIFMTU: {
-Addr req_addr = varargs.get();
-BufferArg req_arg(req_addr, sizeof(ifreq));
+BufferArg req_arg(addr, sizeof(ifreq));
 req_arg.copyIn(tc->getVirtProxy());

 status = ioctl(sfdp->getSimFD(), req, req_arg.bufferPtr());

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/23456
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: I62541237baafaec30bbe3df06b3284dd286a4051
Gerrit-Change-Number: 23456
Gerrit-PatchSet: 20
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Bobby R. Bruce 
Gerrit-Reviewer: Brandon Potter 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev