Bobby R. Bruce has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/52184 )

Change subject: stdlib: Move 'connect_things' to AbstractBoard constructor
......................................................................

stdlib: Move 'connect_things' to AbstractBoard constructor

This patch moves 'connect_things' to the AbstractBoard constructor,
thereby meaning it does not need to be called directly in gem5
configuration scripts. This method has been changed to private as a
result.

As boards that inherit from AbstractBoard require certain things to be
setup prior to `connect_things` being called, a new abstract function,
`_setup_board` has been created. This is called in the AbstractBoard
constructor before `connect_things` and can be overridden by boards to
setup board properties as required.

Change-Id: I558a4321b850a6b19e20b7d56d0bcae5805114b6
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/52184
Reviewed-by: Bobby R. Bruce <bbr...@ucdavis.edu>
Maintainer: Bobby R. Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
---
M configs/example/gem5_library/riscv-fs.py
M src/python/gem5/components/boards/simple_board.py
M src/python/gem5/components/boards/riscv_board.py
M configs/example/gem5_library/arm-hello.py
M tests/gem5/configs/boot_kvm_fork_run.py
M tests/gem5/configs/parsec_disk_run.py
M src/python/gem5/components/boards/test_board.py
M tests/gem5/configs/boot_kvm_switch_exit.py
M configs/example/gem5_library/x86-ubuntu-run.py
M src/python/gem5/components/boards/x86_board.py
M tests/gem5/configs/riscv_boot_exit_run.py
M tests/gem5/configs/simple_binary_run.py
M tests/gem5/configs/simple_traffic_run.py
M tests/gem5/configs/x86_boot_exit_run.py
M src/python/gem5/components/boards/abstract_board.py
15 files changed, 49 insertions(+), 26 deletions(-)

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




diff --git a/configs/example/gem5_library/arm-hello.py b/configs/example/gem5_library/arm-hello.py
index 067c867..dc4266c 100644
--- a/configs/example/gem5_library/arm-hello.py
+++ b/configs/example/gem5_library/arm-hello.py
@@ -75,10 +75,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-# This method must be called to connect all the components specified during
-# the board's construction.
-board.connect_things()
-
# Here we set the workload. In this case we want to run a simple "Hello World!"
 # program compiled to the ARM ISA. The `Resource` class will automatically
# download the binary from the gem5 Resources cloud bucket if it's not already diff --git a/configs/example/gem5_library/riscv-fs.py b/configs/example/gem5_library/riscv-fs.py
index 35a7250..f46a345 100644
--- a/configs/example/gem5_library/riscv-fs.py
+++ b/configs/example/gem5_library/riscv-fs.py
@@ -78,8 +78,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-board.connect_things()
-
 # Set the Full System workload.
 board.set_kernel_disk_workload(
                    kernel=Resource("riscv-bootloader-vmlinux-5.10"),
diff --git a/configs/example/gem5_library/x86-ubuntu-run.py b/configs/example/gem5_library/x86-ubuntu-run.py
index ce3d637..1ab9f3a 100644
--- a/configs/example/gem5_library/x86-ubuntu-run.py
+++ b/configs/example/gem5_library/x86-ubuntu-run.py
@@ -102,8 +102,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-board.connect_things()
-
 # Here we set the Full System workload.
# The `set_kernel_disk_workload` function for the X86Board takes a kernel, a
 # disk image, and, optionally, a command to run.
diff --git a/src/python/gem5/components/boards/abstract_board.py b/src/python/gem5/components/boards/abstract_board.py
index 60ce0de..ce86ab4 100644
--- a/src/python/gem5/components/boards/abstract_board.py
+++ b/src/python/gem5/components/boards/abstract_board.py
@@ -92,7 +92,12 @@
         self.memory = memory
         self.cache_hierarchy = cache_hierarchy

-        self.setup_memory_ranges()
+
+        # Setup board properties unique to the board being constructed.
+        self._setup_board()
+
+        # Connect the memory, processor, and cache hierarchy.
+        self._connect_things()

     def get_processor(self) -> "AbstractProcessor":
         """Get the processor connected to the board.
@@ -135,11 +140,20 @@

     def get_clock_domain(self) -> ClockDomain:
         """Get the clock domain.
-
         :returns: The clock domain.
         """
         return self.clk_domain

+    @abstractmethod
+    def _setup_board(self) -> None:
+        """
+ This function is called in the AbstractBoard constructor, before the + memory, processor, and cache hierarchy components are incorporated via
+        `_connect_thing()`. This function should be overridden by boards to
+        specify components, connections unique to that board.
+        """
+        raise NotImplementedError
+
# Technically `get_dma_ports` returns a list. This list could be empty to
     # indicate the presense of dma ports. Though I quite like having this
     # boolean to quickly check a board.
@@ -220,7 +234,7 @@
         raise NotImplementedError

     @final
-    def connect_things(self) -> None:
+    def _connect_things(self) -> None:
         """Connects all the components to the board.

         The order of this board is always:
diff --git a/src/python/gem5/components/boards/riscv_board.py b/src/python/gem5/components/boards/riscv_board.py
index 2b3261d..99bb630 100644
--- a/src/python/gem5/components/boards/riscv_board.py
+++ b/src/python/gem5/components/boards/riscv_board.py
@@ -90,9 +90,10 @@
         super().__init__(
clk_freq, processor, memory, cache_hierarchy, exit_on_work_items
         )
-
         requires(isa_required=ISA.RISCV)

+    @overrides(AbstractBoard)
+    def _setup_board(self) -> None:
         self.workload = RiscvLinux()

# Contains a CLINT, PLIC, UART, and some functions for the dtb, etc. diff --git a/src/python/gem5/components/boards/simple_board.py b/src/python/gem5/components/boards/simple_board.py
index 3645ed8..8fc43c8 100644
--- a/src/python/gem5/components/boards/simple_board.py
+++ b/src/python/gem5/components/boards/simple_board.py
@@ -72,6 +72,8 @@
             exit_on_work_items=exit_on_work_items,
         )

+    @overrides(AbstractBoard)
+    def _setup_board(self) -> None:
         # Set up the memory ranges
         self.setup_memory_ranges()

diff --git a/src/python/gem5/components/boards/test_board.py b/src/python/gem5/components/boards/test_board.py
index aeb19a6..b591a6e 100644
--- a/src/python/gem5/components/boards/test_board.py
+++ b/src/python/gem5/components/boards/test_board.py
@@ -66,6 +66,8 @@
             cache_hierarchy=cache_hierarchy,
         )

+    @overrides(AbstractBoard)
+    def _setup_board(self) -> None:
         self.setup_memory_ranges()

     @overrides(AbstractBoard)
diff --git a/src/python/gem5/components/boards/x86_board.py b/src/python/gem5/components/boards/x86_board.py
index dfefd49..51e3e25 100644
--- a/src/python/gem5/components/boards/x86_board.py
+++ b/src/python/gem5/components/boards/x86_board.py
@@ -90,6 +90,8 @@

         requires(isa_required=ISA.X86)

+    @overrides(AbstractBoard)
+    def _setup_board(self) -> None:
         self.pc = Pc()

         self.workload = X86FsLinux()
diff --git a/tests/gem5/configs/boot_kvm_fork_run.py b/tests/gem5/configs/boot_kvm_fork_run.py
index aeba340..862235e 100644
--- a/tests/gem5/configs/boot_kvm_fork_run.py
+++ b/tests/gem5/configs/boot_kvm_fork_run.py
@@ -195,8 +195,6 @@
     exit_on_work_items=True,
 )

-motherboard.connect_things()
-
 # Set the Full System workload.
 motherboard.set_kernel_disk_workload(
     kernel=Resource(
diff --git a/tests/gem5/configs/boot_kvm_switch_exit.py b/tests/gem5/configs/boot_kvm_switch_exit.py
index c9985e6..53b04b2 100644
--- a/tests/gem5/configs/boot_kvm_switch_exit.py
+++ b/tests/gem5/configs/boot_kvm_switch_exit.py
@@ -178,8 +178,6 @@
     exit_on_work_items=True,
 )

-motherboard.connect_things()
-
 kernal_args = motherboard.get_default_kernel_args() + [args.kernel_args]

 # Set the Full System workload.
diff --git a/tests/gem5/configs/parsec_disk_run.py b/tests/gem5/configs/parsec_disk_run.py
index 427d1aa..6e75974 100644
--- a/tests/gem5/configs/parsec_disk_run.py
+++ b/tests/gem5/configs/parsec_disk_run.py
@@ -214,8 +214,6 @@
     exit_on_work_items=True,
 )

-board.connect_things()
-
 # The command to run.
 command = (
     "cd /home/gem5/parsec-benchmark\n"
diff --git a/tests/gem5/configs/riscv_boot_exit_run.py b/tests/gem5/configs/riscv_boot_exit_run.py
index 629a0d0..4f0c49f 100644
--- a/tests/gem5/configs/riscv_boot_exit_run.py
+++ b/tests/gem5/configs/riscv_boot_exit_run.py
@@ -143,8 +143,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-board.connect_things()
-
 # Set the Full System workload.
 board.set_kernel_disk_workload(
     kernel=Resource(
diff --git a/tests/gem5/configs/simple_binary_run.py b/tests/gem5/configs/simple_binary_run.py
index 370bf5b..2e2bf3a 100644
--- a/tests/gem5/configs/simple_binary_run.py
+++ b/tests/gem5/configs/simple_binary_run.py
@@ -93,8 +93,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-motherboard.connect_things()
-
 # Set the workload
 binary = Resource(args.resource,
         resource_directory=args.resource_directory)
diff --git a/tests/gem5/configs/simple_traffic_run.py b/tests/gem5/configs/simple_traffic_run.py
index f519335..33e4419 100644
--- a/tests/gem5/configs/simple_traffic_run.py
+++ b/tests/gem5/configs/simple_traffic_run.py
@@ -131,8 +131,6 @@
     cache_hierarchy=cache_hierarchy,
 )

-motherboard.connect_things()
-
 root = Root(full_system=False, system=motherboard)

 m5.instantiate()
diff --git a/tests/gem5/configs/x86_boot_exit_run.py b/tests/gem5/configs/x86_boot_exit_run.py
index b410292..74349e8 100644
--- a/tests/gem5/configs/x86_boot_exit_run.py
+++ b/tests/gem5/configs/x86_boot_exit_run.py
@@ -185,8 +185,6 @@
     exit_on_work_items=True,
 )

-motherboard.connect_things()
-
 kernal_args = motherboard.get_default_kernel_args()
 if args.boot_type == "init":
     kernal_args.append("init=/root/exit.sh")

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/52184
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: I558a4321b850a6b19e20b7d56d0bcae5805114b6
Gerrit-Change-Number: 52184
Gerrit-PatchSet: 12
Gerrit-Owner: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Jason Lowe-Power <ja...@lowepower.com>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
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

Reply via email to