[gem5-dev] Change in gem5/gem5[develop]: mem: Re-remove the arch/isa_traits.hh include in the base prefetcher.
Gabe Black has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/34166 ) Change subject: mem: Re-remove the arch/isa_traits.hh include in the base prefetcher. .. mem: Re-remove the arch/isa_traits.hh include in the base prefetcher. This was removed but then accidentally re-added by a following change, probably from a slighly incorrect rebase. Change-Id: Ia7e8c755f92343c8b5e82febea2c1db4683fa69a Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34166 Reviewed-by: Daniel Carvalho Reviewed-by: Nikos Nikoleris Maintainer: Gabe Black Tested-by: kokoro --- M src/mem/cache/prefetch/base.hh 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: Nikos Nikoleris: Looks good to me, approved Daniel Carvalho: Looks good to me, approved Gabe Black: Looks good to me, approved kokoro: Regressions pass diff --git a/src/mem/cache/prefetch/base.hh b/src/mem/cache/prefetch/base.hh index 5f7ad81..cdd0f5b 100644 --- a/src/mem/cache/prefetch/base.hh +++ b/src/mem/cache/prefetch/base.hh @@ -49,7 +49,6 @@ #include #include "arch/generic/tlb.hh" -#include "arch/isa_traits.hh" #include "base/statistics.hh" #include "base/types.hh" #include "mem/packet.hh" -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34166 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: Ia7e8c755f92343c8b5e82febea2c1db4683fa69a Gerrit-Change-Number: 34166 Gerrit-PatchSet: 3 Gerrit-Owner: Gabe Black Gerrit-Reviewer: Daniel Carvalho Gerrit-Reviewer: Eden Avivi Gerrit-Reviewer: Gabe Black Gerrit-Reviewer: Jason Lowe-Power Gerrit-Reviewer: Nikos Nikoleris 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]: mem: Add HTM fields to Request
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/30316 )
Change subject: mem: Add HTM fields to Request
..
mem: Add HTM fields to Request
This starts the support of Hardware Transactional Memory on the mem side
* The following flags have been added:
HTM_START: The request starts a HTM transaction
HTM_COMMIT: The request commits a HTM transaction
HTM_CANCEL: The request cancels a HTM transaction
HTM_ABORT: The request aborts a HTM transaction
* The following fields have been added:
_instCount: The instruction count at the time this request is created
_htmAbortCause: The cause for HTM transaction abort
https://gem5.atlassian.net/browse/GEM5-587
Change-Id: Ic582a6566fdd23f30eb92723e629d0c4d4ca10e5
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30316
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/mem/request.hh
1 file changed, 96 insertions(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/mem/request.hh b/src/mem/request.hh
index 7b324dc..7f0ddcb 100644
--- a/src/mem/request.hh
+++ b/src/mem/request.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2013,2017-2019 ARM Limited
+ * Copyright (c) 2012-2013,2017-2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -56,6 +56,7 @@
#include "base/logging.hh"
#include "base/types.hh"
#include "cpu/inst_seq.hh"
+#include "mem/htm.hh"
#include "sim/core.hh"
/**
@@ -187,6 +188,42 @@
/** Bits to define the destination of a request */
DST_BITS= 0x0030,
+/** hardware transactional memory **/
+
+/** The request starts a HTM transaction */
+HTM_START = 0x0100,
+
+/** The request commits a HTM transaction */
+HTM_COMMIT = 0x0200,
+
+/** The request cancels a HTM transaction */
+HTM_CANCEL = 0x0400,
+
+/** The request aborts a HTM transaction */
+HTM_ABORT = 0x0800,
+
+// What is the different between HTM cancel and abort?
+//
+// HTM_CANCEL will originate from a user instruction, e.g.
+// Arm's TCANCEL or x86's XABORT. This is an explicit request
+// to end a transaction and restore from the last checkpoint.
+//
+// HTM_ABORT is an internally generated request used to synchronize
+// a transaction's failure between the core and memory subsystem.
+// If a transaction fails in the core, e.g. because an instruction
+// within the transaction generates an exception, the core will
prepare
+// itself to stop fetching/executing more instructions and send an
+// HTM_ABORT to the memory subsystem before restoring the
checkpoint.
+// Similarly, the transaction could fail in the memory subsystem
and
+// this will be communicated to the core via the Packet object.
+// Once the core notices, it will do the same as the above and send
+// a HTM_ABORT to the memory subsystem.
+// A HTM_CANCEL sent to the memory subsystem will ultimately return
+// to the core which in turn will send a HTM_ABORT.
+//
+// This separation is necessary to ensure the disjoint components
+// of the system work correctly together.
+
/**
* These flags are *not* cleared when a Request object is
* reused (assigned a new address).
@@ -196,6 +233,9 @@
static const FlagsType STORE_NO_DATA = CACHE_BLOCK_ZERO |
CLEAN | INVALIDATE;
+static const FlagsType HTM_CMD = HTM_START | HTM_COMMIT |
+HTM_CANCEL | HTM_ABORT;
+
/** Master Ids that are statically allocated
* @{*/
enum : MasterID {
@@ -274,6 +314,11 @@
/** Whether or not the stream ID and substream ID is valid. */
VALID_STREAM_ID = 0x0100,
VALID_SUBSTREAM_ID = 0x0200,
+// hardware transactional memory
+/** Whether or not the abort cause is valid. */
+VALID_HTM_ABORT_CAUSE = 0x0400,
+/** Whether or not the instruction count is valid. */
+VALID_INST_COUNT = 0x0800,
/**
* These flags are *not* cleared when a Request object is reused
* (assigned a new address).
@@ -362,6 +407,12 @@
LocalAccessor _localAccessor;
+/** The instruction count at the time this request is created */
+Counter _instCount = 0;
+
+/** The cause for HTM transaction abort */
+HtmFailureFaultCause _htmAbortCause = HtmFailureFaultCause::INVALID;
+
public:
/**
@@ -517,6 +568,21 @@
[gem5-dev] Change in gem5/gem5[develop]: cpu: Add HTM Instruction Flags
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/30321 )
Change subject: cpu: Add HTM Instruction Flags
..
cpu: Add HTM Instruction Flags
IsHtmStart: Starts a HTM transaction
IsHtmStop: Stops (commits) a HTM transaction
IsHtmCancel: Explicitely aborts a HTM transaction
JIRA: https://gem5.atlassian.net/browse/GEM5-587
Change-Id: I33144f97a2009e28b0c64777f0313cd6eadb7ff9
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30321
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/cpu/StaticInstFlags.py
M src/cpu/static_inst.hh
2 files changed, 19 insertions(+), 2 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/StaticInstFlags.py b/src/cpu/StaticInstFlags.py
index f756ba1..1c2b63a 100644
--- a/src/cpu/StaticInstFlags.py
+++ b/src/cpu/StaticInstFlags.py
@@ -1,3 +1,4 @@
+# Copyright (c) 2020 ARM Limited
# Copyright (c) 2003-2005 The Regents of The University of Michigan
# Copyright (c) 2013 Advanced Micro Devices, Inc.
# All rights reserved.
@@ -109,5 +110,9 @@
'IsMicroBranch',# This microop branches within the microcode
for
# a macroop
'IsDspOp',
-'IsSquashAfter' # Squash all uncommitted state after executed
+'IsSquashAfter', # Squash all uncommitted state after executed
+# hardware transactional memory
+'IsHtmStart', # Starts a HTM transaction
+'IsHtmStop',# Stops (commits) a HTM transaction
+'IsHtmCancel' # Explicitely aborts a HTM transaction
]
diff --git a/src/cpu/static_inst.hh b/src/cpu/static_inst.hh
index b523ef9..146be8c 100644
--- a/src/cpu/static_inst.hh
+++ b/src/cpu/static_inst.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2017 ARM Limited
+ * Copyright (c) 2017, 2020 ARM Limited
* All rights reserved
*
* The license below extends only to copyright in the software and shall
@@ -202,6 +202,18 @@
bool isFirstMicroop() const { return flags[IsFirstMicroop]; }
//This flag doesn't do anything yet
bool isMicroBranch() const { return flags[IsMicroBranch]; }
+// hardware transactional memory
+// HtmCmds must be identified as such in order
+// to provide them with necessary memory ordering semantics.
+bool isHtmStart() const { return flags[IsHtmStart]; }
+bool isHtmStop() const { return flags[IsHtmStop]; }
+bool isHtmCancel() const { return flags[IsHtmCancel]; }
+
+bool
+isHtmCmd() const
+{
+return isHtmStart() || isHtmStop() || isHtmCancel();
+}
//@}
void setFirstMicroop() { flags[IsFirstMicroop] = true; }
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30321
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: I33144f97a2009e28b0c64777f0313cd6eadb7ff9
Gerrit-Change-Number: 30321
Gerrit-PatchSet: 9
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Alexandru Duțu
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Timothy Hayes
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]: cpu: Add HTM CPU API
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/30322 )
Change subject: cpu: Add HTM CPU API
..
cpu: Add HTM CPU API
JIRA: https://gem5.atlassian.net/browse/GEM5-587
Change-Id: Iff95eb97603b4cb9629c04382a824b02594ee5c7
Signed-off-by: Giacomo Travaglini
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30322
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/cpu/o3/cpu.cc
M src/cpu/o3/cpu.hh
M src/cpu/simple/atomic.hh
M src/cpu/simple/base.hh
M src/cpu/simple/timing.cc
M src/cpu/simple/timing.hh
6 files changed, 62 insertions(+), 4 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/o3/cpu.cc b/src/cpu/o3/cpu.cc
index 562a332..613ffd1 100644
--- a/src/cpu/o3/cpu.cc
+++ b/src/cpu/o3/cpu.cc
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011-2012, 2014, 2016, 2017, 2019 ARM Limited
+ * Copyright (c) 2011-2012, 2014, 2016, 2017, 2019-2020 ARM Limited
* Copyright (c) 2013 Advanced Micro Devices, Inc.
* All rights reserved
*
@@ -1827,5 +1827,13 @@
}
}
+template
+void
+FullO3CPU::htmSendAbortSignal(ThreadID tid, uint64_t htmUid,
+ HtmFailureFaultCause cause)
+{
+panic("not yet supported!");
+}
+
// Forward declaration of FullO3CPU.
template class FullO3CPU;
diff --git a/src/cpu/o3/cpu.hh b/src/cpu/o3/cpu.hh
index cc0e2cd..137fbc8 100644
--- a/src/cpu/o3/cpu.hh
+++ b/src/cpu/o3/cpu.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011-2013, 2016-2019 ARM Limited
+ * Copyright (c) 2011-2013, 2016-2020 ARM Limited
* Copyright (c) 2013 Advanced Micro Devices, Inc.
* All rights reserved
*
@@ -788,6 +788,11 @@
//number of misc
Stats::Scalar miscRegfileReads;
Stats::Scalar miscRegfileWrites;
+
+ public:
+// hardware transactional memory
+void htmSendAbortSignal(ThreadID tid, uint64_t htm_uid,
+HtmFailureFaultCause cause);
};
#endif // __CPU_O3_CPU_HH__
diff --git a/src/cpu/simple/atomic.hh b/src/cpu/simple/atomic.hh
index 7333e1f..2d0a465 100644
--- a/src/cpu/simple/atomic.hh
+++ b/src/cpu/simple/atomic.hh
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2012-2013, 2015, 2018 ARM Limited
+ * Copyright (c) 2012-2013, 2015, 2018, 2020 ARM Limited
* All rights reserved.
*
* The license below extends only to copyright in the software and shall
@@ -219,6 +219,18 @@
const std::vector& byte_enable =
std::vector())
override;
+Fault initiateHtmCmd(Request::Flags flags) override
+{
+panic("initiateHtmCmd() is for timing accesses, and should "
+ "never be called on AtomicSimpleCPU.\n");
+}
+
+void htmSendAbortSignal(HtmFailureFaultCause cause) override
+{
+panic("htmSendAbortSignal() is for timing accesses, and should "
+ "never be called on AtomicSimpleCPU.\n");
+}
+
Fault writeMem(uint8_t *data, unsigned size,
Addr addr, Request::Flags flags, uint64_t *res,
const std::vector& byte_enable =
std::vector())
diff --git a/src/cpu/simple/base.hh b/src/cpu/simple/base.hh
index 9f5bf66..82f52d9 100644
--- a/src/cpu/simple/base.hh
+++ b/src/cpu/simple/base.hh
@@ -176,6 +176,21 @@
void serializeThread(CheckpointOut , ThreadID tid) const override;
void unserializeThread(CheckpointIn , ThreadID tid) override;
+/** Hardware transactional memory commands (HtmCmds), e.g. start a
+ * transaction and commit a transaction, are memory operations but are
+ * neither really (true) loads nor stores. For this reason the
interface
+ * is extended and initiateHtmCmd() is used to instigate the command.
*/
+virtual Fault initiateHtmCmd(Request::Flags flags) = 0;
+
+/** This function is used to instruct the memory subsystem that a
+ * transaction should be aborted and the speculative state should be
+ * thrown away. This is called in the transaction's very last breath
in
+ * the core. Afterwards, the core throws away its speculative state
and
+ * resumes execution at the point the transaction started, i.e.
reverses
+ * time. When instruction execution resumes, the core expects the
+ * memory subsystem to be in a stable, i.e. pre-speculative, state as
+ * well. */
+virtual void htmSendAbortSignal(HtmFailureFaultCause cause) = 0;
};
#endif // __CPU_SIMPLE_BASE_HH__
diff --git a/src/cpu/simple/timing.cc b/src/cpu/simple/timing.cc
index 84d7d0e..d3adbcc 100644
--- a/src/cpu/simple/timing.cc
+++ b/src/cpu/simple/timing.cc
@@ -1055,6 +1055,19 @@
dcachePort.printAddr(a);
}
+Fault
+TimingSimpleCPU::initiateHtmCmd(Request::Flags flags)
+{
+panic("not yet supported!");
+return NoFault;
+}
+
+void
+TimingSimpleCPU::htmSendAbortSignal(HtmFailureFaultCause cause)
+{
+panic("not yet
[gem5-dev] Change in gem5/gem5[develop]: cpu: Add HtmCpu DebugFlag
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/30320 )
Change subject: cpu: Add HtmCpu DebugFlag
..
cpu: Add HtmCpu DebugFlag
JIRA: https://gem5.atlassian.net/browse/GEM5-587
Change-Id: Id4b86b8964bc64bce1d2e4af941217eb114f3cc4
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/30320
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/cpu/SConscript
1 file changed, 13 insertions(+), 0 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/cpu/SConscript b/src/cpu/SConscript
index dea7e92..194631a 100644
--- a/src/cpu/SConscript
+++ b/src/cpu/SConscript
@@ -1,5 +1,17 @@
# -*- mode:python -*-
+# Copyright (c) 2020 ARM Limited
+# All rights reserved.
+#
+# The license below extends only to copyright in the software and shall
+# not be construed as granting a license to any other intellectual
+# property including but not limited to intellectual property relating
+# to a hardware implementation of the functionality of the software
+# licensed hereunder. You may use the software subject to the license
+# terms below provided that you ensure that this notice is replicated
+# unmodified and in its entirety in all distributions of the software,
+# modified or unmodified, in source code or in binary form.
+#
# Copyright (c) 2006 The Regents of The University of Michigan
# All rights reserved.
#
@@ -51,6 +63,7 @@
DebugFlag('ExecAsid', 'Format: Include ASID in trace')
DebugFlag('ExecFlags', 'Format: Include instruction flags in trace')
DebugFlag('Fetch')
+DebugFlag('HtmCpu', 'Hardware Transactional Memory (CPU side)')
DebugFlag('IntrControl')
DebugFlag('O3PipeView')
DebugFlag('PCEvent')
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/30320
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: Id4b86b8964bc64bce1d2e4af941217eb114f3cc4
Gerrit-Change-Number: 30320
Gerrit-PatchSet: 9
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Alexandru Duțu
Gerrit-Reviewer: Giacomo Travaglini
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Timothy Hayes
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] Re: Use of page size and TLBs in the prefetchers
Hi Gabe, I agree with Isaac, some prefetchers use the page size to avoid crossing page boundaries. These are prefetchers operate on the PA space and have no access to the TLB to avoid the extra complexity or because they are far from the core (e.g., L3 prefetcher). Some other prefetchers operate on the VA space and once they determine the VA of the block they will prefetch, they use the TLB to map it to a PA. These prefetchers are always connected to a TLB. Unfortunately, we can't really assume that all prefetchers are connected to a TLB. Nikos On 07/09/2020 10:03, Isaac Sánchez Barrera wrote: Hi, I'm not using Ruby, so just talking about classic. Before the code included support for the TLBs, the prefetchers used the page size to detect page-crossing prefetches in order to discard them. Now it uses that to decide if it can do a direct prefetch or it needs to check the TLB for a translation of the address *if it has a TLB*. See `Base::samePage` (`src/mem/cache/prefetch/base.cc`) and an example of its use at https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/mem/cache/prefetch/queued.cc#180 (`src/mem/cache/prefetch/queued.cc` line 180) Would there be any other option without having to include the TLB in all prefetchers? Cheers, Isaac El 7/9/20 a las 10:06, Gabe Black escribió: Hi folks. I've *almost* eliminated use of the getPageBytes and getPageShift functions in the System class, which in combination with a change from Andreas will eliminate the need for the isa_traits.hh switching header file. The only use left is in the Ruby and cache prefetchers: mem/cache/prefetch/base.cc: masterId(p->sys->getMasterId(this)), pageBytes(p->sys->getPageBytes()), mem/ruby/structures/RubyPrefetcher.cc: m_page_shift(p->sys->getPageShift()) I see that the cache prefetcher has a list of TLBs, and one thing I've done is add a method to the TLB class(es) that returns the current page size. Is it safe to assume one of these prefetchers will *necessarily* have a TLB assigned to it? That seems plausible since if it doesn't, why does it care about page sizes? But I don't understand that code well enough to determine that, especially if it's some corner case. The Ruby prefetcher doesn't *seem* to have a TLB attached to it? Although I don't really know how Ruby works. What's going on there? Any idea why it needs to know the page size (I can guess, but don't know for sure), and what plays the role of the TLB in that case? Gabe IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ___ 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]: base: Minor cleanup of the ChunkGenerator.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34177 )
Change subject: base: Minor cleanup of the ChunkGenerator.
..
base: Minor cleanup of the ChunkGenerator.
Minor style fixes, switched to Addr for some types so they'll definitely
be large enough.
Change-Id: I985004116c48ce6fb236c04e04fe54ed49a68277
---
M src/base/chunk_generator.hh
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/src/base/chunk_generator.hh b/src/base/chunk_generator.hh
index d1378aa..3873a66 100644
--- a/src/base/chunk_generator.hh
+++ b/src/base/chunk_generator.hh
@@ -26,8 +26,8 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
-#ifndef __BASE__CHUNK_GENERATOR_HH__
-#define __BASE__CHUNK_GENERATOR_HH__
+#ifndef __BASE_CHUNK_GENERATOR_HH__
+#define __BASE_CHUNK_GENERATOR_HH__
/**
* @file
@@ -60,13 +60,13 @@
/** The starting address of the next chunk (after the current one). */
Addr nextAddr;
/** The size of the current chunk (in bytes). */
-unsigned curSize;
+Addr curSize;
/** The number of bytes remaining in the region after the current
chunk. */
-unsigned sizeLeft;
+Addr sizeLeft;
/** The start address so we can calculate offset in writing block. */
const Addr startAddr;
/** The maximum chunk size, e.g., the cache block size or page size. */
-const unsigned chunkSize;
+const Addr chunkSize;
public:
/**
@@ -76,8 +76,8 @@
* @param _chunkSize The size/alignment of chunks into which
*the region should be decomposed.
*/
-ChunkGenerator(Addr _startAddr, unsigned totalSize, unsigned
_chunkSize)
-: startAddr(_startAddr), chunkSize(_chunkSize)
+ChunkGenerator(Addr _startAddr, Addr totalSize, Addr _chunkSize) :
+startAddr(_startAddr), chunkSize(_chunkSize)
{
// chunkSize must be a power of two
assert(chunkSize == 0 || isPowerOf2(chunkSize));
@@ -85,13 +85,10 @@
// set up initial chunk.
curAddr = startAddr;
-if (chunkSize == 0) //Special Case, if we see 0, assume no
chuncking
-{
+if (chunkSize == 0) { // Special Case, if we see 0, assume no
chunking.
nextAddr = startAddr + totalSize;
-}
-else
-{
-// nextAddr should be *next* chunk start
+} else {
+// nextAddr should be *next* chunk start.
nextAddr = roundUp(startAddr, chunkSize);
if (curAddr == nextAddr) {
// ... even if startAddr is already chunk-aligned
@@ -99,8 +96,8 @@
}
}
-// how many bytes are left between curAddr and the end of this
chunk?
-unsigned left_in_chunk = nextAddr - curAddr;
+// How many bytes are left between curAddr and the end of this
chunk?
+Addr left_in_chunk = nextAddr - curAddr;
curSize = std::min(totalSize, left_in_chunk);
sizeLeft = totalSize - curSize;
}
@@ -108,23 +105,23 @@
/** Return starting address of current chunk. */
Addr addr() const { return curAddr; }
/** Return size in bytes of current chunk. */
-unsigned size() const { return curSize; }
+Addr size() const { return curSize; }
/** Number of bytes we have already chunked up. */
-unsigned complete() const { return curAddr - startAddr; }
+Addr complete() const { return curAddr - startAddr; }
/**
* Are we done? That is, did the last call to next() advance
* past the end of the region?
* @return True if yes, false if more to go.
*/
-bool done() const { return (curSize == 0); }
+bool done() const { return !curSize; }
/**
* Is this the last chunk?
* @return True if yes, false if more to go.
*/
-bool last() const { return (sizeLeft == 0); }
+bool last() const { return !sizeLeft; }
/**
* Advance generator to next chunk.
@@ -134,7 +131,7 @@
bool
next()
{
-if (sizeLeft == 0) {
+if (last()) {
curSize = 0;
return false;
}
@@ -147,4 +144,4 @@
}
};
-#endif // __BASE__CHUNK_GENERATOR_HH__
+#endif // __BASE_CHUNK_GENERATOR_HH__
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34177
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: I985004116c48ce6fb236c04e04fe54ed49a68277
Gerrit-Change-Number: 34177
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]: dev: Stop using the OS page size in the IDE controller.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34178 )
Change subject: dev: Stop using the OS page size in the IDE controller.
..
dev: Stop using the OS page size in the IDE controller.
This size was used to break up DMA transactions so that a single
transaction would not cross a page boundary. This was because on Alpha,
there was an actual page table which translated between PCI and DMA
address spaces. On all currently implemented systems, the mapping is
simply to add a scalar offset, so it's not possible for a legal region
of memory to be contiguous in one space but not in the other.
Additionally, if it *was* possible for there to be a mismatch, it was
only coincidence that Alpha used a page table which had the same sized
pages as it normally used. There is no requirement that there even would
be fixed sized pages in the first place.
To avoid this artificial dependency between the IDE controller and the
ISA, this change simply changes the chunk size for DMA accesses to 4K.
That's the page size at least on x86 and probably other architectures,
and will be a pretty close approximation of the previous behavior.
It's possible that even having this chunking in the first place is
unnecessary and functionally useless, but there are some checks which
happen between chunks, and changing how big they are would change the
frequency of those checks. For instance, the controller/disk may not
notice in the same amount of time if a DMA was cancelled somehow.
Change-Id: I1ec840d1f158c3faa31ba0184458b69bf654c252
---
M src/dev/storage/ide_ctrl.cc
M src/dev/storage/ide_disk.cc
M src/dev/storage/ide_disk.hh
3 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/dev/storage/ide_ctrl.cc b/src/dev/storage/ide_ctrl.cc
index 632144c..cf69d34 100644
--- a/src/dev/storage/ide_ctrl.cc
+++ b/src/dev/storage/ide_ctrl.cc
@@ -120,7 +120,8 @@
panic("IDE controllers support a maximum "
"of 4 devices attached!\n");
}
-params()->disks[i]->setController(this, sys->getPageBytes());
+// Arbitrarily set the chunk size to 4K.
+params()->disks[i]->setController(this, 4 * 1024);
}
primary.select(false);
diff --git a/src/dev/storage/ide_disk.cc b/src/dev/storage/ide_disk.cc
index e97e23b..57fa076 100644
--- a/src/dev/storage/ide_disk.cc
+++ b/src/dev/storage/ide_disk.cc
@@ -435,7 +435,7 @@
// clear out the data buffer
memset(dataBuffer, 0, MAX_DMA_SIZE);
dmaReadCG = new ChunkGenerator(curPrd.getBaseAddr(),
-curPrd.getByteCount(), pageBytes);
+curPrd.getByteCount(), chunkBytes);
}
if (ctrl->dmaPending() || ctrl->drainState() != DrainState::Running) {
@@ -447,7 +447,7 @@
, dataBuffer + dmaReadCG->complete());
dmaReadBytes += dmaReadCG->size();
dmaReadTxs++;
-if (dmaReadCG->size() == pageBytes)
+if (dmaReadCG->size() == chunkBytes)
dmaReadFullPages++;
dmaReadCG->next();
} else {
@@ -518,7 +518,7 @@
if (!dmaWriteCG) {
// clear out the data buffer
dmaWriteCG = new ChunkGenerator(curPrd.getBaseAddr(),
-curPrd.getByteCount(), pageBytes);
+curPrd.getByteCount(), chunkBytes);
}
if (ctrl->dmaPending() || ctrl->drainState() != DrainState::Running) {
schedule(dmaWriteWaitEvent, curTick() + DMA_BACKOFF_PERIOD);
@@ -532,7 +532,7 @@
curPrd.getByteCount(), curPrd.getEOT());
dmaWriteBytes += dmaWriteCG->size();
dmaWriteTxs++;
-if (dmaWriteCG->size() == pageBytes)
+if (dmaWriteCG->size() == chunkBytes)
dmaWriteFullPages++;
dmaWriteCG->next();
} else {
diff --git a/src/dev/storage/ide_disk.hh b/src/dev/storage/ide_disk.hh
index 8a90d1c..25c6566 100644
--- a/src/dev/storage/ide_disk.hh
+++ b/src/dev/storage/ide_disk.hh
@@ -239,8 +239,8 @@
DmaState_t dmaState;
/** Dma transaction is a read */
bool dmaRead;
-/** Size of OS pages. */
-Addr pageBytes;
+/** Size of chunks to DMA. */
+Addr chunkBytes;
/** PRD table base address */
uint32_t curPrdAddr;
/** PRD entry */
@@ -283,11 +283,11 @@
* @param c The IDE controller
*/
void
-setController(IdeController *c, Addr page_bytes)
+setController(IdeController *c, Addr chunk_bytes)
{
panic_if(ctrl, "Cannot change the controller once set!\n");
ctrl = c;
-pageBytes = page_bytes;
+chunkBytes = chunk_bytes;
}
// Device register read/write
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34178
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:
[gem5-dev] Change in gem5/gem5[develop]: mem: Use the page size from the TLB in the translating port proxy.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34176 )
Change subject: mem: Use the page size from the TLB in the translating port
proxy.
..
mem: Use the page size from the TLB in the translating port proxy.
Change-Id: I5ef60f2d24de376b84750a78a31a4148bbf6fbdb
---
M src/mem/translating_port_proxy.cc
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/mem/translating_port_proxy.cc
b/src/mem/translating_port_proxy.cc
index 8bb93cc..e0661fa 100644
--- a/src/mem/translating_port_proxy.cc
+++ b/src/mem/translating_port_proxy.cc
@@ -54,8 +54,7 @@
ThreadContext *tc, Request::Flags _flags) :
PortProxy(tc->getCpuPtr()->getSendFunctional(),
tc->getSystemPtr()->cacheLineSize()), _tc(tc),
- pageBytes(tc->getSystemPtr()->getPageBytes()),
- flags(_flags)
+ pageBytes(tc->getDTBPtr()->pageSize()), flags(_flags)
{}
bool
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34176
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: I5ef60f2d24de376b84750a78a31a4148bbf6fbdb
Gerrit-Change-Number: 34176
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,sim: Get the page size from the page table in SE mode.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34172 )
Change subject: mem,sim: Get the page size from the page table in SE mode.
..
mem,sim: Get the page size from the page table in SE mode.
The page table already knows the size of a page without having to
directly use any ISA specific constants.
Change-Id: I68b575e194697065620a2097d972076886766f74
---
M src/mem/multi_level_page_table.hh
M src/mem/page_table.cc
M src/mem/page_table.hh
M src/sim/mem_state.cc
M src/sim/process.cc
M src/sim/syscall_emul.cc
M src/sim/syscall_emul.hh
7 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/src/mem/multi_level_page_table.hh
b/src/mem/multi_level_page_table.hh
index 68a32b1..3d9ca9b 100644
--- a/src/mem/multi_level_page_table.hh
+++ b/src/mem/multi_level_page_table.hh
@@ -192,8 +192,8 @@
public:
MultiLevelPageTable(const std::string &__name, uint64_t _pid,
-System *_sys, Addr pageSize) :
-EmulationPageTable(__name, _pid, pageSize), system(_sys)
+System *_sys, Addr _pageSize) :
+EmulationPageTable(__name, _pid, _pageSize), system(_sys)
{}
~MultiLevelPageTable() {}
@@ -204,7 +204,7 @@
if (shared)
return;
-_basePtr = prepTopTable(system, pageSize);
+_basePtr = prepTopTable(system, _pageSize);
}
Addr basePtr() { return _basePtr; }
@@ -216,8 +216,8 @@
Final entry;
-for (int64_t offset = 0; offset < size; offset += pageSize) {
-walk(system, pageSize, _basePtr,
+for (int64_t offset = 0; offset < size; offset += _pageSize) {
+walk(system, _pageSize, _basePtr,
vaddr + offset, true, );
entry.reset(paddr + offset, true, flags & Uncacheable,
@@ -236,16 +236,16 @@
Final old_entry, new_entry;
-for (int64_t offset = 0; offset < size; offset += pageSize) {
+for (int64_t offset = 0; offset < size; offset += _pageSize) {
// Unmap the original mapping.
-walk(system, pageSize, _basePtr, vaddr + offset,
+walk(system, _pageSize, _basePtr, vaddr +
offset,
false, _entry);
old_entry.present(false);
old_entry.write(system->physProxy);
// Map the new one.
-walk(system, pageSize, _basePtr, new_vaddr +
offset,
-true, _entry);
+walk(system, _pageSize, _basePtr,
+new_vaddr + offset, true, _entry);
new_entry.reset(old_entry.paddr(), true,
old_entry.uncacheable(),
old_entry.readonly());
new_entry.write(system->physProxy);
@@ -259,8 +259,8 @@
Final entry;
-for (int64_t offset = 0; offset < size; offset += pageSize) {
-walk(system, pageSize, _basePtr,
+for (int64_t offset = 0; offset < size; offset += _pageSize) {
+walk(system, _pageSize, _basePtr,
vaddr + offset, false, );
fatal_if(!entry.present(),
"PageTable::unmap: Address %#x not mapped.", vaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index 601b9c5..d385ab1 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -62,9 +62,9 @@
pTable.emplace(vaddr, Entry(paddr, flags));
}
-size -= pageSize;
-vaddr += pageSize;
-paddr += pageSize;
+size -= _pageSize;
+vaddr += _pageSize;
+paddr += _pageSize;
}
}
@@ -84,9 +84,9 @@
pTable.emplace(new_vaddr, old_it->second);
pTable.erase(old_it);
-size -= pageSize;
-vaddr += pageSize;
-new_vaddr += pageSize;
+size -= _pageSize;
+vaddr += _pageSize;
+new_vaddr += _pageSize;
}
}
@@ -108,8 +108,8 @@
auto it = pTable.find(vaddr);
assert(it != pTable.end());
pTable.erase(it);
-size -= pageSize;
-vaddr += pageSize;
+size -= _pageSize;
+vaddr += _pageSize;
}
}
@@ -119,7 +119,7 @@
// starting address must be page aligned
assert(pageOffset(vaddr) == 0);
-for (int64_t offset = 0; offset < size; offset += pageSize)
+for (int64_t offset = 0; offset < size; offset += _pageSize)
if (pTable.find(vaddr + offset) != pTable.end())
return false;
@@ -158,7 +158,7 @@
if (!translate(req->getVaddr(), paddr))
return Fault(new GenericPageTableFault(req->getVaddr()));
req->setPaddr(paddr);
-if ((paddr & (pageSize - 1)) + req->getSize() > pageSize) {
+if ((paddr & (_pageSize - 1)) + req->getSize() > _pageSize) {
panic("Request spans page boundaries!\n");
return NoFault;
}
diff
[gem5-dev] Change in gem5/gem5[develop]: dev,arm: Use the ArmSystem::PageBytes constant in the generic timer.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34171 )
Change subject: dev,arm: Use the ArmSystem::PageBytes constant in the
generic timer.
..
dev,arm: Use the ArmSystem::PageBytes constant in the generic timer.
This component very specific to ARM, and so there's no reason to use
generic interfaces to get the page size.
Change-Id: Id757b5742c807c5f616a6dc8df94a7709932d071
---
M src/dev/arm/generic_timer.cc
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/dev/arm/generic_timer.cc b/src/dev/arm/generic_timer.cc
index da8de08..75ef38f 100644
--- a/src/dev/arm/generic_timer.cc
+++ b/src/dev/arm/generic_timer.cc
@@ -873,7 +873,7 @@
GenericTimerFrame::GenericTimerFrame(GenericTimerFrameParams *const p)
: PioDevice(p),
- timerRange(RangeSize(p->cnt_base, sys->getPageBytes())),
+ timerRange(RangeSize(p->cnt_base, ArmSystem::PageBytes)),
addrRanges({timerRange}),
systemCounter(*p->counter),
physTimer(csprintf("%s.phys_timer", name()),
@@ -887,7 +887,7 @@
SystemCounter::validateCounterRef(p->counter);
// Expose optional CNTEL0Base register frame
if (p->cnt_el0_base != MaxAddr) {
-timerEl0Range = RangeSize(p->cnt_el0_base, sys->getPageBytes());
+timerEl0Range = RangeSize(p->cnt_el0_base, ArmSystem::PageBytes);
accessBitsEl0 = 0x303;
addrRanges.push_back(timerEl0Range);
}
@@ -1244,9 +1244,9 @@
GenericTimerMem::GenericTimerMem(GenericTimerMemParams *const p)
: PioDevice(p),
- counterCtrlRange(RangeSize(p->cnt_control_base,
sys->getPageBytes())),
- counterStatusRange(RangeSize(p->cnt_read_base, sys->getPageBytes())),
- timerCtrlRange(RangeSize(p->cnt_ctl_base, sys->getPageBytes())),
+ counterCtrlRange(RangeSize(p->cnt_control_base,
ArmSystem::PageBytes)),
+ counterStatusRange(RangeSize(p->cnt_read_base,
ArmSystem::PageBytes)),
+ timerCtrlRange(RangeSize(p->cnt_ctl_base, ArmSystem::PageBytes)),
cnttidr(0x0),
addrRanges{counterCtrlRange, counterStatusRange, timerCtrlRange},
systemCounter(*p->counter),
@@ -1273,7 +1273,7 @@
void
GenericTimerMem::validateFrameRange(const AddrRange )
{
-fatal_if(range.start() % ArmISA::PageBytes,
+fatal_if(range.start() % ArmSystem::PageBytes,
"GenericTimerMem::validateFrameRange: Architecture states
each "
"register frame should be in a separate memory page,
specified "
"range base address [0x%x] is not compliant\n");
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34171
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: Id757b5742c807c5f616a6dc8df94a7709932d071
Gerrit-Change-Number: 34171
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: Report the page size from the TLB.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34175 )
Change subject: arch: Report the page size from the TLB.
..
arch: Report the page size from the TLB.
The TLB knows what the (a?) page size is for its architecture. Make that
queriable. Really there are usually multiple page sizes so this isn't a
very accurate way to do things, but it's consistent with the existing
mechanism.
Change-Id: I0f3afdf6a3db7bfad12accfd90c6edb183c20c8f
---
M src/arch/arm/fastmodel/iris/tlb.hh
M src/arch/arm/tlb.hh
M src/arch/generic/tlb.hh
M src/arch/mips/tlb.hh
M src/arch/power/tlb.hh
M src/arch/riscv/tlb.hh
M src/arch/sparc/tlb.hh
M src/arch/x86/tlb.hh
M src/sim/process.cc
9 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/arch/arm/fastmodel/iris/tlb.hh
b/src/arch/arm/fastmodel/iris/tlb.hh
index 1d9c216..1f7caed 100644
--- a/src/arch/arm/fastmodel/iris/tlb.hh
+++ b/src/arch/arm/fastmodel/iris/tlb.hh
@@ -28,6 +28,7 @@
#ifndef __ARCH_ARM_FASTMODEL_IRIS_TLB_HH__
#define __ARCH_ARM_FASTMODEL_IRIS_TLB_HH__
+#include "arch/arm/system.hh"
#include "arch/generic/tlb.hh"
namespace Iris
@@ -38,6 +39,8 @@
public:
TLB(const Params *p) : BaseTLB(p) {}
+Addr pageShift() const override { return ArmSystem::PageShift; }
+
void demapPage(Addr vaddr, uint64_t asn) override {}
void flushAll() override {}
void takeOverFrom(BaseTLB *otlb) override {}
diff --git a/src/arch/arm/tlb.hh b/src/arch/arm/tlb.hh
index 004ce0b..fc96ca7 100644
--- a/src/arch/arm/tlb.hh
+++ b/src/arch/arm/tlb.hh
@@ -45,6 +45,7 @@
#include "arch/arm/faults.hh"
#include "arch/arm/isa_traits.hh"
#include "arch/arm/pagetable.hh"
+#include "arch/arm/system.hh"
#include "arch/arm/utility.hh"
#include "arch/generic/tlb.hh"
#include "base/statistics.hh"
@@ -215,6 +216,8 @@
virtual ~TLB();
+Addr pageShift() const override { return ArmSystem::PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override;
/// setup all the back pointers
diff --git a/src/arch/generic/tlb.hh b/src/arch/generic/tlb.hh
index f144f69..0c5b8e2 100644
--- a/src/arch/generic/tlb.hh
+++ b/src/arch/generic/tlb.hh
@@ -85,6 +85,12 @@
};
public:
+virtual Addr pageShift() const = 0;
+Addr pageSize() const { return 0x1ULL << pageShift(); }
+Addr pageOffset(Addr a) const { return a & (pageSize() - 1); }
+Addr pageAlign(Addr a) const { return a & ~(pageSize() - 1); }
+Addr pageAlignUp(Addr a) const { return pageAlign(a + pageSize() - 1);
}
+
virtual void demapPage(Addr vaddr, uint64_t asn) = 0;
virtual Fault translateAtomic(
diff --git a/src/arch/mips/tlb.hh b/src/arch/mips/tlb.hh
index 2be2ddf..d9f6d42 100644
--- a/src/arch/mips/tlb.hh
+++ b/src/arch/mips/tlb.hh
@@ -69,6 +69,8 @@
MipsISA::PTE *getEntry(unsigned) const;
virtual ~TLB();
+Addr pageShift() const override { return PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override {}
int smallPages;
diff --git a/src/arch/power/tlb.hh b/src/arch/power/tlb.hh
index c119d93..954ff07 100644
--- a/src/arch/power/tlb.hh
+++ b/src/arch/power/tlb.hh
@@ -115,6 +115,8 @@
TLB(const Params *p);
virtual ~TLB();
+Addr pageShift() const override { return PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override {}
int probeEntry(Addr vpn,uint8_t) const;
diff --git a/src/arch/riscv/tlb.hh b/src/arch/riscv/tlb.hh
index a92bdd2..060cc53 100644
--- a/src/arch/riscv/tlb.hh
+++ b/src/arch/riscv/tlb.hh
@@ -87,6 +87,8 @@
Walker *getWalker();
+Addr pageShift() const override { return PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override {}
TlbEntry *insert(Addr vpn, const TlbEntry );
diff --git a/src/arch/sparc/tlb.hh b/src/arch/sparc/tlb.hh
index 15333ab..df5ce64 100644
--- a/src/arch/sparc/tlb.hh
+++ b/src/arch/sparc/tlb.hh
@@ -156,6 +156,8 @@
typedef SparcTLBParams Params;
TLB(const Params *p);
+Addr pageShift() const override { return PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override {}
void
diff --git a/src/arch/x86/tlb.hh b/src/arch/x86/tlb.hh
index 671b165..1547bbc 100644
--- a/src/arch/x86/tlb.hh
+++ b/src/arch/x86/tlb.hh
@@ -68,6 +68,8 @@
typedef X86TLBParams Params;
TLB(const Params *p);
+Addr pageShift() const override { return PageShift; }
+
void takeOverFrom(BaseTLB *otlb) override {}
TlbEntry *lookup(Addr va, bool update_lru = true);
diff --git a/src/sim/process.cc b/src/sim/process.cc
index 88277ee..e46ff7d 100644
--- a/src/sim/process.cc
+++ b/src/sim/process.cc
@@ -318,7 +318,7 @@
void
Process::allocateMem(Addr vaddr, int64_t size, bool clobber)
{
-int npages = divCeil(size, pTable->pageSize();
+int npages = divCeil(size, pTable->pageSize());
Addr paddr = system->allocPhysPages(npages);
pTable->map(vaddr, paddr, size,
clobber ?
[gem5-dev] Change in gem5/gem5[develop]: gpu: Use X86ISA instead of TheISA in compute_unit.cc.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34174 )
Change subject: gpu: Use X86ISA instead of TheISA in compute_unit.cc.
..
gpu: Use X86ISA instead of TheISA in compute_unit.cc.
This file is nominally not tied to the X86ISA, but in reality it is
because it reaches into the GPU TLB, which is defined unchangeably in the
X86ISA namespaces, and uses data structures within it. Rather than try to
pretend that these structures are generic, we'll instead just use X86ISA
instead of TheISA. If this really does become generic in the future, a
base class with the ISA agnostic essentials defined in it can be used
instead, and the ISA specific TLBs can defined their own derived class
which has whatever else they need. Really the compute unit shouldn't be
communicating with the TLB using sender state since those are supposed
to be little notes for the sender to keep with a transaction, not for
communicating between entities across a port.
Change-Id: Ie6573396f6c77a9a02194f5f4595eefa45d6d66b
---
M src/gpu-compute/compute_unit.cc
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/src/gpu-compute/compute_unit.cc
b/src/gpu-compute/compute_unit.cc
index 920257d..6e24895 100644
--- a/src/gpu-compute/compute_unit.cc
+++ b/src/gpu-compute/compute_unit.cc
@@ -1074,8 +1074,8 @@
pkt->senderState = new DTLBPort::SenderState(gpuDynInst, index);
// This is the senderState needed by the TLB hierarchy to function
-TheISA::GpuTLB::TranslationState *translation_state =
- new TheISA::GpuTLB::TranslationState(TLB_mode, shader->gpuTc,
false,
+X86ISA::GpuTLB::TranslationState *translation_state =
+ new X86ISA::GpuTLB::TranslationState(TLB_mode, shader->gpuTc,
false,
pkt->senderState);
pkt->senderState = translation_state;
@@ -1167,7 +1167,7 @@
delete pkt->senderState;
// Because it's atomic operation, only need TLB translation state
-pkt->senderState = new TheISA::GpuTLB::TranslationState(TLB_mode,
+pkt->senderState = new X86ISA::GpuTLB::TranslationState(TLB_mode,
shader->gpuTc);
tlbPort[tlbPort_index].sendFunctional(pkt);
@@ -1188,8 +1188,8 @@
new_pkt->req->getPaddr());
// safe_cast the senderState
-TheISA::GpuTLB::TranslationState *sender_state =
-
safe_cast(pkt->senderState);
+X86ISA::GpuTLB::TranslationState *sender_state =
+
safe_cast(pkt->senderState);
delete sender_state->tlbEntry;
delete new_pkt;
@@ -1209,7 +1209,7 @@
new ComputeUnit::ScalarDTLBPort::SenderState(gpuDynInst);
pkt->senderState =
-new TheISA::GpuTLB::TranslationState(tlb_mode, shader->gpuTc,
false,
+new X86ISA::GpuTLB::TranslationState(tlb_mode, shader->gpuTc,
false,
pkt->senderState);
if (scalarDTLBPort.isStalled()) {
@@ -1398,8 +1398,8 @@
computeUnit->tlbCycles += curTick();
// pop off the TLB translation state
-TheISA::GpuTLB::TranslationState *translation_state =
-
safe_cast(pkt->senderState);
+X86ISA::GpuTLB::TranslationState *translation_state =
+
safe_cast(pkt->senderState);
// no PageFaults are permitted for data accesses
if (!translation_state->tlbEntry) {
@@ -1471,8 +1471,8 @@
DPRINTF(GPUPrefetch, "CU[%d][%d][%d][%d]: %#x was last\n",
computeUnit->cu_id, simdId, wfSlotId, mp_index, last);
-int stride = last ? (roundDown(vaddr, TheISA::PageBytes) -
- roundDown(last, TheISA::PageBytes)) >>
TheISA::PageShift
+int stride = last ? (roundDown(vaddr, X86ISA::PageBytes) -
+ roundDown(last, X86ISA::PageBytes)) >>
X86ISA::PageShift
: 0;
DPRINTF(GPUPrefetch, "Stride is %d\n", stride);
@@ -1492,13 +1492,13 @@
// Prefetch Next few pages atomically
for (int pf = 1; pf <= computeUnit->prefetchDepth; ++pf) {
DPRINTF(GPUPrefetch, "%d * %d: %#x\n", pf, stride,
-vaddr+stride*pf*TheISA::PageBytes);
+vaddr + stride * pf * X86ISA::PageBytes);
if (!stride)
break;
RequestPtr prefetch_req = std::make_shared(
-vaddr + stride * pf * TheISA::PageBytes,
+vaddr + stride * pf * X86ISA::PageBytes,
sizeof(uint8_t), 0,
computeUnit->masterId(),
0, 0, nullptr);
@@ -1509,15 +1509,15 @@
// Because it's atomic operation, only need TLB translation
state
prefetch_pkt->senderState =
-new
[gem5-dev] Change in gem5/gem5[develop]: gpu: Stop using TheISA in the GPU TLB.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34173 )
Change subject: gpu: Stop using TheISA in the GPU TLB.
..
gpu: Stop using TheISA in the GPU TLB.
This class is defined inside the X86ISA namespace, so there's no point
in pretending it's generic. Remove TheISA and let the code access what
it needs from X86ISA naturally since it's there already.
Change-Id: I21b5d2d2b9af6aa0c10ddbb5b3ddca1692188dcc
---
M src/gpu-compute/gpu_tlb.cc
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index 513106f..6488b57 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -164,7 +164,7 @@
* vpn holds the virtual page address
* The least significant bits are simply masked
*/
-int set = (vpn >> TheISA::PageShift) & setMask;
+int set = (vpn >> PageShift) & setMask;
if (!freeList[set].empty()) {
newEntry = freeList[set].front();
@@ -184,7 +184,7 @@
GpuTLB::EntryList::iterator
GpuTLB::lookupIt(Addr va, bool update_lru)
{
-int set = (va >> TheISA::PageShift) & setMask;
+int set = (va >> PageShift) & setMask;
if (FA) {
assert(!set);
@@ -214,7 +214,7 @@
TlbEntry*
GpuTLB::lookup(Addr va, bool update_lru)
{
-int set = (va >> TheISA::PageShift) & setMask;
+int set = (va >> PageShift) & setMask;
auto entry = lookupIt(va, update_lru);
@@ -266,7 +266,7 @@
GpuTLB::demapPage(Addr va, uint64_t asn)
{
-int set = (va >> TheISA::PageShift) & setMask;
+int set = (va >> PageShift) & setMask;
auto entry = lookupIt(va, false);
if (entry != entryList[set].end()) {
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34173
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: I21b5d2d2b9af6aa0c10ddbb5b3ddca1692188dcc
Gerrit-Change-Number: 34173
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]: arm: Replicate the PageBytes constant in the ArmSystem class.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/34170 ) Change subject: arm: Replicate the PageBytes constant in the ArmSystem class. .. arm: Replicate the PageBytes constant in the ArmSystem class. When isa_traits.hh hopefully goes away in the not too distant future, this constant will need somewhere to live so ARM components can find it. There are valid arguments that this should not be a constant in the first place, but that's outside the scope of this change. Change-Id: Ic5bd046dc1cc196b3cf6b6c36878fdbf5eb4c0bf --- M src/arch/arm/system.hh 1 file changed, 3 insertions(+), 0 deletions(-) diff --git a/src/arch/arm/system.hh b/src/arch/arm/system.hh index cffc235..e76245e 100644 --- a/src/arch/arm/system.hh +++ b/src/arch/arm/system.hh @@ -137,6 +137,9 @@ ArmSemihosting *const semihosting; public: +static constexpr Addr PageBytes = ArmISA::PageBytes; +static constexpr Addr PageShift = ArmISA::PageShift; + typedef ArmSystemParams Params; const Params * params() const -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34170 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: Ic5bd046dc1cc196b3cf6b6c36878fdbf5eb4c0bf Gerrit-Change-Number: 34170 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] Re: Use of page size and TLBs in the prefetchers
Hi, I'm not using Ruby, so just talking about classic. Before the code included support for the TLBs, the prefetchers used the page size to detect page-crossing prefetches in order to discard them. Now it uses that to decide if it can do a direct prefetch or it needs to check the TLB for a translation of the address *if it has a TLB*. See `Base::samePage` (`src/mem/cache/prefetch/base.cc`) and an example of its use at https://gem5.googlesource.com/public/gem5/+/refs/heads/develop/src/mem/cache/prefetch/queued.cc#180 (`src/mem/cache/prefetch/queued.cc` line 180) Would there be any other option without having to include the TLB in all prefetchers? Cheers, Isaac El 7/9/20 a las 10:06, Gabe Black escribió: Hi folks. I've *almost* eliminated use of the getPageBytes and getPageShift functions in the System class, which in combination with a change from Andreas will eliminate the need for the isa_traits.hh switching header file. The only use left is in the Ruby and cache prefetchers: mem/cache/prefetch/base.cc: masterId(p->sys->getMasterId(this)), pageBytes(p->sys->getPageBytes()), mem/ruby/structures/RubyPrefetcher.cc: m_page_shift(p->sys->getPageShift()) I see that the cache prefetcher has a list of TLBs, and one thing I've done is add a method to the TLB class(es) that returns the current page size. Is it safe to assume one of these prefetchers will *necessarily* have a TLB assigned to it? That seems plausible since if it doesn't, why does it care about page sizes? But I don't understand that code well enough to determine that, especially if it's some corner case. The Ruby prefetcher doesn't *seem* to have a TLB attached to it? Although I don't really know how Ruby works. What's going on there? Any idea why it needs to know the page size (I can guess, but don't know for sure), and what plays the role of the TLB in that case? Gabe -- Isaac Sánchez Barrera http://bsc.es/disclaimer ___ 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]: base: Cleanup debug flags API
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34118 )
Change subject: base: Cleanup debug flags API
..
base: Cleanup debug flags API
The debug flags API has a couple of quirks that should be cleaned
up. Specifically:
* Only CompoundFlag should expose a list of children.
* The global enable flag is just called "active", this isn't very
descriptive.
* Only SimpleFlag exposed a status member. This should be in the base
class to make the API symmetric.
* Flag::Sync() is an implementation detail and needs to be protected.
Change-Id: I4d7fd32c80891191aa04f0bd0c334c8cf8d372f5
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34118
Reviewed-by: Jason Lowe-Power
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/base/debug.cc
M src/base/debug.hh
M src/base/trace.cc
M src/python/m5/debug.py
M src/python/pybind11/debug.cc
5 files changed, 79 insertions(+), 44 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/debug.cc b/src/base/debug.cc
index 47febd0..45d9f9d 100644
--- a/src/base/debug.cc
+++ b/src/base/debug.cc
@@ -1,4 +1,16 @@
/*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder. You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
* Copyright (c) 2003-2005 The Regents of The University of Michigan
* All rights reserved.
*
@@ -67,7 +79,7 @@
return flags;
}
-bool SimpleFlag::_active = false;
+bool Flag::_globalEnable = false;
Flag *
findFlag(const std::string )
@@ -96,17 +108,17 @@
}
void
-SimpleFlag::enableAll()
+Flag::globalEnable()
{
-_active = true;
+_globalEnable = true;
for (auto& i : allFlags())
i.second->sync();
}
void
-SimpleFlag::disableAll()
+Flag::globalDisable()
{
-_active = false;
+_globalEnable = false;
for (auto& i : allFlags())
i.second->sync();
}
@@ -125,6 +137,19 @@
k->disable();
}
+bool
+CompoundFlag::status() const
+{
+if (_kids.empty())
+return false;
+
+for (auto& k : _kids) {
+if (!k->status())
+return false;
+}
+
+return true;
+}
bool
changeFlag(const char *s, bool value)
diff --git a/src/base/debug.hh b/src/base/debug.hh
index 1d35be0..a5dc43c 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -1,4 +1,16 @@
/*
+ * Copyright (c) 2020 ARM Limited
+ * All rights reserved
+ *
+ * The license below extends only to copyright in the software and shall
+ * not be construed as granting a license to any other intellectual
+ * property including but not limited to intellectual property relating
+ * to a hardware implementation of the functionality of the software
+ * licensed hereunder. You may use the software subject to the license
+ * terms below provided that you ensure that this notice is replicated
+ * unmodified and in its entirety in all distributions of the software,
+ * modified or unmodified, in source code or in binary form.
+ *
* Copyright (c) 2003-2005 The Regents of The University of Michigan
* Copyright (c) 2010 The Hewlett-Packard Development Company
* All rights reserved.
@@ -42,45 +54,48 @@
class Flag
{
protected:
+static bool _globalEnable; // whether debug tracings are enabled
+
const char *_name;
const char *_desc;
+virtual void sync() { }
+
public:
Flag(const char *name, const char *desc);
virtual ~Flag();
std::string name() const { return _name; }
std::string desc() const { return _desc; }
-virtual std::vector kids() { return std::vector(); }
virtual void enable() = 0;
virtual void disable() = 0;
-virtual void sync() {}
+virtual bool status() const = 0;
+
+operator bool() const { return status(); }
+bool operator!() const { return !status(); }
+
+static void globalEnable();
+static void globalDisable();
};
class SimpleFlag : public Flag
{
-static bool _active; // whether debug tracings are enabled
protected:
bool _tracing; // tracing is enabled and flag is on
bool _status; // flag status
+void sync() override { _tracing = _globalEnable && _status; }
+
public:
SimpleFlag(const char *name, const char *desc)
: Flag(name, desc), _status(false)
{ }
-bool status() const { return _tracing; }
[gem5-dev] Change in gem5/gem5[develop]: scons: Simplify arch enum generation
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34116 )
Change subject: scons: Simplify arch enum generation
..
scons: Simplify arch enum generation
C++ allows a trailing comma after the last item in an enum, so there
is no need for a special case.
Change-Id: I6ead36b4a8562b4a7a5aec88e4f6390182eedf56
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34116
Reviewed-by: Jason Lowe-Power
Reviewed-by: Gabe Black
Maintainer: Jason Lowe-Power
Maintainer: Gabe Black
Tested-by: kokoro
---
M src/SConscript
1 file changed, 4 insertions(+), 10 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Gabe Black: Looks good to me, approved; Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/SConscript b/src/SConscript
index 4b6db44..9f82bdc 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -645,11 +645,8 @@
# create an enum for any run-time determination of the ISA, we
# reuse the same name as the namespaces
code('enum class Arch {')
-for i,isa in enumerate(isas):
-if i + 1 == len(isas):
-code(' $0 = $1', namespace(isa), define(isa))
-else:
-code(' $0 = $1,', namespace(isa), define(isa))
+for isa in isas:
+code(' $0 = $1,', namespace(isa), define(isa))
code('};')
code('''
@@ -690,11 +687,8 @@
# create an enum for any run-time determination of the ISA, we
# reuse the same name as the namespaces
code('enum class GPUArch {')
-for i,isa in enumerate(isas):
-if i + 1 == len(isas):
-code(' $0 = $1', namespace(isa), define(isa))
-else:
-code(' $0 = $1,', namespace(isa), define(isa))
+for isa in isas:
+code(' $0 = $1,', namespace(isa), define(isa))
code('};')
code('''
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34116
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: I6ead36b4a8562b4a7a5aec88e4f6390182eedf56
Gerrit-Change-Number: 34116
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
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]: base: Remove unused Debug::All flag
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34117 )
Change subject: base: Remove unused Debug::All flag
..
base: Remove unused Debug::All flag
The Debug::All flag doesn't seem to be used. Remove it.
Change-Id: I3d6ad1b2f61a2a0a5c52cbc6d520112855946007
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34117
Reviewed-by: Jason Lowe-Power
Reviewed-by: Gabe Black
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/base/debug.cc
M src/base/debug.hh
2 files changed, 0 insertions(+), 31 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/debug.cc b/src/base/debug.cc
index b165f64..47febd0 100644
--- a/src/base/debug.cc
+++ b/src/base/debug.cc
@@ -125,35 +125,6 @@
k->disable();
}
-struct AllFlags : public Flag
-{
-AllFlags()
-: Flag("All", "All Flags")
-{}
-
-void
-enable()
-{
-FlagsMap::iterator i = allFlags().begin();
-FlagsMap::iterator end = allFlags().end();
-for (; i != end; ++i)
-if (i->second != this)
-i->second->enable();
-}
-
-void
-disable()
-{
-FlagsMap::iterator i = allFlags().begin();
-FlagsMap::iterator end = allFlags().end();
-for (; i != end; ++i)
-if (i->second != this)
-i->second->disable();
-}
-};
-
-AllFlags theAllFlags;
-Flag *const All =
bool
changeFlag(const char *s, bool value)
diff --git a/src/base/debug.hh b/src/base/debug.hh
index 479a830..1d35be0 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -108,8 +108,6 @@
Flag *findFlag(const std::string );
-extern Flag *const All;
-
bool changeFlag(const char *s, bool value);
} // namespace Debug
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34117
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: I3d6ad1b2f61a2a0a5c52cbc6d520112855946007
Gerrit-Change-Number: 34117
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Bobby R. Bruce
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-CC: Daniel Carvalho
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]: python: Remove unused debug APIs
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34120 )
Change subject: python: Remove unused debug APIs
..
python: Remove unused debug APIs
The following APIs are not exported from the _m5 namespace and not
used by any of the debug glue code:
* m5.debug.findFlag
* m5.debug.setDebugFlag
* m5.debug.clearDebugFlag
* m5.debug.dumpDebugFlags
All of them have a clean Python interface where flags are exported
using the m5.debug.flags dictionary. There is also an m5.debug.help
function that lists the available debug flags.
Remove the unused APIs to avoid confusion.
Change-Id: I74738451eb5874f83b135adaccd30a0c6b478996
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34120
Reviewed-by: Jason Lowe-Power
Reviewed-by: Gabe Black
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/python/pybind11/debug.cc
1 file changed, 0 insertions(+), 4 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/python/pybind11/debug.cc b/src/python/pybind11/debug.cc
index 69c497c..84673f1 100644
--- a/src/python/pybind11/debug.cc
+++ b/src/python/pybind11/debug.cc
@@ -83,10 +83,6 @@
m_debug
.def("getAllFlagsVersion", []() { return Debug::allFlagsVersion; })
.def("allFlags", ::allFlags,
py::return_value_policy::reference)
-.def("findFlag", ::findFlag)
-.def("setDebugFlag", )
-.def("clearDebugFlag", )
-.def("dumpDebugFlags", )
.def("schedBreak", )
.def("setRemoteGDBPort", )
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34120
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: I74738451eb5874f83b135adaccd30a0c6b478996
Gerrit-Change-Number: 34120
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Jason Lowe-Power
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]: base: Cleanup Debug::CompoundFlag
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34115 )
Change subject: base: Cleanup Debug::CompoundFlag
..
base: Cleanup Debug::CompoundFlag
Compound flags are currently constructed using a constructor with a
finite set of arguments that default to nullptr that refer to child
flags. C++11 introduces two cleaner ways to achieve the same thing,
variadic templates and initializer_list. Use an initializer list to
pass dependent flags.
Change-Id: Iadcd04986ab20efccfae9b92b26c079b9612262e
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34115
Reviewed-by: Jason Lowe-Power
Reviewed-by: Gabe Black
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/SConscript
M src/base/debug.hh
2 files changed, 9 insertions(+), 29 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/SConscript b/src/SConscript
index d9cde28..4b6db44 100644
--- a/src/SConscript
+++ b/src/SConscript
@@ -1071,15 +1071,12 @@
if not compound:
code('SimpleFlag $name("$name", "$desc");')
else:
-comp_code('CompoundFlag $name("$name", "$desc",')
+comp_code('CompoundFlag $name("$name", "$desc", {')
comp_code.indent()
-last = len(compound) - 1
-for i,flag in enumerate(compound):
-if i != last:
-comp_code('&$flag,')
-else:
-comp_code('&$flag);')
+for flag in compound:
+comp_code('&$flag,')
comp_code.dedent()
+comp_code('});')
code.append(comp_code)
code()
diff --git a/src/base/debug.hh b/src/base/debug.hh
index 7c9834c..479a830 100644
--- a/src/base/debug.hh
+++ b/src/base/debug.hh
@@ -30,6 +30,7 @@
#ifndef __BASE_DEBUG_HH__
#define __BASE_DEBUG_HH__
+#include
#include
#include
#include
@@ -87,31 +88,13 @@
protected:
std::vector _kids;
-void
-addFlag(Flag *f)
-{
-if (f != nullptr)
-_kids.push_back(f);
-}
-
public:
+template
CompoundFlag(const char *name, const char *desc,
-Flag *f00 = nullptr, Flag *f01 = nullptr,
-Flag *f02 = nullptr, Flag *f03 = nullptr,
-Flag *f04 = nullptr, Flag *f05 = nullptr,
-Flag *f06 = nullptr, Flag *f07 = nullptr,
-Flag *f08 = nullptr, Flag *f09 = nullptr,
-Flag *f10 = nullptr, Flag *f11 = nullptr,
-Flag *f12 = nullptr, Flag *f13 = nullptr,
-Flag *f14 = nullptr, Flag *f15 = nullptr,
-Flag *f16 = nullptr, Flag *f17 = nullptr,
-Flag *f18 = nullptr, Flag *f19 = nullptr)
-: Flag(name, desc)
+ std::initializer_list flags)
+: Flag(name, desc),
+ _kids(flags)
{
-addFlag(f00); addFlag(f01); addFlag(f02); addFlag(f03);
addFlag(f04);
-addFlag(f05); addFlag(f06); addFlag(f07); addFlag(f08);
addFlag(f09);
-addFlag(f10); addFlag(f11); addFlag(f12); addFlag(f13);
addFlag(f14);
-addFlag(f15); addFlag(f16); addFlag(f17); addFlag(f18);
addFlag(f19);
}
std::vector kids() { return _kids; }
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34115
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: Iadcd04986ab20efccfae9b92b26c079b9612262e
Gerrit-Change-Number: 34115
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg
Gerrit-Assignee: Bobby R. Bruce
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: kokoro
Gerrit-CC: Bobby R. Bruce
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]: python: Add the ability to check if a debug flag has been enabled
Andreas Sandberg has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/34119 )
Change subject: python: Add the ability to check if a debug flag has been
enabled
..
python: Add the ability to check if a debug flag has been enabled
There is currently no Python API to check if a debug flag is
enabled. Add a new status property that can be read or set to control
the status of a flag. The stat of a flag can also be queried by
converting it to a bool.
For example:
m5.debug.flags["XBar"].status = True
if m5.debug.flags["XBar"]:
print("XBar debugging is on")
Change-Id: I5a50c39ced182ab44e18c061c463d7d9c41ef186
Signed-off-by: Andreas Sandberg
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/34119
Reviewed-by: Jason Lowe-Power
Reviewed-by: Gabe Black
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/python/pybind11/debug.cc
1 file changed, 14 insertions(+), 0 deletions(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved; Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/python/pybind11/debug.cc b/src/python/pybind11/debug.cc
index ed2942b..69c497c 100644
--- a/src/python/pybind11/debug.cc
+++ b/src/python/pybind11/debug.cc
@@ -98,6 +98,20 @@
.def_property_readonly("desc", ::Flag::desc)
.def("enable", ::Flag::enable)
.def("disable", ::Flag::disable)
+.def_property("status",
+ [](const Debug::Flag *flag) {
+ return flag->status();
+ },
+ [](Debug::Flag *flag, bool state) {
+ if (state) {
+ flag->enable();
+ } else {
+ flag->disable();
+ }
+ })
+.def("__bool__", [](const Debug::Flag *flag) {
+return flag->status();
+})
;
py::class_(m_debug, "SimpleFlag", c_flag);
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34119
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: I5a50c39ced182ab44e18c061c463d7d9c41ef186
Gerrit-Change-Number: 34119
Gerrit-PatchSet: 2
Gerrit-Owner: Andreas Sandberg
Gerrit-Reviewer: Andreas Sandberg
Gerrit-Reviewer: Gabe Black
Gerrit-Reviewer: Jason Lowe-Power
Gerrit-Reviewer: Jason Lowe-Power
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] Re: Use of page size and TLBs in the prefetchers
Actually that's not *quite* the end for isa_traits.hh since the system class uses the PageBytes and PageShift internally to allocate physical memory. It's still very close though. Gabe On Mon, Sep 7, 2020 at 1:06 AM Gabe Black wrote: > Hi folks. I've *almost* eliminated use of the getPageBytes and > getPageShift functions in the System class, which in combination with a > change from Andreas will eliminate the need for the isa_traits.hh switching > header file. > > The only use left is in the Ruby and cache prefetchers: > > mem/cache/prefetch/base.cc: masterId(p->sys->getMasterId(this)), > pageBytes(p->sys->getPageBytes()), > mem/ruby/structures/RubyPrefetcher.cc: > m_page_shift(p->sys->getPageShift()) > > I see that the cache prefetcher has a list of TLBs, and one thing I've > done is add a method to the TLB class(es) that returns the current page > size. Is it safe to assume one of these prefetchers will *necessarily* have > a TLB assigned to it? That seems plausible since if it doesn't, why does it > care about page sizes? But I don't understand that code well enough to > determine that, especially if it's some corner case. > > The Ruby prefetcher doesn't *seem* to have a TLB attached to it? Although > I don't really know how Ruby works. What's going on there? Any idea why it > needs to know the page size (I can guess, but don't know for sure), and > what plays the role of the TLB in that case? > > 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]: null,sim: Add name() to the dummy CPU and remove two #if THE_ISAs.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34168 )
Change subject: null,sim: Add name() to the dummy CPU and remove two #if
THE_ISAs.
..
null,sim: Add name() to the dummy CPU and remove two #if THE_ISAs.
This #if THE_ISAs were there because the dummy CPU didn't have a name()
method, and so couldn't provide its name for DPRINTFs. Provide that
method so we can get rid of the #ifs.
Change-Id: Ie7045554ddc8c9fb7725dcbcb7d1d2557786d00c
---
M src/arch/null/cpu_dummy.hh
M src/sim/system.cc
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/arch/null/cpu_dummy.hh b/src/arch/null/cpu_dummy.hh
index 7e183eb..5418511 100644
--- a/src/arch/null/cpu_dummy.hh
+++ b/src/arch/null/cpu_dummy.hh
@@ -46,6 +46,7 @@
static int numSimulatedInsts() { return 0; }
static int numSimulatedOps() { return 0; }
static void wakeup(ThreadID tid) { ; }
+static const std::string name() { return "dummy_cpu"; }
};
#endif // __ARCH_NULL_CPU_DUMMY_HH__
diff --git a/src/sim/system.cc b/src/sim/system.cc
index 8185f13..a9f768c 100644
--- a/src/sim/system.cc
+++ b/src/sim/system.cc
@@ -76,10 +76,8 @@
void
System::Threads::Thread::resume()
{
-# if THE_ISA != NULL_ISA
DPRINTFS(Quiesce, context->getCpuPtr(), "activating\n");
context->activate();
-# endif
}
std::string
@@ -179,10 +177,8 @@
System::Threads::quiesce(ContextID id)
{
auto = thread(id);
-# if THE_ISA != NULL_ISA
BaseCPU M5_VAR_USED *cpu = t.context->getCpuPtr();
DPRINTFS(Quiesce, cpu, "quiesce()\n");
-# endif
t.quiesce();
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34168
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: Ie7045554ddc8c9fb7725dcbcb7d1d2557786d00c
Gerrit-Change-Number: 34168
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]: sparc,sim: Remove special handling of SPARC in the clone system call.
Gabe Black has uploaded this change for review. ( https://gem5-review.googlesource.com/c/public/gem5/+/34167 ) Change subject: sparc,sim: Remove special handling of SPARC in the clone system call. .. sparc,sim: Remove special handling of SPARC in the clone system call. We can set the extra syscall return values in the ISA specific archClone function. We don't need a special #ifdef to handle them. Change-Id: I82904b3d4bdf211c89d271d7277a60151191cdfc --- M src/arch/sparc/linux/linux.hh M src/sim/syscall_emul.hh 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/arch/sparc/linux/linux.hh b/src/arch/sparc/linux/linux.hh index 89cd207..3b5aea8 100644 --- a/src/arch/sparc/linux/linux.hh +++ b/src/arch/sparc/linux/linux.hh @@ -230,6 +230,11 @@ if (stack) ctc->setIntReg(SparcISA::StackPointerReg, stack); + +// Set these extra values. Since "clone" doesn't return two values, +// we can set these and they won't be clobbered by the syscall ABI. +ptc->setIntReg(SparcISA::SyscallPseudoReturnReg, 0); +ctc->setIntReg(SparcISA::SyscallPseudoReturnReg, 1); } }; diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh index 05a29f9..9d1f6e2 100644 --- a/src/sim/syscall_emul.hh +++ b/src/sim/syscall_emul.hh @@ -1506,11 +1506,6 @@ desc->returnInto(ctc, 0); -#if THE_ISA == SPARC_ISA -tc->setIntReg(TheISA::SyscallPseudoReturnReg, 0); -ctc->setIntReg(TheISA::SyscallPseudoReturnReg, 1); -#endif - TheISA::PCState cpc = tc->pcState(); if (!p->kvmInSE) cpc.advance(); -- To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34167 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: I82904b3d4bdf211c89d271d7277a60151191cdfc Gerrit-Change-Number: 34167 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: Move many of the generic files outside an NULL guard.
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/c/public/gem5/+/34169 )
Change subject: arch: Move many of the generic files outside an NULL guard.
..
arch: Move many of the generic files outside an NULL guard.
These files can be compiled successfully even if the ISA is the NULL
ISA.
Change-Id: I67133ea674f678f33b0aa1ef55af719f2869241d
---
M src/arch/generic/SConscript
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/arch/generic/SConscript b/src/arch/generic/SConscript
index 6651b7b..78269b7 100644
--- a/src/arch/generic/SConscript
+++ b/src/arch/generic/SConscript
@@ -40,15 +40,15 @@
Source('htm.cc')
-if env['TARGET_ISA'] == 'null':
-Return()
-
-Source('decode_cache.cc')
-Source('decoder.cc')
-
SimObject('BaseInterrupts.py')
SimObject('BaseISA.py')
SimObject('BaseTLB.py')
SimObject('ISACommon.py')
DebugFlag('TLB')
+
+if env['TARGET_ISA'] == 'null':
+Return()
+
+Source('decode_cache.cc')
+Source('decoder.cc')
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/34169
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: I67133ea674f678f33b0aa1ef55af719f2869241d
Gerrit-Change-Number: 34169
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] ChunkGenerator granularity, interface
Hi folks. In gem5, there is a simple but useful utility class called the
ChunkGenerator which takes a region of memory and a size, and breaks the
region into chunks which are broken on that size aligned boundaries.
So for instance, if you wanted to translate every page that some big array
was located in in memory, you would use a ChunkGenerator with the page size
as the chunk size.
The problem here is that this class assumes that there is "the" page size,
or "the" block size to put into it, known ahead of time by the consumer.
This may not be true for page sizes for example, since a region may fall
across pages with different sizes. Or it may be a waste of effort if, for
instance, a region was contiguously mapped. There may also be reasons to
chunk up a region which are also not based on pages or other fixed size
boundaries.
I think it would be a fairly simple extension to make the ChunkGenerator
where it would be created by some other entity which knows how big any
given chunk can be. For instance, there might be a call like this:
chunk_generator = tlb->chunkRegion(region_start, region_end);
Then you could walk through the chunks basically as you do now, but the TLB
would be involved and would know what boundaries to stop you at. I think
the biggest change that would require would be to make the "next()"
function virtual. It might be a good idea to break the class up into a base
class with generic bits, and then make the current, fixed chunk size
version inherit from it. Fancy variable sized versions could inherit from
the base without breaking existing usage.
Also, the chunk generator has a very iterator like design currently. I
think it would be fairly straightforward to go all the way and give it real
iterators so that it can be used in a range based for loop, something like
this:
for (const auto : tlb->chunkRegion(region_start, region_end) {
Addr paddr = translate(chunk.addr());
read(buffer, paddr, chunk.size());
}
I think with these two changes, this class could be both more correct, and
a little easier to use. What do you think?
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] Use of page size and TLBs in the prefetchers
Hi folks. I've *almost* eliminated use of the getPageBytes and getPageShift functions in the System class, which in combination with a change from Andreas will eliminate the need for the isa_traits.hh switching header file. The only use left is in the Ruby and cache prefetchers: mem/cache/prefetch/base.cc: masterId(p->sys->getMasterId(this)), pageBytes(p->sys->getPageBytes()), mem/ruby/structures/RubyPrefetcher.cc: m_page_shift(p->sys->getPageShift()) I see that the cache prefetcher has a list of TLBs, and one thing I've done is add a method to the TLB class(es) that returns the current page size. Is it safe to assume one of these prefetchers will *necessarily* have a TLB assigned to it? That seems plausible since if it doesn't, why does it care about page sizes? But I don't understand that code well enough to determine that, especially if it's some corner case. The Ruby prefetcher doesn't *seem* to have a TLB attached to it? Although I don't really know how Ruby works. What's going on there? Any idea why it needs to know the page size (I can guess, but don't know for sure), and what plays the role of the TLB in that case? 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
