Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/66675?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
)Change subject: fastmodel,dev: Replace the reset port with a
Signal*Port<bool>.
......................................................................
fastmodel,dev: Replace the reset port with a Signal*Port<bool>.
The ResetRequestPort and ResetResponsePort have a few problems:
1. A reset signal should happen during the time a reset is asserted,
or in other words the device should stay in reset and not doing
anything while reset is asserted. It should not immediately restart
execution while the reset is still held.
2. These names are misleading, since there is no response. These names
are inherited from other port types where there is an actual response.
There is a new generic SignalSourcePort and SignalSinkPort set of port
classes which are templated on the type of signal they propogate, and
which can be used in place of reset ports in c++. These ports can
still have a specialized role which will ensure that only reset ports
are connected to each other for a form of type checking, although
the underlying c++ instances are more interoperable than that.
Change-Id: Id98bef901ab61ac5b200dbbe49439bb2d2e6c57f
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/66675
Maintainer: Gabe Black <gabebl...@google.com>
Reviewed-by: Yu-hsin Wang <yuhsi...@google.com>
Tested-by: kokoro <noreply+kok...@google.com>
---
M src/arch/arm/fastmodel/CortexA76/evs.cc
M src/arch/arm/fastmodel/CortexA76/evs.hh
M src/arch/arm/fastmodel/CortexR52/evs.cc
M src/arch/arm/fastmodel/CortexR52/evs.hh
M src/arch/arm/fastmodel/reset_controller/example.cc
M src/arch/arm/fastmodel/reset_controller/example.hh
M src/dev/SConscript
D src/dev/reset_port.cc
D src/dev/reset_port.hh
9 files changed, 63 insertions(+), 188 deletions(-)
Approvals:
Gabe Black: Looks good to me, approved
Yu-hsin Wang: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/fastmodel/CortexA76/evs.cc
b/src/arch/arm/fastmodel/CortexA76/evs.cc
index 1c06935..c9ce3cc 100644
--- a/src/arch/arm/fastmodel/CortexA76/evs.cc
+++ b/src/arch/arm/fastmodel/CortexA76/evs.cc
@@ -71,32 +71,25 @@
}
template <class Types>
-void
-ScxEvsCortexA76<Types>::requestReset()
-{
- // Reset all cores.
- for (auto &poweron_reset : this->poweron_reset) {
- poweron_reset->signal_out.set_state(0, true);
- poweron_reset->signal_out.set_state(0, false);
- }
- // Reset DSU.
- this->top_reset.signal_out.set_state(0, true);
- this->top_reset.signal_out.set_state(0, false);
- // Reset debug APB.
- this->dbg_reset.signal_out.set_state(0, true);
- this->dbg_reset.signal_out.set_state(0, false);
-}
-
-template <class Types>
ScxEvsCortexA76<Types>::ScxEvsCortexA76(
const sc_core::sc_module_name &mod_name, const Params &p) :
Base(mod_name),
amba(Base::amba, p.name + ".amba", -1),
top_reset(p.name + ".top_reset", 0),
dbg_reset(p.name + ".dbg_reset", 0),
- model_reset(p.name + ".model_reset", -1, this),
+ model_reset(p.name + ".model_reset"),
params(p)
{
+ model_reset.onChange([this](const bool &new_val) {
+ // Set reset for all cores.
+ for (auto &poweron_reset : poweron_reset)
+ poweron_reset->signal_out.set_state(0, new_val);
+ // Set reset for DSU.
+ top_reset.signal_out.set_state(0, new_val);
+ // Set reset for debug APB.
+ dbg_reset.signal_out.set_state(0, new_val);
+ });
+
for (int i = 0; i < CoreCount; i++) {
redist.emplace_back(new TlmGicTarget(this->redistributor[i],
csprintf("%s.redistributor[%d]", name(), i), i));
diff --git a/src/arch/arm/fastmodel/CortexA76/evs.hh
b/src/arch/arm/fastmodel/CortexA76/evs.hh
index 081e80f..7c4ef60 100644
--- a/src/arch/arm/fastmodel/CortexA76/evs.hh
+++ b/src/arch/arm/fastmodel/CortexA76/evs.hh
@@ -35,7 +35,6 @@
#include "arch/arm/fastmodel/common/signal_sender.hh"
#include "arch/arm/fastmodel/iris/cpu.hh"
#include "arch/arm/fastmodel/protocol/exported_clock_rate_control.hh"
-#include "dev/reset_port.hh"
#include "mem/port_proxy.hh"
#include "params/FastModelScxEvsCortexA76x1.hh"
#include "params/FastModelScxEvsCortexA76x2.hh"
@@ -45,6 +44,7 @@
#include "scx_evs_CortexA76x2.h"
#include "scx_evs_CortexA76x3.h"
#include "scx_evs_CortexA76x4.h"
+#include "sim/signal.hh"
#include "systemc/ext/core/sc_event.hh"
#include "systemc/ext/core/sc_module.hh"
#include "systemc/tlm_port_wrapper.hh"
@@ -99,7 +99,7 @@
SignalSender dbg_reset;
- ResetResponsePort<ScxEvsCortexA76> model_reset;
+ SignalSinkPort<bool> model_reset;
CortexA76Cluster *gem5CpuCluster;
@@ -129,8 +129,6 @@
void setCluster(SimObject *cluster) override;
void setResetAddr(int core, Addr addr, bool secure) override;
-
- void requestReset();
};
struct ScxEvsCortexA76x1Types
diff --git a/src/arch/arm/fastmodel/CortexR52/evs.cc
b/src/arch/arm/fastmodel/CortexR52/evs.cc
index 734323e..0ad3f18 100644
--- a/src/arch/arm/fastmodel/CortexR52/evs.cc
+++ b/src/arch/arm/fastmodel/CortexR52/evs.cc
@@ -101,9 +101,19 @@
ext_slave(Base::ext_slave, p.name + ".ext_slave", -1),
top_reset(p.name + ".top_reset", 0),
dbg_reset(p.name + ".dbg_reset", 0),
- model_reset(p.name + ".model_reset", -1, this),
+ model_reset(p.name + ".model_reset"),
params(p)
{
+ model_reset.onChange([this](const bool &new_val) {
+ // Set reset for all cores.
+ for (auto &core_pin : corePins)
+ core_pin->poweron_reset.signal_out.set_state(0, new_val);
+ // Set reset for L2 system.
+ top_reset.signal_out.set_state(0, new_val);
+ // Set reset for debug APB.
+ dbg_reset.signal_out.set_state(0, new_val);
+ });
+
for (int i = 0; i < CoreCount; i++)
corePins.emplace_back(new CorePins(this, i));
diff --git a/src/arch/arm/fastmodel/CortexR52/evs.hh
b/src/arch/arm/fastmodel/CortexR52/evs.hh
index 02ef1ae..9cebec3 100644
--- a/src/arch/arm/fastmodel/CortexR52/evs.hh
+++ b/src/arch/arm/fastmodel/CortexR52/evs.hh
@@ -37,7 +37,6 @@
#include "arch/arm/fastmodel/protocol/exported_clock_rate_control.hh"
#include "arch/arm/fastmodel/protocol/signal_interrupt.hh"
#include "dev/intpin.hh"
-#include "dev/reset_port.hh"
#include "mem/port_proxy.hh"
#include "params/FastModelScxEvsCortexR52x1.hh"
#include "params/FastModelScxEvsCortexR52x2.hh"
@@ -47,6 +46,7 @@
#include "scx_evs_CortexR52x2.h"
#include "scx_evs_CortexR52x3.h"
#include "scx_evs_CortexR52x4.h"
+#include "sim/signal.hh"
#include "systemc/ext/core/sc_event.hh"
#include "systemc/ext/core/sc_module.hh"
#include "systemc/tlm_port_wrapper.hh"
@@ -127,7 +127,7 @@
SignalSender dbg_reset;
- ResetResponsePort<ScxEvsCortexR52> model_reset;
+ SignalSinkPort<bool> model_reset;
CortexR52Cluster *gem5CpuCluster;
@@ -149,22 +149,6 @@
this->signalInterrupt->spi(num, false);
}
- void
- requestReset()
- {
- // Reset all cores.
- for (auto &core_pin : corePins) {
- core_pin->poweron_reset.signal_out.set_state(0, true);
- core_pin->poweron_reset.signal_out.set_state(0, false);
- }
- // Reset L2 system.
- this->top_reset.signal_out.set_state(0, true);
- this->top_reset.signal_out.set_state(0, false);
- // Reset debug APB.
- this->dbg_reset.signal_out.set_state(0, true);
- this->dbg_reset.signal_out.set_state(0, false);
- }
-
Port &gem5_getPort(const std::string &if_name, int idx) override;
void
diff --git a/src/arch/arm/fastmodel/reset_controller/example.cc
b/src/arch/arm/fastmodel/reset_controller/example.cc
index 33769ac..04dfa3b 100644
--- a/src/arch/arm/fastmodel/reset_controller/example.cc
+++ b/src/arch/arm/fastmodel/reset_controller/example.cc
@@ -37,8 +37,8 @@
{
ResetControllerExample::CorePins::CorePins(const std::string &module_name)
- : reset(module_name + ".reset", 0, this),
- halt(module_name + ".halt", 0, this)
+ : reset(module_name + ".reset"),
+ halt(module_name + ".halt")
{}
ResetControllerExample::Registers::Registers(
@@ -65,22 +65,14 @@
{
panic_if(!pins->reset.isConnected(),
"%s is not connected.", pins->reset.name());
-
- if (val)
- pins->reset.raise();
- else
- pins->reset.lower();
+ pins->reset.set(val);
});
halt.writer(
[this] (auto ®, auto val)
{
panic_if(!pins->halt.isConnected(),
"%s is not connected.", pins->halt.name());
-
- if (val)
- pins->halt.raise();
- else
- pins->halt.lower();
+ pins->halt.set(val);
});
addRegisters({
diff --git a/src/arch/arm/fastmodel/reset_controller/example.hh
b/src/arch/arm/fastmodel/reset_controller/example.hh
index 2805d6f..af66323 100644
--- a/src/arch/arm/fastmodel/reset_controller/example.hh
+++ b/src/arch/arm/fastmodel/reset_controller/example.hh
@@ -31,11 +31,11 @@
#include <string>
#include "arch/arm/fastmodel/iris/cpu.hh"
-#include "dev/intpin.hh"
#include "dev/io_device.hh"
#include "dev/reg_bank.hh"
#include "mem/packet_access.hh"
#include "params/FastModelResetControllerExample.hh"
+#include "sim/signal.hh"
namespace gem5
{
@@ -48,9 +48,8 @@
private:
struct CorePins
{
- using CoreInt = IntSourcePin<CorePins>;
- CoreInt reset;
- CoreInt halt;
+ SignalSourcePort<bool> reset;
+ SignalSourcePort<bool> halt;
explicit CorePins(const std::string &);
};
diff --git a/src/dev/SConscript b/src/dev/SConscript
index d991ed5..a7714a2 100644
--- a/src/dev/SConscript
+++ b/src/dev/SConscript
@@ -36,9 +36,7 @@
Source('dma_virt_device.cc')
SimObject('IntPin.py', sim_objects=[])
-
SimObject('ResetPort.py', sim_objects=[])
-Source('reset_port.cc')
DebugFlag('IsaFake')
DebugFlag('DMA')
diff --git a/src/dev/reset_port.cc b/src/dev/reset_port.cc
deleted file mode 100644
index 8d32c7d..0000000
--- a/src/dev/reset_port.cc
+++ /dev/null
@@ -1,57 +0,0 @@
-/*
- * Copyright 2022 Google, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "dev/reset_port.hh"
-
-#include "base/logging.hh"
-
-namespace gem5
-{
-
-void
-ResetRequestPort::bind(Port &p)
-{
- peer = dynamic_cast<ResetResponsePortBase*>(&p);
- fatal_if(peer == nullptr, "Attempt to bind reset request port %s to "
- "incompatible port %s.", name(), p.name());
- Port::bind(p);
-}
-
-void
-ResetRequestPort::unbind()
-{
- peer = nullptr;
- Port::unbind();
-}
-
-void
-ResetRequestPort::requestReset()
-{
- peer->requestReset();
-}
-
-} // namespace gem5
diff --git a/src/dev/reset_port.hh b/src/dev/reset_port.hh
deleted file mode 100644
index a08db1c..0000000
--- a/src/dev/reset_port.hh
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * Copyright 2022 Google, Inc.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met: redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer;
- * redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution;
- * neither the name of the copyright holders nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef __DEV_RESET_PORT_HH__
-#define __DEV_RESET_PORT_HH__
-
-#include "sim/port.hh"
-
-#include <string>
-
-namespace gem5
-{
-
-class ResetResponsePortBase : public Port
-{
- public:
- using Port::Port;
- virtual void requestReset() = 0;
-};
-
-template <class Device>
-class ResetResponsePort : public ResetResponsePortBase
-{
- public:
- ResetResponsePort(const std::string &name, PortID id, Device *dev) :
- ResetResponsePortBase(name, id), device(dev) {}
- void requestReset() override { device->requestReset(); }
-
- private:
- Device *device = nullptr;
-};
-
-class ResetRequestPort : public Port
-{
- public:
- ResetRequestPort(const std::string &_name, PortID _id)
- : Port(_name, _id) {}
- void bind(Port &p) override;
- void unbind() override;
- void requestReset();
-
- private:
- ResetResponsePortBase *peer = nullptr;
-};
-
-} // namespace gem5
-
-#endif // __DEV_RESET_PORT_HH__
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/66675?usp=email
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: Id98bef901ab61ac5b200dbbe49439bb2d2e6c57f
Gerrit-Change-Number: 66675
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Gabe Black <gabebl...@google.com>
Gerrit-Reviewer: Jui-min Lee <f...@google.com>
Gerrit-Reviewer: Yu-hsin Wang <yuhsi...@google.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
