From 25d4fb2d91cec126b5d0c0ae312c0d7ef7440d89 Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Thu, 27 Oct 2022 15:16:58 -0700 Subject: [PATCH] stdlib: Move `_connect_things` to run as pre_instantiation Through working with the gem5 stdlib there have been instances where connecting the memory, processor, and cache hierarchy to the board (via the AbstractBoard's `_connect_things` function) at the point of the AbstractBoard's construction is problematic as the memory, processor, and cache hierarchy may require information to connect correctly that is only known to the AbstractBoard after construction. In particular this can occur when a Workload contains information needed to configure correctly. To resolve this problem the `_connect_things` function has been moved to run as a pre-initialization step. That is, run immediately before `m5.instantiate`. This is done in the Simulator module. This will break cases where a user utilizes the stdlib AbstractBoard but does not use the stdlib Simulator module. As such, an Exception is raised in these cases explaining the fix to the user. This is done via a hack where the boards' `createCCObject` function (inheritted from SimObject) is overriden with a check to ensure `_connect_things` has been run. To fix the `_pre_instantiate` function must be executed prior to `m5.instantiate` in the Python configuration script. Test and config scripts in the gem5 repo have been updated accordingly. Change-Id: Ibaef36eb7433ce104b861b1da80fc600f08f715a Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/65051 Maintainer: Bobby Bruce Reviewed-by: Giacomo Travaglini Reviewed-by: Jason Lowe-Power Tested-by: kokoro --- .../gem5_library/x86-gapbs-benchmarks.py | 1 + .../gem5_library/x86-npb-benchmarks.py | 1 + .../gem5_library/x86-parsec-benchmarks.py | 1 + .../x86-spec-cpu2006-benchmarks.py | 1 + .../x86-spec-cpu2017-benchmarks.py | 1 + .../gem5/components/boards/abstract_board.py | 52 ++++++++++++++++++- .../boards/abstract_system_board.py | 12 ++++- .../gem5/components/boards/arm_board.py | 10 ++++ src/python/gem5/simulate/simulator.py | 5 ++ tests/gem5/configs/boot_kvm_fork_run.py | 2 +- tests/gem5/traffic_gen/simple_traffic_run.py | 1 + 11 files changed, 83 insertions(+), 4 deletions(-) diff --git a/configs/example/gem5_library/x86-gapbs-benchmarks.py b/configs/example/gem5_library/x86-gapbs-benchmarks.py index 407881111d..bdc0d9427d 100644 --- a/configs/example/gem5_library/x86-gapbs-benchmarks.py +++ b/configs/example/gem5_library/x86-gapbs-benchmarks.py @@ -216,6 +216,7 @@ root = Root(full_system=True, system=board) root.sim_quantum = int(1e9) +board._pre_instantiate() m5.instantiate() # We maintain the wall clock time. diff --git a/configs/example/gem5_library/x86-npb-benchmarks.py b/configs/example/gem5_library/x86-npb-benchmarks.py index b3a78c2909..385760c7a7 100644 --- a/configs/example/gem5_library/x86-npb-benchmarks.py +++ b/configs/example/gem5_library/x86-npb-benchmarks.py @@ -218,6 +218,7 @@ root = Root(full_system=True, system=board) root.sim_quantum = int(1e9) +board._pre_instantiate() m5.instantiate() # We maintain the wall clock time. diff --git a/configs/example/gem5_library/x86-parsec-benchmarks.py b/configs/example/gem5_library/x86-parsec-benchmarks.py index 08fa636f1e..82183802c7 100644 --- a/configs/example/gem5_library/x86-parsec-benchmarks.py +++ b/configs/example/gem5_library/x86-parsec-benchmarks.py @@ -204,6 +204,7 @@ root = Root(full_system=True, system=board) root.sim_quantum = int(1e9) +board._pre_instantiate() m5.instantiate() # We maintain the wall clock time. diff --git a/configs/example/gem5_library/x86-spec-cpu2006-benchmarks.py b/configs/example/gem5_library/x86-spec-cpu2006-benchmarks.py index 0bdc5ed271..d656e61145 100644 --- a/configs/example/gem5_library/x86-spec-cpu2006-benchmarks.py +++ b/configs/example/gem5_library/x86-spec-cpu2006-benchmarks.py @@ -274,6 +274,7 @@ root = Root(full_system=True, system=board) root.sim_quantum = int(1e9) +board._pre_instantiate() m5.instantiate() # We maintain the wall clock time. diff --git a/configs/example/gem5_library/x86-spec-cpu2017-benchmarks.py b/configs/example/gem5_library/x86-spec-cpu2017-benchmarks.py index 6b0cc116be..2bc948aea1 100644 --- a/configs/example/gem5_library/x86-spec-cpu2017-benchmarks.py +++ b/configs/example/gem5_library/x86-spec-cpu2017-benchmarks.py @@ -290,6 +290,7 @@ root = Root(full_system=True, system=board) root.sim_quantum = int(1e9) +board._pre_instantiate() m5.instantiate() # We maintain the wall clock time. diff --git a/src/python/gem5/components/boards/abstract_board.py b/src/python/gem5/components/boards/abstract_board.py index 720aacaf65..4ea8866009 100644 --- a/src/python/gem5/components/boards/abstract_board.py +++ b/src/python/gem5/components/boards/abstract_board.py @@ -112,8 +112,9 @@ class AbstractBoard: # Setup board properties unique to the board being constructed. self._setup_board() - # Connect the memory, processor, and cache hierarchy. - self._connect_things() + # A private variable to record whether `_connect_things` has been + # been called. + self._connect_things_called = False def get_processor(self) -> "AbstractProcessor": """Get the processor connected to the board. @@ -341,8 +342,15 @@ class AbstractBoard: * The processor is incorporated after the cache hierarchy due to a bug noted here: https://gem5.atlassian.net/browse/GEM5-1113. Until this bug is fixed, this ordering must be maintained. + * Once this function is called `_connect_things_called` *must* be set + to `True`. """ + if self._connect_things_called: + raise Exception( + "The `_connect_things` function has already been called." + ) + # Incorporate the memory into the motherboard. self.get_memory().incorporate_memory(self) @@ -353,9 +361,49 @@ class AbstractBoard: # Incorporate the processor into the motherboard. self.get_processor().incorporate_processor(self) + self._connect_things_called = True + def _post_instantiate(self): """Called to set up anything needed after m5.instantiate""" self.get_processor()._post_instantiate() if self.get_cache_hierarchy(): self.get_cache_hierarchy()._post_instantiate() self.get_memory()._post_instantiate() + + def _pre_instantiate(self): + """To be called immediately before m5.instantiate. This is where + `_connect_things` is executed by default.""" + + # Connect the memory, processor, and cache hierarchy. + self._connect_things() + + def _connect_things_check(self): + """ + Here we check that connect things has been called and throw an + Exception if it has not. + + Since v22.1 `_connect_things` function has + been moved from the AbstractBoard constructor to the + `_pre_instantation` function. Users who have used the gem5 stdlib + components (i.e., boards which inherit from AbstractBoard) and the + Simulator module should notice no change. Those who do not use the + Simulator module and instead called `m5.instantiate` directly must + call `AbstractBoard._pre_instantation` prior so `_connect_things` is + called. In order to avoid confusion, this check has been incorporated + and the Exception thrown explains the fix needed to convert old scripts + to function with v22.1. + + This function is called in `AbstractSystemBoard.createCCObject` and + ArmBoard.createCCObject`. Both these functions override + `SimObject.createCCObject`. We can not do that here as AbstractBoard + does not inherit form System. + """ + if not self._connect_things_called: + raise Exception( + """ +AbstractBoard's `_connect_things` function has not been called. This is likely +due to not running a board outside of the gem5 Standard Library Simulator +module. If this is the case, this can be resolved by calling +`._pre_instantiate()` prior to `m5.instantiate()`. +""" + ) diff --git a/src/python/gem5/components/boards/abstract_system_board.py b/src/python/gem5/components/boards/abstract_system_board.py index be39b4a5ca..2812b37260 100644 --- a/src/python/gem5/components/boards/abstract_system_board.py +++ b/src/python/gem5/components/boards/abstract_system_board.py @@ -27,8 +27,9 @@ from abc import ABCMeta from .abstract_board import AbstractBoard +from ...utils.override import overrides -from m5.objects import System +from m5.objects import System, SimObject class AbstractSystemBoard(System, AbstractBoard): @@ -54,3 +55,12 @@ class AbstractSystemBoard(System, AbstractBoard): memory=memory, cache_hierarchy=cache_hierarchy, ) + + @overrides(SimObject) + def createCCObject(self): + """We override this function as it is called in `m5.instantiate`. This + means we can insert a check to ensure the `_connect_things` function + has been run. + """ + super()._connect_things_check() + super().createCCObject() diff --git a/src/python/gem5/components/boards/arm_board.py b/src/python/gem5/components/boards/arm_board.py index eec9432864..afb5cd6e7e 100644 --- a/src/python/gem5/components/boards/arm_board.py +++ b/src/python/gem5/components/boards/arm_board.py @@ -44,6 +44,7 @@ from m5.objects import ( ArmDefaultRelease, VExpress_GEM5_Base, VExpress_GEM5_Foundation, + SimObject, ) import os @@ -388,3 +389,12 @@ class ArmBoard(ArmSystem, AbstractBoard, KernelDiskWorkload): "rw", "mem=%s" % self.get_memory().get_size(), ] + + @overrides(SimObject) + def createCCObject(self): + """We override this function as it is called in `m5.instantiate`. This + means we can insert a check to ensure the `_connect_things` function + has been run. + """ + super()._connect_things_check() + super().createCCObject() diff --git a/src/python/gem5/simulate/simulator.py b/src/python/gem5/simulate/simulator.py index b45ff55162..5a4ab171de 100644 --- a/src/python/gem5/simulate/simulator.py +++ b/src/python/gem5/simulate/simulator.py @@ -377,6 +377,11 @@ class Simulator: """ if not self._instantiated: + + # Before anything else we run the AbstractBoard's + # `_pre_instantiate` function. + self._board._pre_instantiate() + root = Root( full_system=self._full_system if self._full_system is not None diff --git a/tests/gem5/configs/boot_kvm_fork_run.py b/tests/gem5/configs/boot_kvm_fork_run.py index 8a28608265..18f6e9d416 100644 --- a/tests/gem5/configs/boot_kvm_fork_run.py +++ b/tests/gem5/configs/boot_kvm_fork_run.py @@ -211,7 +211,7 @@ root.sim_quantum = int(1e9) # Disable the gdb ports. Required for forking. m5.disableAllListeners() - +motherboard._pre_instantiate() m5.instantiate() # Simulate the inital boot with the starting KVM cpu diff --git a/tests/gem5/traffic_gen/simple_traffic_run.py b/tests/gem5/traffic_gen/simple_traffic_run.py index bfd21fe8e8..4e38155070 100644 --- a/tests/gem5/traffic_gen/simple_traffic_run.py +++ b/tests/gem5/traffic_gen/simple_traffic_run.py @@ -201,6 +201,7 @@ motherboard = TestBoard( root = Root(full_system=False, system=motherboard) +motherboard._pre_instantiate() m5.instantiate() generator.start_traffic()