[gem5-dev] Change in gem5/gem5[develop]: arch: Simplify and tidy up PCState classes.

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


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: arch: Simplify and tidy up PCState classes.
..

arch: Simplify and tidy up PCState classes.

Change-Id: Ife5412fdd8cc8093371365b8dd4705f77b952191
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52034
Tested-by: kokoro 
Maintainer: Gabe Black 
Reviewed-by: Daniel Carvalho 
---
M src/arch/x86/pcstate.hh
M src/arch/arm/pcstate.hh
M src/arch/power/pcstate.hh
M src/arch/riscv/pcstate.hh
4 files changed, 30 insertions(+), 38 deletions(-)

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




diff --git a/src/arch/arm/pcstate.hh b/src/arch/arm/pcstate.hh
index b6f46ce..b2a88c8 100644
--- a/src/arch/arm/pcstate.hh
+++ b/src/arch/arm/pcstate.hh
@@ -79,22 +79,19 @@
 AArch64Bit = (1 << 2)
 };

-uint8_t flags;
-uint8_t nextFlags;
-uint8_t _itstate;
-uint8_t _nextItstate;
-uint8_t _size;
-bool _illegalExec;
+uint8_t flags = 0;
+uint8_t nextFlags = 0;
+uint8_t _itstate = 0;
+uint8_t _nextItstate = 0;
+uint8_t _size = 0;
+bool _illegalExec = false;

 // Software Step flags
-bool _debugStep;
-bool _stepped;
+bool _debugStep = false;
+bool _stepped = false;

   public:
-PCState() : flags(0), nextFlags(0), _itstate(0), _nextItstate(0),
-_size(0), _illegalExec(false), _debugStep(false),
-_stepped(false)
-{}
+PCState() {}

 void
 set(Addr val)
@@ -103,10 +100,7 @@
 npc(val + (thumb() ? 2 : 4));
 }

-PCState(Addr val) : flags(0), nextFlags(0), _itstate(0),
-_nextItstate(0), _size(0), _illegalExec(false),
-_debugStep(false), _stepped(false)
-{ set(val); }
+PCState(Addr val) { set(val); }

 bool
 illegalExec() const
diff --git a/src/arch/power/pcstate.hh b/src/arch/power/pcstate.hh
index 393c001..7784005 100644
--- a/src/arch/power/pcstate.hh
+++ b/src/arch/power/pcstate.hh
@@ -42,24 +42,10 @@
 class PCState : public GenericISA::SimplePCState<4>
 {
   private:
-typedef GenericISA::SimplePCState<4> Base;
 ByteOrder guestByteOrder = ByteOrder::big;

   public:
-PCState()
-{}
-
-void
-set(Addr val)
-{
-Base::set(val);
-npc(val + 4);
-}
-
-PCState(Addr val)
-{
-set(val);
-}
+using GenericISA::SimplePCState<4>::SimplePCState;

 ByteOrder
 byteOrder() const
diff --git a/src/arch/riscv/pcstate.hh b/src/arch/riscv/pcstate.hh
index 06030b3..0ab2b91 100644
--- a/src/arch/riscv/pcstate.hh
+++ b/src/arch/riscv/pcstate.hh
@@ -53,15 +53,14 @@
 class PCState : public GenericISA::UPCState<4>
 {
   private:
-bool _compressed;
-bool _rv32;
+bool _compressed = false;
+bool _rv32 = false;

   public:
-PCState() : UPCState() { _compressed = false; _rv32 = false; }
-PCState(Addr val) : UPCState(val) { _compressed = false; _rv32 =  
false; }

+using GenericISA::UPCState<4>::UPCState;

 void compressed(bool c) { _compressed = c; }
-bool compressed() { return _compressed; }
+bool compressed() const { return _compressed; }

 void rv32(bool val) { _rv32 = val; }
 bool rv32() const { return _rv32; }
diff --git a/src/arch/x86/pcstate.hh b/src/arch/x86/pcstate.hh
index d04ec2c..077ea15 100644
--- a/src/arch/x86/pcstate.hh
+++ b/src/arch/x86/pcstate.hh
@@ -97,14 +97,14 @@
 }

 void
-serialize(CheckpointOut ) const
+serialize(CheckpointOut ) const override
 {
 Base::serialize(cp);
 SERIALIZE_SCALAR(_size);
 }

 void
-unserialize(CheckpointIn )
+unserialize(CheckpointIn ) override
 {
 Base::unserialize(cp);
 UNSERIALIZE_SCALAR(_size);

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52034
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: Ife5412fdd8cc8093371365b8dd4705f77b952191
Gerrit-Change-Number: 52034
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: dev: Separate generateDeviceTree into a RiscvUart8250 SimObject.

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



Change subject: dev: Separate generateDeviceTree into a RiscvUart8250  
SimObject.

..

dev: Separate generateDeviceTree into a RiscvUart8250 SimObject.

The only difference between the RiscvUart8250 and the regular Uart8250
is that the Riscv version knows how to generate a device tree node
appropriate for use in a Riscv system. This lets us drop the TARGET_ISA
check from that method, since that should be called iff the target
system is Riscv.

Also update the HiFive platform to use the RiscvUart8250 so that it can
continue to generate device trees successfully.

Change-Id: I306596efffed5e5eed337d3db492d2782ebfaa8d
---
M src/dev/riscv/HiFive.py
M src/dev/serial/Uart.py
2 files changed, 33 insertions(+), 15 deletions(-)



diff --git a/src/dev/riscv/HiFive.py b/src/dev/riscv/HiFive.py
index d945590..2923b88 100755
--- a/src/dev/riscv/HiFive.py
+++ b/src/dev/riscv/HiFive.py
@@ -38,7 +38,7 @@
 from m5.objects.Clint import Clint
 from m5.objects.Plic import Plic
 from m5.objects.RTC import RiscvRTC
-from m5.objects.Uart import Uart8250
+from m5.objects.Uart import RiscvUart8250
 from m5.objects.Terminal import Terminal
 from m5.params import *
 from m5.proxy import *
@@ -106,7 +106,7 @@
 plic = Param.Plic(Plic(pio_addr=0xc00), "PLIC")

 # Uart
-uart = Uart8250(pio_addr=0x1000)
+uart = RiscvUart8250(pio_addr=0x1000)
 # Int source ID to redirect console interrupts to
 # Set to 0 if using a pci interrupt for Uart instead
 uart_int_id = Param.Int(0xa, "PLIC Uart interrupt ID")
diff --git a/src/dev/serial/Uart.py b/src/dev/serial/Uart.py
index aecdfe3..aea6fa6 100644
--- a/src/dev/serial/Uart.py
+++ b/src/dev/serial/Uart.py
@@ -67,17 +67,17 @@
 cxx_class = 'gem5::Uart8250'
 pio_size = Param.Addr(0x8, "Size of address range")

+class RiscvUart8250(Uart8250):
 def generateDeviceTree(self, state):
-if buildEnv['TARGET_ISA'] == "riscv":
-node = self.generateBasicPioDeviceNode(
-state, "uart", self.pio_addr, self.pio_size)
-platform = self.platform.unproxy(self)
-plic = platform.plic
-node.append(
-FdtPropertyWords("interrupts", [platform.uart_int_id]))
-node.append(
-FdtPropertyWords("clock-frequency", [0x384000]))
-node.append(
-FdtPropertyWords("interrupt-parent", state.phandle(plic)))
-node.appendCompatible(["ns8250"])
-yield node
+node = self.generateBasicPioDeviceNode(
+state, "uart", self.pio_addr, self.pio_size)
+platform = self.platform.unproxy(self)
+plic = platform.plic
+node.append(
+FdtPropertyWords("interrupts", [platform.uart_int_id]))
+node.append(
+FdtPropertyWords("clock-frequency", [0x384000]))
+node.append(
+FdtPropertyWords("interrupt-parent", state.phandle(plic)))
+node.appendCompatible(["ns8250"])
+yield node

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52144
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: I306596efffed5e5eed337d3db492d2782ebfaa8d
Gerrit-Change-Number: 52144
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] CPU complex wrapper

2021-10-27 Thread Gabe Black via gem5-dev
Hi folks. There are some helper functions in BaseCPU which help set up
ancillary structures like caches, and tries to keep track of the frontier
of ports that need to be connected so that a CPU + caches can be
generically hooked into a system.

This code is a bit clunky and complex, and makes it more difficult to delay
setting up ISA specific components like MMUs and interrupt controllers
which may need to connect to the memory system as well.

I'm thinking that one way to clean this up could be to make a wrapper which
represents the CPU complex as a whole, which can be nested to add new
layers and which would provide a consistent interface no matter how much
extra stuff got layered on.

Importantly, these layers would be set up so that their ports were just a
layer of indirection, and they would not represent extra levels of stuff to
traverse in c++. I think systemc has a concept *roughly* analogous to this
called exports (ex-ports, as opposed to ports? get it?) which let you poke
ports from internal components out of the external interface.

I'm thinking these port repeaters, or port proxies (overloaded term) or
exports, or whatever they're called could be added to the existing
SubSystem container to make a more generic and useful config level wrapper.

class CpuComplex(SubSystem):
inst_ports = VectorPortProxy
data_ports = VectorPortProxy
uncached_ports = VectorPortProxy

class AtomicSimpleCpuComplex(CpuComplex):
cpu = AtomicSimpleCPU
inst_ports = cpu.icache_port
data_ports = cpu.dcache_port

class WithCaches(CpuComplex):
cpu = AtomicSimpleCpuComplex
inst_ports = cpu.inst_ports
data_ports = cpu.data_ports
uncached_ports = cpu.uncached_ports


Something similar to this could generically hold the interrupts object,
etc, which may or may not have certain ports connected, and then if a proxy
has nothing on the other side of it, it could just not actually connect?

There would be some python/config/SimObject/param hacking necessary to make
this work, but I think it would generalize these different sorts of
connections and make this easier to work with.

Ideally in the long run we might not want to have these scripts which
generically support x86, arm, etc, etc, but unless we're prepared to break
all those scripts, we're going to need to keep that working somehow.

Gabe
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem: Fix whitespace in mem/ruby/system/Sequencer.py.

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


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: mem: Fix whitespace in mem/ruby/system/Sequencer.py.
..

mem: Fix whitespace in mem/ruby/system/Sequencer.py.

Some aspects of the formatting in this file were questionable, like
aligning =s between adjacent lines, although not technically against the
style rules as far as I know.

More strangely though, the whole file used three space indents instead
of the typical four.

Change-Id: I7b60f1978c5b2c60a15296b10d09d5701cf7fa5c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52108
Reviewed-by: Jason Lowe-Power 
Maintainer: Jason Lowe-Power 
Tested-by: kokoro 
---
M src/mem/ruby/system/Sequencer.py
1 file changed, 97 insertions(+), 77 deletions(-)

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




diff --git a/src/mem/ruby/system/Sequencer.py  
b/src/mem/ruby/system/Sequencer.py

index ee92308..50e8718 100644
--- a/src/mem/ruby/system/Sequencer.py
+++ b/src/mem/ruby/system/Sequencer.py
@@ -42,100 +42,100 @@
 from m5.objects.ClockedObject import ClockedObject

 class RubyPort(ClockedObject):
-   type = 'RubyPort'
-   abstract = True
-   cxx_header = "mem/ruby/system/RubyPort.hh"
-   cxx_class = 'gem5::ruby::RubyPort'
+type = 'RubyPort'
+abstract = True
+cxx_header = "mem/ruby/system/RubyPort.hh"
+cxx_class = 'gem5::ruby::RubyPort'

-   version = Param.Int(0, "")
+version = Param.Int(0, "")

-   in_ports = VectorResponsePort("CPU side of this RubyPort/Sequencer. "
-   "The CPU request ports should be connected to this. If a  
CPU "
-   "has multiple ports (e.g., I/D ports) all of the ports for  
a "

-   "single CPU can connect to one RubyPort.")
-   slave= DeprecatedParam(in_ports,
-'`slave` is now called `in_ports`')
+in_ports = VectorResponsePort("CPU side of this RubyPort/Sequencer. "
+"The CPU request ports should be connected to this. If a CPU "
+"has multiple ports (e.g., I/D ports) all of the ports for a "
+"single CPU can connect to one RubyPort.")
+slave = DeprecatedParam(in_ports, '`slave` is now called `in_ports`')

-   interrupt_out_port = VectorRequestPort("Port to connect to x86  
interrupt "
-"controller to send the CPU requests from  
outside.")

-   master = DeprecatedParam(interrupt_out_port,
-'`master` is now called `interrupt_out_port`')
+interrupt_out_port = VectorRequestPort("Port to connect to x86  
interrupt "

+"controller to send the CPU requests from outside.")
+master = DeprecatedParam(interrupt_out_port,
+'`master` is now called `interrupt_out_port`')

-   pio_request_port = RequestPort("Ruby pio request port")
-   pio_master_port  = DeprecatedParam(pio_request_port,
-'`pio_master_port` is now called  
`pio_request_port`')

+pio_request_port = RequestPort("Ruby pio request port")
+pio_master_port = DeprecatedParam(pio_request_port,
+'`pio_master_port` is now called `pio_request_port`')

-   mem_request_port = RequestPort("Ruby mem request port")
-   mem_master_port  = DeprecatedParam(mem_request_port,
-'`mem_master_port` is now called  
`mem_request_port`')

+mem_request_port = RequestPort("Ruby mem request port")
+mem_master_port = DeprecatedParam(mem_request_port,
+'`mem_master_port` is now called `mem_request_port`')

-   pio_response_port = ResponsePort("Ruby pio response port")
-   pio_slave_port= DeprecatedParam(pio_response_port,
-'`pio_slave_port` is now called  
`pio_response_port`')

+pio_response_port = ResponsePort("Ruby pio response port")
+pio_slave_port = DeprecatedParam(pio_response_port,
+'`pio_slave_port` is now called `pio_response_port`')

-   using_ruby_tester = Param.Bool(False, "")
-   no_retry_on_stall = Param.Bool(False, "")
-   ruby_system = Param.RubySystem(Parent.any, "")
-   system = Param.System(Parent.any, "system object")
-   support_data_reqs = Param.Bool(True, "data cache requests supported")
-   support_inst_reqs = Param.Bool(True, "inst cache requests supported")
-   is_cpu_sequencer = Param.Bool(True, "connected to a cpu")
+using_ruby_tester = Param.Bool(False, "")
+no_retry_on_stall = Param.Bool(False, "")
+ruby_system = Param.RubySystem(Parent.any, "")
+system = Param.System(Parent.any, "system object")
+support_data_reqs = Param.Bool(True, "data cache requests supported")
+support_inst_reqs = Param.Bool(True, "inst cache requests supported")
+is_cpu_sequencer = 

[gem5-dev] Change in gem5/gem5[develop]: tests, gpu-compute: test dynamic register policy in weekly

2021-10-27 Thread Matt Sinclair (Gerrit) via gem5-dev
Matt Sinclair has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52163 )



Change subject: tests, gpu-compute: test dynamic register policy in weekly
..

tests, gpu-compute: test dynamic register policy in weekly

The GPU models support a simple register allocation policy (1 WF/CU at a
time) and a dynamic register allocation policy (up to max WF/CU at a
time).  By default, the simple policy is used.  However, the dynamic
policy is much more realistic relative to real hardware and thus much
more important to ensure it works in the regressions.  This commit
updates the nightly and weekly regressions accordingly to run the
dynamic register allocation policy.

Change-Id: Id263d3d5e19e4ff47f0eb6d9b08cbafdf2177fb9
---
M tests/weekly.sh
M tests/nightly.sh
2 files changed, 36 insertions(+), 8 deletions(-)



diff --git a/tests/nightly.sh b/tests/nightly.sh
index b3708fd..41db369 100755
--- a/tests/nightly.sh
+++ b/tests/nightly.sh
@@ -83,7 +83,6 @@
 ./main.py run --length long -j${threads} -t${threads}

 # Run the GPU tests.
-
 # For the GPU tests we compile and run GCN3_X86 inside a gcn-gpu container.
 docker pull gcr.io/gem5-test/gcn-gpu:latest
 docker run --rm -u $UID:$GID --volume "${gem5_root}":"${gem5_root}" -w \
@@ -101,7 +100,7 @@
 # basic GPU functionality is working.
 docker run --rm -u $UID:$GID --volume "${gem5_root}":"${gem5_root}" -w \
 "${gem5_root}" gcr.io/gem5-test/gcn-gpu:latest build/GCN3_X86/gem5.opt  
\

-configs/example/apu_se.py -n3 -c square
+configs/example/apu_se.py --reg-alloc-policy=dynamic -n3 -c square

 # get HeteroSync
 wget -qN  
http://dist.gem5.org/dist/develop/test-progs/heterosync/gcn3/allSyncPrims-1kernel

@@ -112,8 +111,8 @@
 # atomics are tested.
 docker run --rm -u $UID:$GID --volume "${gem5_root}":"${gem5_root}" -w \
 "${gem5_root}" gcr.io/gem5-test/gcn-gpu:latest build/GCN3_X86/gem5.opt  
\

-configs/example/apu_se.py -n3  -c allSyncPrims-1kernel \
---options="sleepMutex 10 16 4"
+configs/example/apu_se.py --reg-alloc-policy=dynamic -n3 -c \
+allSyncPrims-1kernel --options="sleepMutex 10 16 4"

 # run HeteroSync LFBarr -- similar setup to sleepMutex above -- 16 WGs
 # accessing unique data and then joining a lock-free barrier, 10 Ld/St per
@@ -122,5 +121,5 @@
 # atomics are tested.
 docker run --rm -u $UID:$GID --volume "${gem5_root}":"${gem5_root}" -w \
 "${gem5_root}" gcr.io/gem5-test/gcn-gpu:latest build/GCN3_X86/gem5.opt  
\

-configs/example/apu_se.py -n3  -c allSyncPrims-1kernel \
---options="lfTreeBarrUniq 10 16 4"
+configs/example/apu_se.py --reg-alloc-policy=dynamic -n3 -c \
+allSyncPrims-1kernel --options="lfTreeBarrUniq 10 16 4"
diff --git a/tests/weekly.sh b/tests/weekly.sh
index 51376bd..172d955 100755
--- a/tests/weekly.sh
+++ b/tests/weekly.sh
@@ -95,7 +95,7 @@
 # stressing several GPU compute and memory components
 docker run --rm -u $UID:$GID --volume "${gem5_root}":"${gem5_root}" -w \
 "${gem5_root}" hacc-test-weekly build/GCN3_X86/gem5.opt \
-configs/example/apu_se.py -n3 --mem-size=8GB \
+configs/example/apu_se.py -n3 --mem-size=8GB  
--reg-alloc-policy=dynamic \
 --benchmark-root="${gem5_root}/gem5-resources/src/gpu/lulesh/bin" -c  
lulesh


 # test DNNMark
@@ -137,6 +137,7 @@
"${gem5_root}/gem5-resources/src/gpu/DNNMark/cachefiles:/root/.cache/miopen/2.9.0"  
\

-w "${gem5_root}/gem5-resources/src/gpu/DNNMark" hacc-test-weekly \
"${gem5_root}/build/GCN3_X86/gem5.opt" "${gem5_root}/configs/example/apu_se.py"  
-n3  
\

+   --reg-alloc-policy=dynamic \
 
--benchmark-root="${gem5_root}/gem5-resources/src/gpu/DNNMark/build/benchmarks/test_fwd_softmax"  
\

-c dnnmark_test_fwd_softmax \
--options="-config  
${gem5_root}/gem5-resources/src/gpu/DNNMark/config_example/softmax_config.dnnmark  
\

@@ -146,6 +147,7 @@
"${gem5_root}/gem5-resources/src/gpu/DNNMark/cachefiles:/root/.cache/miopen/2.9.0"  
\

-w "${gem5_root}/gem5-resources/src/gpu/DNNMark" hacc-test-weekly \
"${gem5_root}/build/GCN3_X86/gem5.opt" "${gem5_root}/configs/example/apu_se.py"  
-n3  
\

+   --reg-alloc-policy=dynamic \
 
--benchmark-root="${gem5_root}/gem5-resources/src/gpu/DNNMark/build/benchmarks/test_fwd_pool"  
\

-c dnnmark_test_fwd_pool \
--options="-config  
${gem5_root}/gem5-resources/src/gpu/DNNMark/config_example/pool_config.dnnmark  
\

@@ -155,6 +157,7 @@
"${gem5_root}/gem5-resources/src/gpu/DNNMark/cachefiles:/root/.cache/miopen/2.9.0"  
\

-w "${gem5_root}/gem5-resources/src/gpu/DNNMark" hacc-test-weekly \
"${gem5_root}/build/GCN3_X86/gem5.opt" "${gem5_root}/configs/example/apu_se.py"  
-n3  
\

+   --reg-alloc-policy=dynamic \
 
--benchmark-root="${gem5_root}/gem5-resources/src/gpu/DNNMark/build/benchmarks/test_bwd_bn"  
\

-c 

[gem5-dev] Change in gem5/gem5[develop]: python: Remove incorrect usage of typing 'Optional'

2021-10-27 Thread Bobby R. Bruce (Gerrit) via gem5-dev
Bobby R. Bruce has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52143 )



Change subject: python: Remove incorrect usage of typing 'Optional'
..

python: Remove incorrect usage of typing 'Optional'

There has been some confusion about usage of 'Optional'. In some areas
of the codebase it was assumed this specifies an optional parameter
(i.e., one which may or may not set, as it has a default value). This is
incorrect. 'Optional[]' is shorthand for 'Union[, None]',
i.e., it is used to state the value may be 'None'. This patch corrects
this throughout the gem5 codebase.

Change-Id: I77a6708dee448e8480870d073e128aed3d6ae904
---
M tests/gem5/x86-boot-tests/test_linux_boot.py
M src/python/gem5/utils/requires.py
M src/python/gem5/resources/downloader.py
M src/python/gem5/components/cachehierarchies/classic/no_cache.py
M src/python/gem5/resources/resource.py
M src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py
M src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py
M  
src/python/gem5/components/cachehierarchies/classic/private_l1_private_l2_cache_hierarchy.py

M src/python/gem5/components/cachehierarchies/classic/caches/mmu_cache.py
M src/python/gem5/components/boards/x86_board.py
M src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py
M  
src/python/gem5/components/cachehierarchies/classic/private_l1_cache_hierarchy.py

12 files changed, 59 insertions(+), 54 deletions(-)



diff --git a/src/python/gem5/components/boards/x86_board.py  
b/src/python/gem5/components/boards/x86_board.py

index e766c3e..2338acca 100644
--- a/src/python/gem5/components/boards/x86_board.py
+++ b/src/python/gem5/components/boards/x86_board.py
@@ -275,7 +275,7 @@
 kernel: AbstractResource,
 disk_image: AbstractResource,
 command: Optional[str] = None,
-kernel_args: Optional[List[str]] = [],
+kernel_args: List[str] = [],
 ):
 """Setup the full system files

diff --git  
a/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py  
b/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py

index 7346e7a..c80032b 100644
--- a/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py
+++ b/src/python/gem5/components/cachehierarchies/classic/caches/l1dcache.py
@@ -28,7 +28,7 @@

 from m5.objects import Cache, BasePrefetcher, StridePrefetcher

-from typing import Optional, Type
+from typing import Type


 class L1DCache(Cache):
@@ -39,13 +39,13 @@
 def __init__(
 self,
 size: str,
-assoc: Optional[int] = 8,
-tag_latency: Optional[int] = 1,
-data_latency: Optional[int] = 1,
-response_latency: Optional[int] = 1,
-mshrs: Optional[int] = 16,
-tgts_per_mshr: Optional[int] = 20,
-writeback_clean: Optional[bool] = True,
+assoc: int = 8,
+tag_latency: int = 1,
+data_latency: int = 1,
+response_latency: int = 1,
+mshrs: int = 16,
+tgts_per_mshr: int = 20,
+writeback_clean: bool = True,
 PrefetcherCls: Type[BasePrefetcher] = StridePrefetcher,
 ):
 super(L1DCache, self).__init__()
diff --git  
a/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py  
b/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py

index d1bf5aa..8e4ba09 100644
--- a/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py
+++ b/src/python/gem5/components/cachehierarchies/classic/caches/l1icache.py
@@ -24,7 +24,7 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

-from typing import Optional, Type
+from typing import Type

 from m5.objects import Cache, BasePrefetcher, StridePrefetcher

@@ -39,13 +39,13 @@
 def __init__(
 self,
 size: str,
-assoc: Optional[int] = 8,
-tag_latency: Optional[int] = 1,
-data_latency: Optional[int] = 1,
-response_latency: Optional[int] = 1,
-mshrs: Optional[int] = 16,
-tgts_per_mshr: Optional[int] = 20,
-writeback_clean: Optional[bool] = True,
+assoc: int = 8,
+tag_latency: int = 1,
+data_latency: int = 1,
+response_latency: int = 1,
+mshrs: int = 16,
+tgts_per_mshr: int = 20,
+writeback_clean: bool = True,
 PrefetcherCls: Type[BasePrefetcher] = StridePrefetcher,
 ):
 super(L1ICache, self).__init__()
diff --git  
a/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py  
b/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py

index f3d7c14..b326255 100644
--- a/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py
+++ b/src/python/gem5/components/cachehierarchies/classic/caches/l2cache.py
@@ -28,7 +28,7 @@

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allowing table descriptor to be inserted in TLB

2021-10-27 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52125 )



Change subject: arch-arm: Allowing table descriptor to be inserted in TLB
..

arch-arm: Allowing table descriptor to be inserted in TLB

This patch is modifying both TableWalker and MMU to effectively
store/use partial translations

* TableWalker changes: If there is a TLB supporting partial
translations (implemented with previous patch), the TableWalker will
craft partial entries and forward them to the TLB as walks are performed

* MMU changes: We now instruct the table walker to start a page
table traversal even if we hit in the TLB, if the matching entry
holds a partial translation

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: Id20aaf4ea02960d50d8345f3e174c698af21ad1c
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/mmu.cc
M src/arch/arm/table_walker.cc
M src/arch/arm/table_walker.hh
3 files changed, 167 insertions(+), 45 deletions(-)



diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index 6b6d10c..f35f665 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -1449,7 +1449,7 @@
 *te = lookup(vaddr, state.asid, state.vmid, state.isHyp, is_secure,  
false,

  false, target_el, false, state.isStage2, mode);

-if (*te == NULL) {
+if (*te == nullptr || (*te)->partial) {
 if (req->isPrefetch()) {
 // if the request is a prefetch don't attempt to fill the TLB  
or go
 // any further with the memory access (here we can safely use  
the

@@ -1469,7 +1469,7 @@
 fault = getTableWalker(mode, state.isStage2)->walk(
 req, tc, state.asid, state.vmid, state.isHyp, mode,
 translation, timing, functional, is_secure,
-tran_type, state.stage2DescReq);
+tran_type, state.stage2DescReq, *te);

 // for timing mode, return and wait for table walk,
 if (timing || fault != NoFault) {
diff --git a/src/arch/arm/table_walker.cc b/src/arch/arm/table_walker.cc
index 96170d3..271f672 100644
--- a/src/arch/arm/table_walker.cc
+++ b/src/arch/arm/table_walker.cc
@@ -286,7 +286,7 @@
   vmid_t _vmid, bool _isHyp, MMU::Mode _mode,
   MMU::Translation *_trans, bool _timing, bool _functional,
   bool secure, MMU::ArmTranslationType tranType,
-  bool _stage2Req)
+  bool _stage2Req, const TlbEntry *walk_entry)
 {
 assert(!(_functional && _timing));
 ++stats.walks;
@@ -340,6 +340,11 @@
 }
 currState->transState = _trans;
 currState->req = _req;
+if (walk_entry) {
+currState->walkEntry = *walk_entry;
+} else {
+currState->walkEntry = TlbEntry();
+}
 currState->fault = NoFault;
 currState->asid = _asid;
 currState->vmid = _vmid;
@@ -892,7 +897,6 @@
 currState->tcr,
 currState->el);

-LookupLevel start_lookup_level = LookupLevel::Num_ArmLookupLevel;
 bool vaddr_fault = false;
 switch (currState->el) {
   case EL0:
@@ -954,11 +958,6 @@
 tsz = 64 - currState->vtcr.t0sz64;
 tg = GrainMap_tg0[currState->vtcr.tg0];

-start_lookup_level = getPageTableOps(tg)->firstS2Level(
-currState->vtcr.sl0);
-
-panic_if(start_lookup_level == LookupLevel::Num_ArmLookupLevel,
- "Cannot discern lookup level from vtcr.{sl0,tg0}");
 ps = currState->vtcr.ps;
 currState->isUncacheable = currState->vtcr.irgn0 == 0;
 } else {
@@ -1091,15 +1090,6 @@
 tg = Grain4KB;
 }

-// Determine starting lookup level
-if (start_lookup_level == LookupLevel::Num_ArmLookupLevel) {
-const auto* ptops = getPageTableOps(tg);
-
-start_lookup_level = ptops->firstLevel(64 - tsz);
-panic_if(start_lookup_level == LookupLevel::Num_ArmLookupLevel,
- "Table walker couldn't find lookup level\n");
-}
-
 // Clamp to lower limit
 int pa_range = decodePhysAddrRange64(ps);
 if (pa_range > _physAddrRange) {
@@ -1108,22 +1098,12 @@
 currState->physAddrRange = pa_range;
 }

-// Determine table base address
-int stride = tg - 3;
-int base_addr_lo = 3 + tsz - stride * (3 - start_lookup_level) - tg;
-Addr base_addr = 0;
-
-if (pa_range == 52) {
-int z = (base_addr_lo < 6) ? 6 : base_addr_lo;
-base_addr = mbits(ttbr, 47, z);
-base_addr |= (bits(ttbr, 5, 2) << 48);
-} else {
-base_addr = mbits(ttbr, 47, base_addr_lo);
-}
+auto [table_addr, desc_addr, start_lookup_level] = walkAddresses(
+ttbr, tg, tsz, pa_range);

 // Determine physical address size and raise an Address Size Fault if
 // necessary
-if (checkAddrSizeFaultAArch64(base_addr, currState->physAddrRange)) {
+if 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allow TLB to be used as a WalkCache

2021-10-27 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52124 )



Change subject: arch-arm: Allow TLB to be used as a WalkCache
..

arch-arm: Allow TLB to be used as a WalkCache

This patch allows partial translation entries (intermediate PAs obtained
from a table walk) to be stored in an ArmTLB. This effectively means
reserving a fraction of the TLB entries to cache table walks

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: Id0efb7d75dd017366c4c3b74de7b57355a53a01a
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/ArmTLB.py
M src/arch/arm/mmu.cc
M src/arch/arm/mmu.hh
M src/arch/arm/tlb.cc
M src/arch/arm/tlb.hh
5 files changed, 185 insertions(+), 37 deletions(-)



diff --git a/src/arch/arm/ArmTLB.py b/src/arch/arm/ArmTLB.py
index 4c86f72..10ed48b 100644
--- a/src/arch/arm/ArmTLB.py
+++ b/src/arch/arm/ArmTLB.py
@@ -51,6 +51,11 @@
 size = Param.Int(64, "TLB size")
 is_stage2 = Param.Bool(False, "Is this a stage 2 TLB?")

+partial_levels = VectorParam.ArmLookupLevel([],
+"List of intermediate lookup levels allowed to be cached in the  
TLB "

+"(=holding intermediate PAs obtained during a table walk")
+
+
 class ArmStage2TLB(ArmTLB):
 size = 32
 is_stage2 = True
diff --git a/src/arch/arm/mmu.cc b/src/arch/arm/mmu.cc
index ce052f1..6b6d10c 100644
--- a/src/arch/arm/mmu.cc
+++ b/src/arch/arm/mmu.cc
@@ -65,7 +65,7 @@
 test(nullptr),
 miscRegContext(0),
 s1State(this, false), s2State(this, true),
-_attr(0),
+_attr(0), _hasWalkCache(false),
 stats(this)
 {
 // Cache system-level properties
@@ -101,6 +101,27 @@
 getDTBPtr()->setTableWalker(dtbWalker);

 BaseMMU::init();
+
+_hasWalkCache = checkWalkCache();
+}
+
+bool
+MMU::checkWalkCache() const
+{
+for (auto tlb : instruction) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+for (auto tlb : data) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+for (auto tlb : unified) {
+if (static_cast(tlb)->walkCache())
+return true;
+}
+
+return false;
 }

 void
diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh
index 2d0ef7b..7fa8210 100644
--- a/src/arch/arm/mmu.hh
+++ b/src/arch/arm/mmu.hh
@@ -342,6 +342,8 @@
 _attr = attr;
 }

+bool hasWalkCache() const { return _hasWalkCache; }
+
 /**
  * Determine the EL to use for the purpose of a translation given
  * a specific translation type. If the translation type doesn't
@@ -423,6 +425,13 @@
LookupLevel lookup_level, CachedState );

   protected:
+bool checkWalkCache() const;
+
+CachedState& updateMiscReg(
+ThreadContext *tc, ArmTranslationType tran_type,
+bool stage2);
+
+  protected:
 ContextID miscRegContext;

   public:
@@ -439,9 +448,7 @@

 AddrRange m5opRange;

-CachedState& updateMiscReg(
-ThreadContext *tc, ArmTranslationType tran_type,
-bool stage2);
+bool _hasWalkCache;

 struct Stats : public statistics::Group
 {
diff --git a/src/arch/arm/tlb.cc b/src/arch/arm/tlb.cc
index e2897f8..a7c3f12 100644
--- a/src/arch/arm/tlb.cc
+++ b/src/arch/arm/tlb.cc
@@ -61,9 +61,33 @@
 TLB::TLB(const ArmTLBParams )
 : BaseTLB(p), table(new TlbEntry[p.size]), size(p.size),
   isStage2(p.is_stage2),
+  _walkCache(false),
   tableWalker(nullptr),
   stats(*this), rangeMRU(1), vmid(0)
 {
+for (int lvl = LookupLevel::L0;
+ lvl < LookupLevel::Num_ArmLookupLevel; lvl++) {
+
+auto it = std::find(
+p.partial_levels.begin(),
+p.partial_levels.end(),
+lvl);
+
+auto lookup_lvl = static_cast(lvl);
+
+if (it != p.partial_levels.end()) {
+// A partial entry from of the current LookupLevel can be
+// cached within the TLB
+partialLevels[lookup_lvl] = true;
+
+// Make sure this is not the last level (complete translation)
+if (lvl != LookupLevel::Num_ArmLookupLevel - 1) {
+_walkCache = true;
+}
+} else {
+partialLevels[lookup_lvl] = false;
+}
+}
 }

 TLB::~TLB()
@@ -79,32 +103,63 @@
 }

 TlbEntry*
-TLB::lookup(const Lookup _data)
+TLB::match(const Lookup _data)
 {
-TlbEntry *retval = NULL;
-const auto functional = lookup_data.functional;
-const auto mode = lookup_data.mode;
+// Vector of TLB entry candidates.
+// Only one of them will be assigned to retval and will
+// be returned to the MMU (in case of a hit)
+// The vector has one entry per lookup level as it stores
+// both complete and partial matches
+std::vector> hits{
+LookupLevel::Num_ArmLookupLevel, {0, nullptr}};

-// Maintaining LRU array
 int x = 0;
-while (retval == NULL && x < size) {
+

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Allow the L2 unified TLB to store partial translations

2021-10-27 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52126 )



Change subject: arch-arm: Allow the L2 unified TLB to store partial  
translations

..

arch-arm: Allow the L2 unified TLB to store partial translations

We are allowing the L2 TLB to store partial translations from the
second level of lookup

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: I1286c14a256470c2075fe5533930617139d4d087
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/ArmMMU.py
1 file changed, 17 insertions(+), 1 deletion(-)



diff --git a/src/arch/arm/ArmMMU.py b/src/arch/arm/ArmMMU.py
index d32cbff..0374f35 100644
--- a/src/arch/arm/ArmMMU.py
+++ b/src/arch/arm/ArmMMU.py
@@ -64,7 +64,8 @@
 cxx_header = 'arch/arm/mmu.hh'

 # L2 TLBs
-l2_shared = ArmTLB(entry_type="unified", size=1280)
+l2_shared = ArmTLB(entry_type="unified", size=1280,
+partial_levels=["L2"])

 # L1 TLBs
 itb = ArmTLB(entry_type="instruction", next_level=Parent.l2_shared)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52126
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: I1286c14a256470c2075fe5533930617139d4d087
Gerrit-Change-Number: 52126
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Add partial param to TlbEntry

2021-10-27 Thread Giacomo Travaglini (Gerrit) via gem5-dev
Giacomo Travaglini has uploaded this change for review. (  
https://gem5-review.googlesource.com/c/public/gem5/+/52123 )



Change subject: arch-arm: Add partial param to TlbEntry
..

arch-arm: Add partial param to TlbEntry

The boolean will flag a partial translation

JIRA: https://gem5.atlassian.net/browse/GEM5-1108

Change-Id: Ia27e321cd5c4b062cafcfd0dc39a208a4d5e3247
Signed-off-by: Giacomo Travaglini 
---
M src/arch/arm/pagetable.hh
1 file changed, 18 insertions(+), 1 deletion(-)



diff --git a/src/arch/arm/pagetable.hh b/src/arch/arm/pagetable.hh
index 4cfdf2a..8300175 100644
--- a/src/arch/arm/pagetable.hh
+++ b/src/arch/arm/pagetable.hh
@@ -244,6 +244,8 @@
 // This is used to distinguish between instruction and data entries
 // in unified TLBs
 TypeTLB type;
+// True if the entry is caching a partial translation (a table walk)
+bool partial;

 // Type of memory
 bool nonCacheable; // Can we wrap this in mtype?
@@ -266,6 +268,7 @@
  domain(DomainType::Client),  mtype(MemoryType::StronglyOrdered),
  longDescFormat(false), isHyp(false), global(false), valid(true),
  ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
+ partial(false),
  nonCacheable(uncacheable),
  shareable(false), outerShareable(false), xn(0), pxn(0)
 {
@@ -283,7 +286,7 @@
  domain(DomainType::Client), mtype(MemoryType::StronglyOrdered),
  longDescFormat(false), isHyp(false), global(false), valid(false),
  ns(true), nstid(true), el(EL0), type(TypeTLB::unified),
- nonCacheable(false),
+ partial(false), nonCacheable(false),
  shareable(false), outerShareable(false), xn(0), pxn(0)
 {
 // no restrictions by default, hap = 0x3

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52123
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: Ia27e321cd5c4b062cafcfd0dc39a208a4d5e3247
Gerrit-Change-Number: 52123
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem-ruby: Don't conditionalize setting RubySequencer's pio_response_port

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



Change subject: mem-ruby: Don't conditionalize setting RubySequencer's  
pio_response_port

..

mem-ruby: Don't conditionalize setting RubySequencer's pio_response_port

This was conditioned on the TARGET_ISA being x86 because the code it
replaced was, and that was because the x86 interrupts object had an
extra port that didn't appear for other ISAs. This inconsistency is not
present on either side of this connection, and so we don't need it to be
conditional.

We do, however, need to ensure that the port sends a range change even
if it doesn't have any ranges to send, to satisfy the bookkeeping of the
bus on the other side of the connection. We do that in init, like leaf
devices do.

Change-Id: Idec6f6c5e2cf78b113fb238d0edd2c63d6cd2c23
---
M src/mem/ruby/system/RubyPort.cc
M src/mem/ruby/system/Sequencer.py
2 files changed, 24 insertions(+), 2 deletions(-)



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

index ff02fde..37c34c5 100644
--- a/src/mem/ruby/system/RubyPort.cc
+++ b/src/mem/ruby/system/RubyPort.cc
@@ -95,6 +95,9 @@
 m_mandatory_q_ptr = m_controller->getMandatoryQueue();
 for (const auto _port : response_ports)
 response_port->sendRangeChange();
+if (gotAddrRanges == 0 && FullSystem) {
+pioResponsePort.sendRangeChange();
+}
 }

 Port &
diff --git a/src/mem/ruby/system/Sequencer.py  
b/src/mem/ruby/system/Sequencer.py

index 50e8718..53e4f9c 100644
--- a/src/mem/ruby/system/Sequencer.py
+++ b/src/mem/ruby/system/Sequencer.py
@@ -125,8 +125,7 @@
 import m5.defines
 self.pio_request_port = piobus.cpu_side_ports
 self.mem_request_port = piobus.cpu_side_ports
-if m5.defines.buildEnv['TARGET_ISA'] == "x86":
-self.pio_response_port = piobus.mem_side_ports
+self.pio_response_port = piobus.mem_side_ports

 class RubyHTMSequencer(RubySequencer):
 type = 'RubyHTMSequencer'

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52109
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: Idec6f6c5e2cf78b113fb238d0edd2c63d6cd2c23
Gerrit-Change-Number: 52109
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: sim: Remove some old transitional code in SEWorkload.

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



Change subject: sim: Remove some old transitional code in SEWorkload.
..

sim: Remove some old transitional code in SEWorkload.

This code was just to catch cases where the SEWorkload init_compatible
function couldn't find a compatible SEWorkload subclass. Now that all of
these classes are set up with this mechanism, there's no need to keep
this code around.

Change-Id: Ie847f5a90ccf98eb58c149a22a6881529344946d
---
M src/sim/Workload.py
1 file changed, 14 insertions(+), 4 deletions(-)



diff --git a/src/sim/Workload.py b/src/sim/Workload.py
index c881447..ad460b1 100644
--- a/src/sim/Workload.py
+++ b/src/sim/Workload.py
@@ -102,10 +102,6 @@
 if len(options) > 1:
 raise ValueError("More than one SE workload is compatible  
with %s")

 elif len(options) < 1:
-# For now, fall back to the base class if there are no matches.
-# After we've had a chance to implement everything, this  
default
-# can be removed since this should always find exactly one  
match.

-return SEWorkload(*args, **kwargs)
 raise ValueError("No SE workload is compatible with %s", path)

 return options[0](*args, **kwargs)

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52107
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: Ie847f5a90ccf98eb58c149a22a6881529344946d
Gerrit-Change-Number: 52107
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: sim: Remove the byte_order parameter from System.

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



Change subject: sim: Remove the byte_order parameter from System.
..

sim: Remove the byte_order parameter from System.

Instead, get the byte order from the workload. The workload has a better
idea what the byte order should be, for instance based on what software
it's loaded or how the hardware was configured, and this gets rid of a
use of TARGET_ISA which was setting a default endianness.

Change-Id: Ic5d8a6f69a664957c4f837e3799ff93397ccfc64
---
M src/sim/system.hh
M src/sim/System.py
2 files changed, 15 insertions(+), 9 deletions(-)



diff --git a/src/sim/System.py b/src/sim/System.py
index 115fb94..596e25c 100644
--- a/src/sim/System.py
+++ b/src/sim/System.py
@@ -49,11 +49,6 @@
 class MemoryMode(Enum): vals = ['invalid', 'atomic', 'timing',
 'atomic_noncaching']

-if buildEnv['TARGET_ISA'] in ('sparc', 'power'):
-default_byte_order = 'big'
-else:
-default_byte_order = 'little'
-
 class System(SimObject):
 type = 'System'
 cxx_header = "sim/system.hh"
@@ -96,9 +91,6 @@

 cache_line_size = Param.Unsigned(64, "Cache line size in bytes")

-byte_order = Param.ByteOrder(default_byte_order,
- "Default byte order of system components")
-
 redirect_paths = VectorParam.RedirectPath([], "Path redirections")

 exit_on_work_items = Param.Bool(False, "Exit from the simulation loop  
when "

diff --git a/src/sim/system.hh b/src/sim/system.hh
index 7aeeea7..8f09b96 100644
--- a/src/sim/system.hh
+++ b/src/sim/system.hh
@@ -388,7 +388,7 @@
 ByteOrder
 getGuestByteOrder() const
 {
-return params().byte_order;
+return workload->byteOrder();
 }

 /**

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52106
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: Ic5d8a6f69a664957c4f837e3799ff93397ccfc64
Gerrit-Change-Number: 52106
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: mem: Fix whitespace in mem/ruby/system/Sequencer.py.

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



Change subject: mem: Fix whitespace in mem/ruby/system/Sequencer.py.
..

mem: Fix whitespace in mem/ruby/system/Sequencer.py.

Some aspects of the formatting in this file were questionable, like
aligning =s between adjacent lines, although not technically against the
style rules as far as I know.

More strangely though, the whole file used three space indents instead
of the typical four.

Change-Id: I7b60f1978c5b2c60a15296b10d09d5701cf7fa5c
---
M src/mem/ruby/system/Sequencer.py
1 file changed, 93 insertions(+), 77 deletions(-)



diff --git a/src/mem/ruby/system/Sequencer.py  
b/src/mem/ruby/system/Sequencer.py

index ee92308..50e8718 100644
--- a/src/mem/ruby/system/Sequencer.py
+++ b/src/mem/ruby/system/Sequencer.py
@@ -42,100 +42,100 @@
 from m5.objects.ClockedObject import ClockedObject

 class RubyPort(ClockedObject):
-   type = 'RubyPort'
-   abstract = True
-   cxx_header = "mem/ruby/system/RubyPort.hh"
-   cxx_class = 'gem5::ruby::RubyPort'
+type = 'RubyPort'
+abstract = True
+cxx_header = "mem/ruby/system/RubyPort.hh"
+cxx_class = 'gem5::ruby::RubyPort'

-   version = Param.Int(0, "")
+version = Param.Int(0, "")

-   in_ports = VectorResponsePort("CPU side of this RubyPort/Sequencer. "
-   "The CPU request ports should be connected to this. If a  
CPU "
-   "has multiple ports (e.g., I/D ports) all of the ports for  
a "

-   "single CPU can connect to one RubyPort.")
-   slave= DeprecatedParam(in_ports,
-'`slave` is now called `in_ports`')
+in_ports = VectorResponsePort("CPU side of this RubyPort/Sequencer. "
+"The CPU request ports should be connected to this. If a CPU "
+"has multiple ports (e.g., I/D ports) all of the ports for a "
+"single CPU can connect to one RubyPort.")
+slave = DeprecatedParam(in_ports, '`slave` is now called `in_ports`')

-   interrupt_out_port = VectorRequestPort("Port to connect to x86  
interrupt "
-"controller to send the CPU requests from  
outside.")

-   master = DeprecatedParam(interrupt_out_port,
-'`master` is now called `interrupt_out_port`')
+interrupt_out_port = VectorRequestPort("Port to connect to x86  
interrupt "

+"controller to send the CPU requests from outside.")
+master = DeprecatedParam(interrupt_out_port,
+'`master` is now called `interrupt_out_port`')

-   pio_request_port = RequestPort("Ruby pio request port")
-   pio_master_port  = DeprecatedParam(pio_request_port,
-'`pio_master_port` is now called  
`pio_request_port`')

+pio_request_port = RequestPort("Ruby pio request port")
+pio_master_port = DeprecatedParam(pio_request_port,
+'`pio_master_port` is now called `pio_request_port`')

-   mem_request_port = RequestPort("Ruby mem request port")
-   mem_master_port  = DeprecatedParam(mem_request_port,
-'`mem_master_port` is now called  
`mem_request_port`')

+mem_request_port = RequestPort("Ruby mem request port")
+mem_master_port = DeprecatedParam(mem_request_port,
+'`mem_master_port` is now called `mem_request_port`')

-   pio_response_port = ResponsePort("Ruby pio response port")
-   pio_slave_port= DeprecatedParam(pio_response_port,
-'`pio_slave_port` is now called  
`pio_response_port`')

+pio_response_port = ResponsePort("Ruby pio response port")
+pio_slave_port = DeprecatedParam(pio_response_port,
+'`pio_slave_port` is now called `pio_response_port`')

-   using_ruby_tester = Param.Bool(False, "")
-   no_retry_on_stall = Param.Bool(False, "")
-   ruby_system = Param.RubySystem(Parent.any, "")
-   system = Param.System(Parent.any, "system object")
-   support_data_reqs = Param.Bool(True, "data cache requests supported")
-   support_inst_reqs = Param.Bool(True, "inst cache requests supported")
-   is_cpu_sequencer = Param.Bool(True, "connected to a cpu")
+using_ruby_tester = Param.Bool(False, "")
+no_retry_on_stall = Param.Bool(False, "")
+ruby_system = Param.RubySystem(Parent.any, "")
+system = Param.System(Parent.any, "system object")
+support_data_reqs = Param.Bool(True, "data cache requests supported")
+support_inst_reqs = Param.Bool(True, "inst cache requests supported")
+is_cpu_sequencer = Param.Bool(True, "connected to a cpu")

 class RubyPortProxy(RubyPort):
-   type = 'RubyPortProxy'
-   cxx_header = "mem/ruby/system/RubyPortProxy.hh"
-   cxx_class = 'gem5::ruby::RubyPortProxy'
+type = 'RubyPortProxy'
+cxx_header = "mem/ruby/system/RubyPortProxy.hh"
+cxx_class = 'gem5::ruby::RubyPortProxy'

 class RubySequencer(RubyPort):
-   type = 'RubySequencer'
-   cxx_class 

[gem5-dev] Change in gem5/gem5[develop]: dev: Get the system's byte order for VirtIO using c++.

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



Change subject: dev: Get the system's byte order for VirtIO using c++.
..

dev: Get the system's byte order for VirtIO using c++.

We already have a reference to the system object, so we don't need to
take that as a separate explicit parameter.

Change-Id: Ie8de22541d409f2b2e5544237f472dae6714b437
---
M src/dev/virtio/VirtIO.py
M src/dev/virtio/base.cc
2 files changed, 14 insertions(+), 2 deletions(-)



diff --git a/src/dev/virtio/VirtIO.py b/src/dev/virtio/VirtIO.py
index b0ddb0e..7ecf636 100644
--- a/src/dev/virtio/VirtIO.py
+++ b/src/dev/virtio/VirtIO.py
@@ -51,7 +51,6 @@
 subsystem = Param.UInt8(0x00, "VirtIO subsystem ID")

 system = Param.System(Parent.any, "system object")
-byte_order = Param.ByteOrder(Parent.byte_order, "Device byte order")

 class VirtIODummyDevice(VirtIODeviceBase):
 type = 'VirtIODummyDevice'
diff --git a/src/dev/virtio/base.cc b/src/dev/virtio/base.cc
index f528cea..afc1e97 100644
--- a/src/dev/virtio/base.cc
+++ b/src/dev/virtio/base.cc
@@ -42,6 +42,7 @@
 #include "params/VirtIODeviceBase.hh"
 #include "params/VirtIODummyDevice.hh"
 #include "sim/serialize.hh"
+#include "sim/system.hh"

 namespace gem5
 {
@@ -340,7 +341,7 @@
size_t config_size, FeatureBits  
features)

 : SimObject(params),
   guestFeatures(0),
-  byteOrder(params.byte_order),
+  byteOrder(params.system->getGuestByteOrder()),
   deviceId(id), configSize(config_size), deviceFeatures(features),
   _deviceStatus(0), _queueSelect(0)
 {

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52105
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: Ie8de22541d409f2b2e5544237f472dae6714b437
Gerrit-Change-Number: 52105
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black 
Gerrit-MessageType: newchange
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch,sim: Add a byteOrder accessor to the Workload class.

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



Change subject: arch,sim: Add a byteOrder accessor to the Workload class.
..

arch,sim: Add a byteOrder accessor to the Workload class.

The workload would have a better idea of what it's endianness is than
the system object that holds it. This is the first step towards getting
rid of the getByteOrder method on the system object, which currently
checks TARGET_ISA to determine what the default endianness should be.

If it makes sense for a Workload, it could determine the endianness
dynamically by, for instance, reading it out of a binary image before
putting it into memory.

This does assume that the workload has a consistent endianness
throughout which may not be true, but this is not a new assumption.

Also, mark the SEWorkload SimObject class as "abstract", since it isn't
useful until they get subclassed by some arch specific version.

Change-Id: I8d4ba8382f22236a81f9738cc3506cdb97bdbfb2
---
M src/arch/mips/linux/se_workload.hh
M src/arch/riscv/bare_metal/fs_workload.hh
M src/sim/workload.hh
M src/arch/x86/linux/se_workload.hh
M src/arch/power/linux/se_workload.hh
M src/arch/riscv/linux/se_workload.hh
M src/sim/kernel_workload.hh
M src/arch/riscv/linux/fs_workload.hh
M src/arch/sparc/linux/se_workload.hh
M src/arch/sparc/fs_workload.hh
M src/arch/arm/freebsd/se_workload.hh
M src/arch/arm/fs_workload.hh
M src/arch/x86/fs_workload.hh
M src/arch/arm/linux/se_workload.hh
M src/sim/Workload.py
15 files changed, 46 insertions(+), 0 deletions(-)



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

index a7f3453..8069bd2 100644
--- a/src/arch/arm/freebsd/se_workload.hh
+++ b/src/arch/arm/freebsd/se_workload.hh
@@ -55,6 +55,8 @@

 EmuFreebsd(const Params ) : SEWorkload(p, PageShift) {}

+ByteOrder byteOrder() const override { return ByteOrder::little; }
+
 struct BaseSyscallABI {};
 struct SyscallABI32 : public SEWorkload::SyscallABI32,
   public BaseSyscallABI
diff --git a/src/arch/arm/fs_workload.hh b/src/arch/arm/fs_workload.hh
index c917ffd..547bbf1 100644
--- a/src/arch/arm/fs_workload.hh
+++ b/src/arch/arm/fs_workload.hh
@@ -143,6 +143,8 @@
 return loader::Arm64;
 }

+ByteOrder byteOrder() const override { return ByteOrder::little; }
+
 FsWorkload(const Params );

 void initState() override;
diff --git a/src/arch/arm/linux/se_workload.hh  
b/src/arch/arm/linux/se_workload.hh

index 0ff08c7..b22688f 100644
--- a/src/arch/arm/linux/se_workload.hh
+++ b/src/arch/arm/linux/se_workload.hh
@@ -47,6 +47,7 @@
 using Params = ArmEmuLinuxParams;

 EmuLinux(const Params ) : SEWorkload(p, PageShift) {}
+ByteOrder byteOrder() const override { return ByteOrder::little; }

 struct BaseSyscallABI {};
 struct SyscallABI32 : public SEWorkload::SyscallABI32,
diff --git a/src/arch/mips/linux/se_workload.hh  
b/src/arch/mips/linux/se_workload.hh

index c94112c..7e4d863 100644
--- a/src/arch/mips/linux/se_workload.hh
+++ b/src/arch/mips/linux/se_workload.hh
@@ -51,6 +51,7 @@
 using Params = MipsEmuLinuxParams;

 EmuLinux(const Params ) : SEWorkload(p, PageShift) {}
+ByteOrder byteOrder() const override { return ByteOrder::little; }

 void syscall(ThreadContext *tc) override;
 };
diff --git a/src/arch/power/linux/se_workload.hh  
b/src/arch/power/linux/se_workload.hh

index 1b380ad..192147e 100644
--- a/src/arch/power/linux/se_workload.hh
+++ b/src/arch/power/linux/se_workload.hh
@@ -53,6 +53,8 @@

 EmuLinux(const Params ) : SEWorkload(p, PageShift) {}

+ByteOrder byteOrder() const override { return ByteOrder::big; }
+
 void syscall(ThreadContext *tc) override;
 };

diff --git a/src/arch/riscv/bare_metal/fs_workload.hh  
b/src/arch/riscv/bare_metal/fs_workload.hh

index 875910a..e10c0a0 100644
--- a/src/arch/riscv/bare_metal/fs_workload.hh
+++ b/src/arch/riscv/bare_metal/fs_workload.hh
@@ -64,6 +64,7 @@
 }

 loader::Arch getArch() const override { return bootloader->getArch(); }
+ByteOrder byteOrder() const override { return ByteOrder::little; }

 const loader::SymbolTable &
 symtab(ThreadContext *tc) override
diff --git a/src/arch/riscv/linux/fs_workload.hh  
b/src/arch/riscv/linux/fs_workload.hh

index f85ec16..cb29bee 100644
--- a/src/arch/riscv/linux/fs_workload.hh
+++ b/src/arch/riscv/linux/fs_workload.hh
@@ -53,6 +53,8 @@
 KernelWorkload::setSystem(sys);
 gdb = BaseRemoteGDB::build(system);
 }
+
+ByteOrder byteOrder() const override { return ByteOrder::little; }
 };

 } // namespace RiscvISA
diff --git a/src/arch/riscv/linux/se_workload.hh  
b/src/arch/riscv/linux/se_workload.hh

index bdc39ce..41a3d41 100644
--- a/src/arch/riscv/linux/se_workload.hh
+++ b/src/arch/riscv/linux/se_workload.hh
@@ -57,6 +57,8 @@

 EmuLinux(const Params ) : 

[gem5-dev] Change in gem5/gem5[develop]: sim: Create a StubWorkload for System.workload to default to.

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



Change subject: sim: Create a StubWorkload for System.workload to default  
to.

..

sim: Create a StubWorkload for System.workload to default to.

This way there will always be a workload object, even if nothing needs
to be set up. This default can also be used in low_power_sweep.py,
where the workload object was just a placeholder.

Change-Id: Idfc3e75c65318d75a3eae6a19944ae1f79a2d111
---
M src/sim/workload.hh
M src/sim/system.cc
M src/sim/System.py
M configs/dram/low_power_sweep.py
M src/sim/Workload.py
5 files changed, 55 insertions(+), 13 deletions(-)



diff --git a/configs/dram/low_power_sweep.py  
b/configs/dram/low_power_sweep.py

index c21a180..db02a06 100644
--- a/configs/dram/low_power_sweep.py
+++ b/configs/dram/low_power_sweep.py
@@ -90,8 +90,6 @@
voltage_domain =
VoltageDomain(voltage = '1V'))

-system.workload = SEWorkload()
-
 # We are fine with 256 MB memory for now.
 mem_range = AddrRange('256MB')
 # Start address is 0
diff --git a/src/sim/System.py b/src/sim/System.py
index d7b88b0..115fb94 100644
--- a/src/sim/System.py
+++ b/src/sim/System.py
@@ -44,6 +44,7 @@

 from m5.objects.DVFSHandler import *
 from m5.objects.SimpleMemory import *
+from m5.objects.Workload import StubWorkload

 class MemoryMode(Enum): vals = ['invalid', 'atomic', 'timing',
 'atomic_noncaching']
@@ -117,7 +118,7 @@
 work_cpus_ckpt_count = Param.Counter(0,
 "create checkpoint when active cpu count value is reached")

-workload = Param.Workload(NULL, "Workload to run on this system")
+workload = Param.Workload(StubWorkload(), "Workload to run on this  
system")

 init_param = Param.UInt64(0, "numerical value to pass into simulator")
 readfile = Param.String("", "file to read startup script from")
 symbolfile = Param.String("", "file to get the symbols from")
diff --git a/src/sim/Workload.py b/src/sim/Workload.py
index 62aa047..92ed7c3 100644
--- a/src/sim/Workload.py
+++ b/src/sim/Workload.py
@@ -37,6 +37,15 @@
 wait_for_remote_gdb = Param.Bool(False,
 "Wait for a remote GDB connection");

+class StubWorkload(Workload):
+type = 'StubWorkload'
+cxx_header = "sim/workload.hh"
+cxx_class = 'gem5::StubWorkload'
+
+entry = Param.Addr(0, 'Dummy entry point for this workload.')
+byte_order = Param.ByteOrder('little',
+'Dummy byte order for this workload.')
+
 class KernelWorkload(Workload):
 type = 'KernelWorkload'
 cxx_header = "sim/kernel_workload.hh"
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 14e6a78..5ebf0f4 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -96,9 +96,7 @@
 System::Threads::Thread::quiesce() const
 {
 context->suspend();
-auto *workload = context->getSystemPtr()->workload;
-if (workload)
-workload->recordQuiesce();
+context->getSystemPtr()->workload->recordQuiesce();
 }

 void
@@ -217,8 +215,8 @@
  AddrRange(1, 0)), // Create an empty range if disabled
   redirectPaths(p.redirect_paths)
 {
-if (workload)
-workload->setSystem(this);
+panic_if(!workload, "No workload set for system %s.", name());
+workload->setSystem(this);

 // add self to global system list
 systemList.push_back(this);
@@ -277,8 +275,7 @@
 {
 threads.insert(tc, assigned);

-if (workload)
-workload->registerThreadContext(tc);
+workload->registerThreadContext(tc);

 for (auto *e: liveEvents)
 tc->schedule(e);
@@ -310,8 +307,7 @@
 auto *otc = threads[context_id];
 threads.replace(tc, context_id);

-if (workload)
-workload->replaceThreadContext(tc);
+workload->replaceThreadContext(tc);

 for (auto *e: liveEvents) {
 otc->remove(e);
@@ -454,7 +450,7 @@
 bool
 System::trapToGdb(int signal, ContextID ctx_id) const
 {
-return workload && workload->trapToGdb(signal, ctx_id);
+return workload->trapToGdb(signal, ctx_id);
 }

 void
diff --git a/src/sim/workload.hh b/src/sim/workload.hh
index fa62555..a57b6c1 100644
--- a/src/sim/workload.hh
+++ b/src/sim/workload.hh
@@ -33,6 +33,7 @@

 #include "base/loader/object_file.hh"
 #include "base/loader/symtab.hh"
+#include "params/StubWorkload.hh"
 #include "params/Workload.hh"
 #include "sim/sim_object.hh"
 #include "sim/stats.hh"
@@ -159,6 +160,30 @@
 /** @} */
 };

+class StubWorkload : public Workload
+{
+  private:
+PARAMS(StubWorkload);
+loader::SymbolTable _symtab;
+
+  public:
+StubWorkload(const StubWorkloadParams ) : Workload(params) {}
+
+Addr getEntry() const override { return params().entry; }
+ByteOrder byteOrder() const override { return params().byte_order; }
+loader::Arch getArch() const override { return loader::UnknownArch; }

[gem5-dev] Presentation about multi ISA gem5

2021-10-27 Thread Gabe Black via gem5-dev
Hi folks. I wrote a presentation about my work to make gem5 able to support
more than one ISA at a time. It's *slightly* out of date now, since more
work has been finished since it was last updated. The structure of it
seemed like a good idea initially, but I think it may have made the
presentation a little more repetitive than would be ideal. Please take a
look if you're curious, or if you want more insight into what I'm doing and
how.

https://docs.google.com/presentation/d/1PmyLLBYO1wjpGE6vVLeADsw_JWgN8uB967pp-dthZmw/edit?usp=sharing

Gabe
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Add walkBits method to PageTableOps

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


 (

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the  
submitted one.

 )Change subject: arch-arm: Add walkBits method to PageTableOps
..

arch-arm: Add walkBits method to PageTableOps

Change-Id: I84cea3bcc5a3b566b8c26ebd3a54443c96199483
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52008
Tested-by: kokoro 
Reviewed-by: Jason Lowe-Power 
Reviewed-by: Andreas Sandberg 
Maintainer: Jason Lowe-Power 
Maintainer: Andreas Sandberg 
---
M src/arch/arm/pagetable.cc
M src/arch/arm/pagetable.hh
2 files changed, 55 insertions(+), 31 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/pagetable.cc b/src/arch/arm/pagetable.cc
index 0834e7b..542610d 100644
--- a/src/arch/arm/pagetable.cc
+++ b/src/arch/arm/pagetable.cc
@@ -51,6 +51,12 @@
 const GrainSize GrainMap_tg1[] =
   { ReservedGrain, Grain16KB, Grain4KB, Grain64KB };

+Addr
+PageTableOps::walkMask(unsigned level) const
+{
+return ~mask(walkBits(level));
+}
+
 bool
 V7LPageTableOps::isValid(pte_t pte, unsigned level) const
 {
@@ -119,14 +125,14 @@
 }
 }

-Addr
-V7LPageTableOps::walkMask(unsigned level) const
+unsigned
+V7LPageTableOps::walkBits(unsigned level) const
 {
 switch (level) {
-case 1: return ~mask(30);
-case 2: return ~mask(21);
-case 3: return ~mask(12);
-default: panic("bad level %d", level);
+  case 1: return 30;
+  case 2: return 21;
+  case 3: return 12;
+  default: panic("bad level %d", level);
 }
 }

@@ -212,15 +218,15 @@
 }
 }

-Addr
-V8PageTableOps4k::walkMask(unsigned level) const
+unsigned
+V8PageTableOps4k::walkBits(unsigned level) const
 {
 switch (level) {
-case 0: return ~mask(39);
-case 1: return ~mask(30);
-case 2: return ~mask(21);
-case 3: return ~mask(12);
-default: panic("bad level %d", level);
+  case 0: return 39;
+  case 1: return 30;
+  case 2: return 21;
+  case 3: return 12;
+  default: panic("bad level %d", level);
 }
 }

@@ -323,15 +329,15 @@
 }
 }

-Addr
-V8PageTableOps16k::walkMask(unsigned level) const
+unsigned
+V8PageTableOps16k::walkBits(unsigned level) const
 {
 switch (level) {
-case 0: return ~mask(47);
-case 1: return ~mask(36);
-case 2: return ~mask(25);
-case 3: return ~mask(14);
-default: panic("bad level %d", level);
+  case 0: return 47;
+  case 1: return 36;
+  case 2: return 25;
+  case 3: return 14;
+  default: panic("bad level %d", level);
 }
 }

@@ -428,14 +434,14 @@
 }
 }

-Addr
-V8PageTableOps64k::walkMask(unsigned level) const
+unsigned
+V8PageTableOps64k::walkBits(unsigned level) const
 {
 switch (level) {
-case 1: return ~mask(42);
-case 2: return ~mask(29);
-case 3: return ~mask(16);
-default: panic("bad level %d", level);
+  case 1: return 42;
+  case 2: return 29;
+  case 3: return 16;
+  default: panic("bad level %d", level);
 }
 }

diff --git a/src/arch/arm/pagetable.hh b/src/arch/arm/pagetable.hh
index bf4a5f7..4cfdf2a 100644
--- a/src/arch/arm/pagetable.hh
+++ b/src/arch/arm/pagetable.hh
@@ -98,10 +98,12 @@
 virtual Addr nextLevelPointer(pte_t pte, unsigned level) const = 0;
 virtual Addr index(Addr va, unsigned level, int tsz) const = 0;
 virtual Addr pageMask(pte_t pte, unsigned level) const = 0;
-virtual Addr walkMask(unsigned level) const = 0;
+virtual unsigned walkBits(unsigned level) const = 0;
 virtual LookupLevel firstLevel(uint8_t tsz) const = 0;
 virtual LookupLevel firstS2Level(uint8_t sl0) const = 0;
 virtual LookupLevel lastLevel() const = 0;
+
+Addr walkMask(unsigned level) const;
 };

 struct V7LPageTableOps : public PageTableOps
@@ -112,7 +114,7 @@
 Addr nextLevelPointer(pte_t pte, unsigned level) const override;
 Addr index(Addr va, unsigned level, int tsz) const override;
 Addr pageMask(pte_t pte, unsigned level) const override;
-Addr walkMask(unsigned level) const override;
+unsigned walkBits(unsigned level) const override;
 LookupLevel firstLevel(uint8_t tsz) const override;
 LookupLevel lastLevel() const override;
 };
@@ -125,7 +127,7 @@
 Addr nextLevelPointer(pte_t pte, unsigned level) const override;
 Addr index(Addr va, unsigned level, int tsz) const override;
 Addr pageMask(pte_t pte, unsigned level) const override;
-Addr walkMask(unsigned level) const override;
+unsigned walkBits(unsigned level) const override;
 LookupLevel firstLevel(uint8_t tsz) const 

[gem5-dev] Change in gem5/gem5[develop]: arch-arm: Expose LookupLevel enum to the python world

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


Change subject: arch-arm: Expose LookupLevel enum to the python world
..

arch-arm: Expose LookupLevel enum to the python world

Our goal is to make it a SimObject Param

Change-Id: I90673fada66f59d4a90354660d2513a39c0ccba5
Signed-off-by: Giacomo Travaglini 
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52007
Reviewed-by: Andreas Sandberg 
Maintainer: Andreas Sandberg 
Tested-by: kokoro 
---
M src/arch/arm/ArmTLB.py
M src/arch/arm/mmu.hh
M src/arch/arm/pagetable.cc
M src/arch/arm/pagetable.hh
M src/arch/arm/tlb.hh
M src/arch/arm/table_walker.cc
M src/arch/arm/table_walker.hh
7 files changed, 148 insertions(+), 121 deletions(-)

Approvals:
  Andreas Sandberg: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass




diff --git a/src/arch/arm/ArmTLB.py b/src/arch/arm/ArmTLB.py
index e2d61e4..4c86f72 100644
--- a/src/arch/arm/ArmTLB.py
+++ b/src/arch/arm/ArmTLB.py
@@ -1,6 +1,6 @@
 # -*- mode:python -*-

-# Copyright (c) 2009, 2013, 2015 ARM Limited
+# Copyright (c) 2009, 2013, 2015, 2021 Arm Limited
 # All rights reserved.
 #
 # The license below extends only to copyright in the software and shall
@@ -40,6 +40,9 @@
 from m5.proxy import *
 from m5.objects.BaseTLB import BaseTLB

+class ArmLookupLevel(Enum):
+vals = [ "L0", "L1", "L2", "L3" ]
+
 class ArmTLB(BaseTLB):
 type = 'ArmTLB'
 cxx_class = 'gem5::ArmISA::TLB'
diff --git a/src/arch/arm/mmu.hh b/src/arch/arm/mmu.hh
index 9391612..2d0ef7b 100644
--- a/src/arch/arm/mmu.hh
+++ b/src/arch/arm/mmu.hh
@@ -45,6 +45,8 @@
 #include "arch/arm/tlb.hh"
 #include "arch/generic/mmu.hh"

+#include "enums/ArmLookupLevel.hh"
+
 #include "params/ArmMMU.hh"

 namespace gem5
@@ -57,6 +59,8 @@
 class MMU : public BaseMMU
 {
   protected:
+using LookupLevel = enums::ArmLookupLevel;
+
 ArmISA::TLB *
 getDTBPtr() const
 {
diff --git a/src/arch/arm/pagetable.cc b/src/arch/arm/pagetable.cc
index 506b615..0834e7b 100644
--- a/src/arch/arm/pagetable.cc
+++ b/src/arch/arm/pagetable.cc
@@ -130,16 +130,16 @@
 }
 }

-LookupLevel
+V7LPageTableOps::LookupLevel
 V7LPageTableOps::firstLevel(uint8_t tsz) const
 {
-return L1;
+return LookupLevel::L1;
 }

-LookupLevel
+V7LPageTableOps::LookupLevel
 V7LPageTableOps::lastLevel() const
 {
-return L3;
+return LookupLevel::L3;
 }

 bool
@@ -224,31 +224,31 @@
 }
 }

-LookupLevel
+V8PageTableOps4k::LookupLevel
 V8PageTableOps4k::firstLevel(uint8_t tsz) const
 {
-if (tsz >= 16 && tsz <= 24) return L0;
-if (tsz >= 25 && tsz <= 33) return L1;
-if (tsz >= 34 && tsz <= 39) return L2;
+if (tsz >= 16 && tsz <= 24) return LookupLevel::L0;
+if (tsz >= 25 && tsz <= 33) return LookupLevel::L1;
+if (tsz >= 34 && tsz <= 39) return LookupLevel::L2;

 panic("Unsupported TnSZ: %d\n", tsz);
 }

-LookupLevel
+V8PageTableOps4k::LookupLevel
 V8PageTableOps4k::firstS2Level(uint8_t sl0) const
 {
 switch (sl0) {
-  case 0: return L2;
-  case 1: return L1;
-  case 2: return L0;
+  case 0: return LookupLevel::L2;
+  case 1: return LookupLevel::L1;
+  case 2: return LookupLevel::L0;
   default: panic("Unsupported VTCR_EL2.SL0: %d", sl0);
 }
 }

-LookupLevel
+V8PageTableOps4k::LookupLevel
 V8PageTableOps4k::lastLevel() const
 {
-return L3;
+return LookupLevel::L3;
 }

 bool
@@ -335,32 +335,32 @@
 }
 }

-LookupLevel
+V8PageTableOps16k::LookupLevel
 V8PageTableOps16k::firstLevel(uint8_t tsz) const
 {
-if (tsz == 16) return L0;
-if (tsz >= 17 && tsz <= 27) return L1;
-if (tsz >= 28 && tsz <= 38) return L2;
-if (tsz == 39) return L3;
+if (tsz == 16) return LookupLevel::L0;
+if (tsz >= 17 && tsz <= 27) return LookupLevel::L1;
+if (tsz >= 28 && tsz <= 38) return LookupLevel::L2;
+if (tsz == 39) return LookupLevel::L3;

 panic("Unsupported TnSZ: %d\n", tsz);
 }

-LookupLevel
+V8PageTableOps16k::LookupLevel
 V8PageTableOps16k::firstS2Level(uint8_t sl0) const
 {
 switch (sl0) {
-  case 0: return L3;
-  case 1: return L2;
-  case 2: return L1;
+  case 0: return LookupLevel::L3;
+  case 1: return LookupLevel::L2;
+  case 2: return LookupLevel::L1;
   default: panic("Unsupported VTCR_EL2.SL0: %d", sl0);
 }
 }

-LookupLevel
+V8PageTableOps16k::LookupLevel
 V8PageTableOps16k::lastLevel() const
 {
-return L3;
+return LookupLevel::L3;
 }

 bool
@@ -439,31 +439,31 @@
 }
 }

-LookupLevel
+V8PageTableOps64k::LookupLevel
 V8PageTableOps64k::firstLevel(uint8_t tsz) const
 {
-if (tsz >= 12 && tsz <= 21) return L1;
-if (tsz >= 22 && tsz <= 34) return L2;
-if (tsz >= 35 && tsz <= 39) return L3;
+if (tsz >= 12 && tsz <= 21) return LookupLevel::L1;
+if (tsz >= 22 && tsz <= 34) return LookupLevel::L2;
+if (tsz >= 35 && tsz <= 39) return 

[gem5-dev] Change in gem5/gem5[develop]: cpu-o3: Don't update stats in (read|set)Arch*Reg methods.

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


Change subject: cpu-o3: Don't update stats in (read|set)Arch*Reg methods.
..

cpu-o3: Don't update stats in (read|set)Arch*Reg methods.

These are called from the ThreadContext, and should not be counted in
the statistics. The (read|set)*Reg methods, aka readIntReg and not
readArchIntReg, are called from the (read|set)*RegOperand methods in the
DynInst, which is the ExecContext implementation when running on O3.

Change-Id: I9abf90fc7bbe80a742325b6dfd3c0e14392af54c
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/51428
Maintainer: Gabe Black 
Tested-by: kokoro 
Reviewed-by: Giacomo Travaglini 
---
M src/cpu/o3/cpu.cc
1 file changed, 26 insertions(+), 14 deletions(-)

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




diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 8f0c531..a574a4b 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -1244,7 +1244,6 @@
 RegVal
 CPU::readArchIntReg(int reg_idx, ThreadID tid)
 {
-cpuStats.intRegfileReads++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(IntRegClass, reg_idx));

@@ -1254,7 +1253,6 @@
 RegVal
 CPU::readArchFloatReg(int reg_idx, ThreadID tid)
 {
-cpuStats.fpRegfileReads++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(FloatRegClass, reg_idx));

@@ -1266,7 +1264,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecRegClass, reg_idx));
-return readVecReg(phys_reg);
+return regFile.readVecReg(phys_reg);
 }

 TheISA::VecRegContainer&
@@ -1274,7 +1272,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecRegClass, reg_idx));
-return getWritableVecReg(phys_reg);
+return regFile.getWritableVecReg(phys_reg);
 }

 RegVal
@@ -1283,7 +1281,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecElemClass, reg_idx, ldx));
-return readVecElem(phys_reg);
+return regFile.readVecElem(phys_reg);
 }

 const TheISA::VecPredRegContainer&
@@ -1291,7 +1289,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecPredRegClass, reg_idx));
-return readVecPredReg(phys_reg);
+return regFile.readVecPredReg(phys_reg);
 }

 TheISA::VecPredRegContainer&
@@ -1299,13 +1297,12 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecPredRegClass, reg_idx));
-return getWritableVecPredReg(phys_reg);
+return regFile.getWritableVecPredReg(phys_reg);
 }

 RegVal
 CPU::readArchCCReg(int reg_idx, ThreadID tid)
 {
-cpuStats.ccRegfileReads++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(CCRegClass, reg_idx));

@@ -1315,7 +1312,6 @@
 void
 CPU::setArchIntReg(int reg_idx, RegVal val, ThreadID tid)
 {
-cpuStats.intRegfileWrites++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(IntRegClass, reg_idx));

@@ -1325,7 +1321,6 @@
 void
 CPU::setArchFloatReg(int reg_idx, RegVal val, ThreadID tid)
 {
-cpuStats.fpRegfileWrites++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(FloatRegClass, reg_idx));

@@ -1338,7 +1333,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecRegClass, reg_idx));
-setVecReg(phys_reg, val);
+regFile.setVecReg(phys_reg, val);
 }

 void
@@ -1347,7 +1342,7 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecElemClass, reg_idx, ldx));
-setVecElem(phys_reg, val);
+regFile.setVecElem(phys_reg, val);
 }

 void
@@ -1356,13 +1351,12 @@
 {
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(VecPredRegClass, reg_idx));
-setVecPredReg(phys_reg, val);
+regFile.setVecPredReg(phys_reg, val);
 }

 void
 CPU::setArchCCReg(int reg_idx, RegVal val, ThreadID tid)
 {
-cpuStats.ccRegfileWrites++;
 PhysRegIdPtr phys_reg = commitRenameMap[tid].lookup(
 RegId(CCRegClass, reg_idx));


--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/51428
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: I9abf90fc7bbe80a742325b6dfd3c0e14392af54c
Gerrit-Change-Number: 51428
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black 
Gerrit-Reviewer: Andreas Sandberg 
Gerrit-Reviewer: Daniel Carvalho 
Gerrit-Reviewer: Gabe Black 
Gerrit-Reviewer: Giacomo Travaglini 
Gerrit-Reviewer: Jason Lowe-Power 
Gerrit-Reviewer: Richard Cooper 
Gerrit-Reviewer: kokoro 
Gerrit-MessageType: merged
___
gem5-dev mailing list -- gem5-dev@gem5.org
To