Discussion:
[gem5-dev] Change in public/gem5[master]: arch,
(too old to reply)
Gabe Black (Gerrit)
2017-12-23 03:32:04 UTC
Permalink
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/6981


Change subject: arch,mem: Move page table construction into the arch
classes.
......................................................................

arch,mem: Move page table construction into the arch classes.

This gets rid of an awkward NoArchPageTable class, and also gives the
arch a place to inject ISA specific parameters (specifically page size)
without having to have TheISA:: in the generic version of these types.

Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
---
M src/arch/alpha/process.cc
M src/arch/alpha/process.hh
M src/arch/arm/process.cc
M src/arch/arm/process.hh
M src/arch/mips/process.cc
M src/arch/mips/process.hh
M src/arch/power/process.cc
M src/arch/power/process.hh
M src/arch/riscv/process.cc
M src/arch/riscv/process.hh
M src/arch/sparc/process.cc
M src/arch/sparc/process.hh
M src/arch/x86/process.cc
M src/arch/x86/process.hh
M src/mem/page_table.hh
M src/sim/process.cc
M src/sim/process.hh
17 files changed, 45 insertions(+), 61 deletions(-)



diff --git a/src/arch/alpha/process.cc b/src/arch/alpha/process.cc
index 58fe0bd..3107d27 100644
--- a/src/arch/alpha/process.cc
+++ b/src/arch/alpha/process.cc
@@ -38,6 +38,7 @@
#include "cpu/thread_context.hh"
#include "debug/Loader.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/byteswap.hh"
#include "sim/process_impl.hh"
@@ -48,8 +49,9 @@
using namespace std;

AlphaProcess::AlphaProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ assert(!params->useArchPT);
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
objFile->bssSize();
brk_point = roundUp(brk_point, PageBytes);
diff --git a/src/arch/alpha/process.hh b/src/arch/alpha/process.hh
index a02b8ce..28ecd68 100644
--- a/src/arch/alpha/process.hh
+++ b/src/arch/alpha/process.hh
@@ -61,7 +61,4 @@
virtual bool mmapGrowsDown() const override { return false; }
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __ARCH_ALPHA_PROCESS_HH__
diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index dcc9145..d4b59aa 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -51,6 +51,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/byteswap.hh"
#include "sim/process_impl.hh"
@@ -62,8 +63,10 @@

ArmProcess::ArmProcess(ProcessParams *params, ObjectFile *objFile,
ObjectFile::Arch _arch)
- : Process(params, objFile), arch(_arch)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ arch(_arch)
{
+ assert(!params->useArchPT);
}

ArmProcess32::ArmProcess32(ProcessParams *params, ObjectFile *objFile,
diff --git a/src/arch/arm/process.hh b/src/arch/arm/process.hh
index f8e6542..f4f0874 100644
--- a/src/arch/arm/process.hh
+++ b/src/arch/arm/process.hh
@@ -95,8 +95,5 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __ARM_PROCESS_HH__

diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index 4d0d5e3..943d4a9 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -39,6 +39,7 @@
#include "cpu/thread_context.hh"
#include "debug/Loader.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process.hh"
#include "sim/process_impl.hh"
@@ -48,9 +49,10 @@
using namespace std;
using namespace MipsISA;

-MipsProcess::MipsProcess(ProcessParams * params, ObjectFile *objFile)
- : Process(params, objFile)
+MipsProcess::MipsProcess(ProcessParams *params, ObjectFile *objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ assert(!params->useArchPT);
// Set up stack. On MIPS, stack starts at the top of kuseg
// user address space. MIPS stack grows down from here
Addr stack_base = 0x7FFFFFFF;
diff --git a/src/arch/mips/process.hh b/src/arch/mips/process.hh
index ed6561c..e9e0585 100644
--- a/src/arch/mips/process.hh
+++ b/src/arch/mips/process.hh
@@ -58,8 +58,4 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
-
#endif // __MIPS_PROCESS_HH__
diff --git a/src/arch/power/process.cc b/src/arch/power/process.cc
index 4a34dec..f933302 100644
--- a/src/arch/power/process.cc
+++ b/src/arch/power/process.cc
@@ -40,6 +40,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_return.hh"
@@ -49,8 +50,9 @@
using namespace PowerISA;

PowerProcess::PowerProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ assert(!params->useArchPT);
// Set up break point (Top of Heap)
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
objFile->bssSize();
diff --git a/src/arch/power/process.hh b/src/arch/power/process.hh
index 08efdfe..348e375 100644
--- a/src/arch/power/process.hh
+++ b/src/arch/power/process.hh
@@ -57,8 +57,5 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __POWER_PROCESS_HH__

diff --git a/src/arch/riscv/process.cc b/src/arch/riscv/process.cc
index 6fe935c..bdabc9e 100644
--- a/src/arch/riscv/process.cc
+++ b/src/arch/riscv/process.cc
@@ -59,9 +59,10 @@
using namespace std;
using namespace RiscvISA;

-RiscvProcess::RiscvProcess(ProcessParams * params,
- ObjectFile *objFile) : Process(params, objFile)
+RiscvProcess::RiscvProcess(ProcessParams *params, ObjectFile *objFile) :
+ Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ assert(!params->useArchPT);
const Addr stack_base = 0x7FFFFFFFFFFFFFFFL;
const Addr max_stack_size = 8 * 1024 * 1024;
const Addr next_thread_stack_base = stack_base - max_stack_size;
diff --git a/src/arch/riscv/process.hh b/src/arch/riscv/process.hh
index 2a27f35..bda278e 100644
--- a/src/arch/riscv/process.hh
+++ b/src/arch/riscv/process.hh
@@ -65,8 +65,4 @@
virtual bool mmapGrowsDown() const override { return false; }
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
-
#endif // __RISCV_PROCESS_HH__
diff --git a/src/arch/sparc/process.cc b/src/arch/sparc/process.cc
index da7032f..f92b646 100644
--- a/src/arch/sparc/process.cc
+++ b/src/arch/sparc/process.cc
@@ -42,6 +42,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_return.hh"
@@ -53,10 +54,12 @@
static const int FirstArgumentReg = 8;


-SparcProcess::SparcProcess(ProcessParams * params, ObjectFile *objFile,
+SparcProcess::SparcProcess(ProcessParams *params, ObjectFile *objFile,
Addr _StackBias)
- : Process(params, objFile), StackBias(_StackBias)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ StackBias(_StackBias)
{
+ assert(!params->useArchPT);
// Initialize these to 0s
fillStart = 0;
spillStart = 0;
diff --git a/src/arch/sparc/process.hh b/src/arch/sparc/process.hh
index 6a203a4..d7e0967 100644
--- a/src/arch/sparc/process.hh
+++ b/src/arch/sparc/process.hh
@@ -160,7 +160,4 @@
void setSyscallArg(ThreadContext *tc, int i, SparcISA::IntReg val);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __SPARC_PROCESS_HH__
diff --git a/src/arch/x86/process.cc b/src/arch/x86/process.cc
index 1ad1315..f11cc34 100644
--- a/src/arch/x86/process.cc
+++ b/src/arch/x86/process.cc
@@ -60,6 +60,7 @@
#include "debug/Stack.hh"
#include "mem/multi_level_page_table.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_desc.hh"
@@ -95,10 +96,16 @@
static const int NumArgumentRegs32 M5_VAR_USED =
sizeof(ArgumentReg) / sizeof(const int);

-X86Process::X86Process(ProcessParams * params, ObjectFile *objFile,
+X86Process::X86Process(ProcessParams *params, ObjectFile *objFile,
SyscallDesc *_syscallDescs, int _numSyscallDescs)
- : Process(params, objFile), syscallDescs(_syscallDescs),
- numSyscallDescs(_numSyscallDescs)
+ : Process(params, params->useArchPT ?
+ static_cast<PageTableBase *>(
+ new ArchPageTable(params->name, params->pid,
+ params->system)) :
+ static_cast<PageTableBase *>(
+ new FuncPageTable(params->name,
params->pid)),
+ objFile),
+ syscallDescs(_syscallDescs), numSyscallDescs(_numSyscallDescs)
{
}

diff --git a/src/arch/x86/process.hh b/src/arch/x86/process.hh
index 3eb9620..e5e1857 100644
--- a/src/arch/x86/process.hh
+++ b/src/arch/x86/process.hh
@@ -59,6 +59,14 @@
class X86Process : public Process
{
protected:
+ /**
+ * Declaration of architectural page table for x86.
+ *
+ * These page tables are stored in system memory and respect x86
+ * specification.
+ */
+ typedef MultiLevelPageTable<PageTableOps> ArchPageTable;
+
Addr _gdtStart;
Addr _gdtSize;

@@ -189,14 +197,6 @@
Process *process, TheISA::IntReg flags) override;
};

- /**
- * Declaration of architectural page table for x86.
- *
- * These page tables are stored in system memory and respect x86
- * specification.
- */
- typedef MultiLevelPageTable<PageTableOps> ArchPageTable;
-
}

#endif // __ARCH_X86_PROCESS_HH__
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 0d0a75e..883b47c 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -246,19 +246,4 @@
void getMappings(std::vector<std::pair<Addr, Addr>> *addr_maps)
override;
};

-/**
- * Faux page table class indended to stop the usage of
- * an architectural page table, when there is none defined
- * for a particular ISA.
- */
-class NoArchPageTable : public FuncPageTable
-{
- public:
- NoArchPageTable(const std::string &__name, uint64_t _pid, System *_sys,
- Addr _pageSize = TheISA::PageBytes) : FuncPageTable(__name,
_pid)
- {
- fatal("No architectural page table defined for this ISA.\n");
- }
-};
-
#endif // __MEM_PAGE_TABLE_HH__
diff --git a/src/sim/process.cc b/src/sim/process.cc
index ee90667..77d7903 100644
--- a/src/sim/process.cc
+++ b/src/sim/process.cc
@@ -101,14 +101,12 @@
using namespace std;
using namespace TheISA;

-Process::Process(ProcessParams * params, ObjectFile * obj_file)
+Process::Process(ProcessParams *params, PageTableBase *pTable,
+ ObjectFile *obj_file)
: SimObject(params), system(params->system),
useArchPT(params->useArchPT),
kvmInSE(params->kvmInSE),
- pTable(useArchPT ?
- static_cast<PageTableBase *>(new ArchPageTable(name(), params->pid,
- system)) :
- static_cast<PageTableBase *>(new FuncPageTable(name(),
params->pid))),
+ pTable(pTable),
initVirtMem(system->getSystemPort(), this,
SETranslatingPortProxy::Always),
objFile(obj_file),
diff --git a/src/sim/process.hh b/src/sim/process.hh
index e4a52e3..6d465ac 100644
--- a/src/sim/process.hh
+++ b/src/sim/process.hh
@@ -63,7 +63,8 @@
class Process : public SimObject
{
public:
- Process(ProcessParams *params, ObjectFile *obj_file);
+ Process(ProcessParams *params, PageTableBase *pTable,
+ ObjectFile *obj_file);

void serialize(CheckpointOut &cp) const override;
void unserialize(CheckpointIn &cp) override;
--
To view, visit https://gem5-review.googlesource.com/6981
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
Gerrit-Change-Number: 6981
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <***@google.com>
Gabe Black (Gerrit)
2017-12-23 03:32:04 UTC
Permalink
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/6982


Change subject: arch,mem: Remove the default value for page size.
......................................................................

arch,mem: Remove the default value for page size.

This breaks one more architecture dependence outside of the ISAs.

Change-Id: I071f9ed73aef78e1cd1752247c183e30854b2d28
---
M src/arch/alpha/process.cc
M src/arch/arm/process.cc
M src/arch/mips/process.cc
M src/arch/power/process.cc
M src/arch/riscv/process.cc
M src/arch/sparc/process.cc
M src/arch/x86/process.cc
M src/mem/multi_level_page_table.hh
M src/mem/multi_level_page_table_impl.hh
M src/mem/page_table.hh
10 files changed, 23 insertions(+), 16 deletions(-)



diff --git a/src/arch/alpha/process.cc b/src/arch/alpha/process.cc
index 3107d27..13a5130 100644
--- a/src/arch/alpha/process.cc
+++ b/src/arch/alpha/process.cc
@@ -49,7 +49,8 @@
using namespace std;

AlphaProcess::AlphaProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
assert(!params->useArchPT);
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index d4b59aa..db49a32 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -63,7 +63,8 @@

ArmProcess::ArmProcess(ProcessParams *params, ObjectFile *objFile,
ObjectFile::Arch _arch)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile),
arch(_arch)
{
assert(!params->useArchPT);
diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index 943d4a9..5d61675 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -50,7 +50,8 @@
using namespace MipsISA;

MipsProcess::MipsProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
assert(!params->useArchPT);
// Set up stack. On MIPS, stack starts at the top of kuseg
diff --git a/src/arch/power/process.cc b/src/arch/power/process.cc
index f933302..7eddb23 100644
--- a/src/arch/power/process.cc
+++ b/src/arch/power/process.cc
@@ -50,7 +50,8 @@
using namespace PowerISA;

PowerProcess::PowerProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
assert(!params->useArchPT);
// Set up break point (Top of Heap)
diff --git a/src/arch/riscv/process.cc b/src/arch/riscv/process.cc
index bdabc9e..776fd1f 100644
--- a/src/arch/riscv/process.cc
+++ b/src/arch/riscv/process.cc
@@ -60,7 +60,9 @@
using namespace RiscvISA;

RiscvProcess::RiscvProcess(ProcessParams *params, ObjectFile *objFile) :
- Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ Process(params, new FuncPageTable(params->name, params->pid,
+ PageBytes),
+ objFile)
{
assert(!params->useArchPT);
const Addr stack_base = 0x7FFFFFFFFFFFFFFFL;
diff --git a/src/arch/sparc/process.cc b/src/arch/sparc/process.cc
index f92b646..64e4541 100644
--- a/src/arch/sparc/process.cc
+++ b/src/arch/sparc/process.cc
@@ -56,8 +56,9 @@

SparcProcess::SparcProcess(ProcessParams *params, ObjectFile *objFile,
Addr _StackBias)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
- StackBias(_StackBias)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile),
+ StackBias(_StackBias)
{
assert(!params->useArchPT);
// Initialize these to 0s
diff --git a/src/arch/x86/process.cc b/src/arch/x86/process.cc
index f11cc34..bea002d 100644
--- a/src/arch/x86/process.cc
+++ b/src/arch/x86/process.cc
@@ -101,9 +101,10 @@
: Process(params, params->useArchPT ?
static_cast<PageTableBase *>(
new ArchPageTable(params->name, params->pid,
- params->system)) :
+ params->system,
PageBytes)) :
static_cast<PageTableBase *>(
- new FuncPageTable(params->name,
params->pid)),
+ new FuncPageTable(params->name, params->pid,
+ PageBytes)),
objFile),
syscallDescs(_syscallDescs), numSyscallDescs(_numSyscallDescs)
{
diff --git a/src/mem/multi_level_page_table.hh
b/src/mem/multi_level_page_table.hh
index 402c371..0e079b7 100644
--- a/src/mem/multi_level_page_table.hh
+++ b/src/mem/multi_level_page_table.hh
@@ -140,7 +140,7 @@

public:
MultiLevelPageTable(const std::string &__name, uint64_t _pid,
- System *_sys);
+ System *_sys, Addr pageSize);
~MultiLevelPageTable();

void initState(ThreadContext* tc) override;
diff --git a/src/mem/multi_level_page_table_impl.hh
b/src/mem/multi_level_page_table_impl.hh
index 7cd32db..4ff5f5c 100644
--- a/src/mem/multi_level_page_table_impl.hh
+++ b/src/mem/multi_level_page_table_impl.hh
@@ -46,8 +46,9 @@

template <class ISAOps>
MultiLevelPageTable<ISAOps>::MultiLevelPageTable(const std::string &__name,
- uint64_t _pid, System
*_sys)
- : PageTableBase(__name, _pid), system(_sys),
+ uint64_t _pid, System
*_sys,
+ Addr pageSize)
+ : PageTableBase(__name, _pid, pageSize), system(_sys),
logLevelSize(PageTableLayout),
numLevels(logLevelSize.size())
{
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 883b47c..f784b21 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -73,8 +73,7 @@

public:

- PageTableBase(const std::string &__name, uint64_t _pid,
- Addr _pageSize = TheISA::PageBytes)
+ PageTableBase(const std::string &__name, uint64_t _pid, Addr _pageSize)
: pageSize(_pageSize), offsetMask(mask(floorLog2(_pageSize))),
pid(_pid), _name(__name)
{
@@ -211,8 +210,7 @@

public:

- FuncPageTable(const std::string &__name, uint64_t _pid,
- Addr _pageSize = TheISA::PageBytes);
+ FuncPageTable(const std::string &__name, uint64_t _pid, Addr
_pageSize);

~FuncPageTable();
--
To view, visit https://gem5-review.googlesource.com/6982
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I071f9ed73aef78e1cd1752247c183e30854b2d28
Gerrit-Change-Number: 6982
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <***@google.com>
Gabe Black (Gerrit)
2018-01-11 06:30:16 UTC
Permalink
Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/6982 )

Change subject: arch,mem: Remove the default value for page size.
......................................................................

arch,mem: Remove the default value for page size.

This breaks one more architecture dependence outside of the ISAs.

Change-Id: I071f9ed73aef78e1cd1752247c183e30854b2d28
Reviewed-on: https://gem5-review.googlesource.com/6982
Maintainer: Gabe Black <***@google.com>
Reviewed-by: Alexandru Duțu <***@amd.com>
Reviewed-by: Jason Lowe-Power <***@lowepower.com>
Reviewed-by: Brandon Potter <***@amd.com>
---
M src/arch/alpha/process.cc
M src/arch/arm/process.cc
M src/arch/mips/process.cc
M src/arch/power/process.cc
M src/arch/riscv/process.cc
M src/arch/sparc/process.cc
M src/arch/x86/process.cc
M src/mem/multi_level_page_table.hh
M src/mem/multi_level_page_table_impl.hh
M src/mem/page_table.hh
10 files changed, 23 insertions(+), 16 deletions(-)

Approvals:
Jason Lowe-Power: Looks good to me, approved
Alexandru Duțu: Looks good to me, approved
Brandon Potter: Looks good to me, approved
Gabe Black: Looks good to me, approved



diff --git a/src/arch/alpha/process.cc b/src/arch/alpha/process.cc
index 3cc0b0d..bcfe362 100644
--- a/src/arch/alpha/process.cc
+++ b/src/arch/alpha/process.cc
@@ -49,7 +49,8 @@
using namespace std;

AlphaProcess::AlphaProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index b64579a..5daa54a 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -63,7 +63,8 @@

ArmProcess::ArmProcess(ProcessParams *params, ObjectFile *objFile,
ObjectFile::Arch _arch)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile),
arch(_arch)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index f3b1108..76f7e86 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -50,7 +50,8 @@
using namespace MipsISA;

MipsProcess::MipsProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Set up stack. On MIPS, stack starts at the top of kuseg
diff --git a/src/arch/power/process.cc b/src/arch/power/process.cc
index 87e5bac..343cd4b 100644
--- a/src/arch/power/process.cc
+++ b/src/arch/power/process.cc
@@ -50,7 +50,8 @@
using namespace PowerISA;

PowerProcess::PowerProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Set up break point (Top of Heap)
diff --git a/src/arch/riscv/process.cc b/src/arch/riscv/process.cc
index 88a093a..44b276a 100644
--- a/src/arch/riscv/process.cc
+++ b/src/arch/riscv/process.cc
@@ -60,7 +60,9 @@
using namespace RiscvISA;

RiscvProcess::RiscvProcess(ProcessParams *params, ObjectFile *objFile) :
- Process(params, new FuncPageTable(params->name, params->pid),
objFile)
+ Process(params, new FuncPageTable(params->name, params->pid,
+ PageBytes),
+ objFile)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
const Addr stack_base = 0x7FFFFFFFFFFFFFFFL;
diff --git a/src/arch/sparc/process.cc b/src/arch/sparc/process.cc
index fe91589..59ef5c4 100644
--- a/src/arch/sparc/process.cc
+++ b/src/arch/sparc/process.cc
@@ -56,8 +56,9 @@

SparcProcess::SparcProcess(ProcessParams *params, ObjectFile *objFile,
Addr _StackBias)
- : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
- StackBias(_StackBias)
+ : Process(params, new FuncPageTable(params->name, params->pid,
PageBytes),
+ objFile),
+ StackBias(_StackBias)
{
fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Initialize these to 0s
diff --git a/src/arch/x86/process.cc b/src/arch/x86/process.cc
index f11cc34..bea002d 100644
--- a/src/arch/x86/process.cc
+++ b/src/arch/x86/process.cc
@@ -101,9 +101,10 @@
: Process(params, params->useArchPT ?
static_cast<PageTableBase *>(
new ArchPageTable(params->name, params->pid,
- params->system)) :
+ params->system,
PageBytes)) :
static_cast<PageTableBase *>(
- new FuncPageTable(params->name,
params->pid)),
+ new FuncPageTable(params->name, params->pid,
+ PageBytes)),
objFile),
syscallDescs(_syscallDescs), numSyscallDescs(_numSyscallDescs)
{
diff --git a/src/mem/multi_level_page_table.hh
b/src/mem/multi_level_page_table.hh
index 402c371..0e079b7 100644
--- a/src/mem/multi_level_page_table.hh
+++ b/src/mem/multi_level_page_table.hh
@@ -140,7 +140,7 @@

public:
MultiLevelPageTable(const std::string &__name, uint64_t _pid,
- System *_sys);
+ System *_sys, Addr pageSize);
~MultiLevelPageTable();

void initState(ThreadContext* tc) override;
diff --git a/src/mem/multi_level_page_table_impl.hh
b/src/mem/multi_level_page_table_impl.hh
index 7cd32db..4ff5f5c 100644
--- a/src/mem/multi_level_page_table_impl.hh
+++ b/src/mem/multi_level_page_table_impl.hh
@@ -46,8 +46,9 @@

template <class ISAOps>
MultiLevelPageTable<ISAOps>::MultiLevelPageTable(const std::string &__name,
- uint64_t _pid, System
*_sys)
- : PageTableBase(__name, _pid), system(_sys),
+ uint64_t _pid, System
*_sys,
+ Addr pageSize)
+ : PageTableBase(__name, _pid, pageSize), system(_sys),
logLevelSize(PageTableLayout),
numLevels(logLevelSize.size())
{
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 883b47c..f784b21 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -73,8 +73,7 @@

public:

- PageTableBase(const std::string &__name, uint64_t _pid,
- Addr _pageSize = TheISA::PageBytes)
+ PageTableBase(const std::string &__name, uint64_t _pid, Addr _pageSize)
: pageSize(_pageSize), offsetMask(mask(floorLog2(_pageSize))),
pid(_pid), _name(__name)
{
@@ -211,8 +210,7 @@

public:

- FuncPageTable(const std::string &__name, uint64_t _pid,
- Addr _pageSize = TheISA::PageBytes);
+ FuncPageTable(const std::string &__name, uint64_t _pid, Addr
_pageSize);

~FuncPageTable();


--
To view, visit https://gem5-review.googlesource.com/6982
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I071f9ed73aef78e1cd1752247c183e30854b2d28
Gerrit-Change-Number: 6982
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Assignee: Nikos Nikoleris <***@arm.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gerrit-CC: Nikos Nikoleris <***@arm.com>
Gabe Black (Gerrit)
2018-01-11 06:26:41 UTC
Permalink
Hello Jason Lowe-Power, Alexandru Duțu, Andreas Sandberg, Brandon Potter,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/6981

to look at the new patch set (#3).

Change subject: arch,mem: Move page table construction into the arch
classes.
......................................................................

arch,mem: Move page table construction into the arch classes.

This gets rid of an awkward NoArchPageTable class, and also gives the
arch a place to inject ISA specific parameters (specifically page size)
without having to have TheISA:: in the generic version of these types.

Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
---
M src/arch/alpha/process.cc
M src/arch/alpha/process.hh
M src/arch/arm/process.cc
M src/arch/arm/process.hh
M src/arch/mips/process.cc
M src/arch/mips/process.hh
M src/arch/power/process.cc
M src/arch/power/process.hh
M src/arch/riscv/process.cc
M src/arch/riscv/process.hh
M src/arch/sparc/process.cc
M src/arch/sparc/process.hh
M src/arch/x86/process.cc
M src/arch/x86/process.hh
M src/mem/page_table.hh
M src/sim/process.cc
M src/sim/process.hh
17 files changed, 45 insertions(+), 61 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/6981
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
Gerrit-Change-Number: 6981
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-11 06:26:46 UTC
Permalink
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/7350


Change subject: arch, mem: Abstract the data stored in the SE page tables.
......................................................................

arch, mem: Abstract the data stored in the SE page tables.

Rather than store the actual TLB entry that corresponds to a mapping,
we can just store some abstracted information (address, a few flags)
and then let the caller turn that into the appropriate entry. There
could potentially be some small amount of overhead from creating
entries vs. storing them and just installing them, but it's likely
pretty minimal since that only happens on a TLB miss (ideally rare),
and, if it is problematic, there could be some preallocated TLB
entries which are just minimally filled in as necessary.

This has the nice effect of finally making the page tables ISA
agnostic.

Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
---
M src/arch/alpha/faults.cc
M src/arch/sparc/faults.cc
M src/arch/x86/tlb.cc
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
M src/sim/syscall_emul.hh
7 files changed, 110 insertions(+), 92 deletions(-)



diff --git a/src/arch/alpha/faults.cc b/src/arch/alpha/faults.cc
index 89b0ece..3433844 100644
--- a/src/arch/alpha/faults.cc
+++ b/src/arch/alpha/faults.cc
@@ -196,11 +196,14 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(pc);
- panic_if(!entry, "Tried to execute unmapped address %#x.\n", pc);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(pc);
+ panic_if(!pte, "Tried to execute unmapped address %#x.\n", pc);

VAddr vaddr(pc);
- dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), *entry);
+ TlbEntry entry(p->pTable->pid(), vaddr.page(), pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+ dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), entry);
}

void
@@ -212,11 +215,14 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- if (!entry && p->fixupStackFault(vaddr))
- entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to access unmapped address %#x.\n",
(Addr)vaddr);
- dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), *entry);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ if (!pte && p->fixupStackFault(vaddr))
+ pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to access unmapped address %#x.\n", (Addr)vaddr);
+ TlbEntry entry(p->pTable->pid(), vaddr.page(), pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+ dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), entry);
}

} // namespace AlphaISA
diff --git a/src/arch/sparc/faults.cc b/src/arch/sparc/faults.cc
index 0b6a289..0f042b4 100644
--- a/src/arch/sparc/faults.cc
+++ b/src/arch/sparc/faults.cc
@@ -35,6 +35,7 @@

#include "arch/sparc/isa_traits.hh"
#include "arch/sparc/process.hh"
+#include "arch/sparc/tlb.hh"
#include "arch/sparc/types.hh"
#include "base/bitfield.hh"
#include "base/trace.hh"
@@ -629,8 +630,8 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to execute unmapped address %#x.\n", vaddr);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to execute unmapped address %#x.\n", vaddr);

Addr alignedvaddr = p->pTable->pageAlign(vaddr);

@@ -662,13 +663,17 @@
// the logic works out to the following for the context.
int context_id = (is_real_address || trapped) ? 0 : primary_context;

+ TlbEntry entry(p->pTable->pid(), alignedvaddr, pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+
// Insert the TLB entry.
// The entry specifying whether the address is "real" is set to
// false for syscall emulation mode regardless of whether the
// address is real in preceding code. Not sure sure that this is
// correct, but also not sure if it matters at all.
dynamic_cast<TLB *>(tc->getITBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry->pte);
+ insert(alignedvaddr, partition_id, context_id, false, entry.pte);
}

void
@@ -680,10 +685,10 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- if (!entry && p->fixupStackFault(vaddr))
- entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to access unmapped address %#x.\n", vaddr);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ if (!pte && p->fixupStackFault(vaddr))
+ pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to access unmapped address %#x.\n", vaddr);

Addr alignedvaddr = p->pTable->pageAlign(vaddr);

@@ -745,13 +750,17 @@
// The partition id distinguishes between virtualized environments.
int const partition_id = 0;

+ TlbEntry entry(p->pTable->pid(), alignedvaddr, pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+
// Insert the TLB entry.
// The entry specifying whether the address is "real" is set to
// false for syscall emulation mode regardless of whether the
// address is real in preceding code. Not sure sure that this is
// correct, but also not sure if it matters at all.
dynamic_cast<TLB *>(tc->getDTBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry->pte);
+ insert(alignedvaddr, partition_id, context_id, false, entry.pte);
}

void
diff --git a/src/arch/x86/tlb.cc b/src/arch/x86/tlb.cc
index 9336949..a3aec16 100644
--- a/src/arch/x86/tlb.cc
+++ b/src/arch/x86/tlb.cc
@@ -357,22 +357,26 @@
assert(entry);
} else {
Process *p = tc->getProcessPtr();
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && mode != Execute) {
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);
+ if (!pte && mode != Execute) {
// Check if we just need to grow the stack.
if (p->fixupStackFault(vaddr)) {
// If we did, lookup the entry for the new
page.
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}
}
- if (!newEntry) {
+ if (!pte) {
return std::make_shared<PageFault>(vaddr, true,
mode,
true, false);
} else {
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
DPRINTF(TLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
- entry = insert(alignedVaddr, *newEntry);
+ pte->paddr);
+ entry = insert(alignedVaddr, TlbEntry(
+ p->pTable->pid(), alignedVaddr, pte->paddr,
+ pte->flags &
EmulationPageTable::Uncacheable,
+ pte->flags &
EmulationPageTable::ReadOnly));
}
DPRINTF(TLB, "Miss was serviced.\n");
}
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index 9cbf3e8..05d22da 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -808,18 +808,19 @@
"at pc %#x.\n", vaddr, tc->instAddr());

Process *p = tc->getProcessPtr();
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);

- if (!newEntry && mode != BaseTLB::Execute) {
+ if (!pte && mode != BaseTLB::Execute) {
// penalize a "page fault" more
if (timing)
latency += missLatency2;

if (p->fixupStackFault(vaddr))
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

- if (!newEntry) {
+ if (!pte) {
return std::make_shared<PageFault>(vaddr, true,
mode, true,
false);
@@ -827,11 +828,11 @@
Addr alignedVaddr =
p->pTable->pageAlign(vaddr);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
- alignedVaddr, newEntry->pageStart());
+ alignedVaddr, pte->paddr);

- GpuTlbEntry gpuEntry;
- *(TlbEntry *)&gpuEntry = *newEntry;
- gpuEntry.valid = true;
+ GpuTlbEntry gpuEntry(
+ p->pTable->pid(), alignedVaddr,
+ pte->paddr, true);
entry = insert(alignedVaddr, gpuEntry);
}

@@ -1335,18 +1336,18 @@
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
assert(alignedVaddr == virtPageAddr);
#endif
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+ const EmulationPageTable::Entry *pte =
p->pTable->lookup(vaddr);
+ if (!pte && sender_state->tlbMode != BaseTLB::Execute &&
p->fixupStackFault(vaddr)) {
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

- if (newEntry) {
+ if (pte) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr, newEntry->paddr,
true);
+ new GpuTlbEntry(0, virtPageAddr, pte->paddr, true);
} else {
sender_state->tlbEntry =
new GpuTlbEntry(0, 0, 0, false);
@@ -1533,10 +1534,11 @@
assert(alignedVaddr == virt_page_addr);
#endif

- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && sender_state->tlbMode != BaseTLB::Execute
&&
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);
+ if (!pte && sender_state->tlbMode != BaseTLB::Execute &&
p->fixupStackFault(vaddr)) {
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

if (!sender_state->prefetch) {
@@ -1545,23 +1547,23 @@
assert(success);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr,
- newEntry->paddr, success);
+ new GpuTlbEntry(0, virt_page_addr,
+ pte->paddr, success);
} else {
// If this was a prefetch, then do the normal thing if
it
// was a successful translation. Otherwise, send an
empty
// TLB entry back so that it can be figured out as
empty and
// handled accordingly.
- if (newEntry) {
+ if (pte) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr,
- newEntry->paddr, success);
+ new GpuTlbEntry(0, virt_page_addr,
+ pte->paddr, success);
} else {
DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
alignedVaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index 2d38877..266d368 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -40,19 +40,11 @@
#include <string>

#include "base/trace.hh"
-#include "config/the_isa.hh"
#include "debug/MMU.hh"
#include "sim/faults.hh"
#include "sim/serialize.hh"

using namespace std;
-using namespace TheISA;
-
-EmulationPageTable::~EmulationPageTable()
-{
- for (auto &iter : pTable)
- delete iter.second;
-}

void
EmulationPageTable::map(Addr vaddr, Addr paddr, int64_t size, uint64_t
flags)
@@ -66,22 +58,17 @@
while (size > 0) {
auto it = pTable.find(vaddr);
if (it != pTable.end()) {
- if (clobber) {
- delete it->second;
- } else {
- // already mapped
- panic("EmulationPageTable::allocate: addr %#x already
mapped",
- vaddr);
- }
+ // already mapped
+ panic_if(!clobber,
+ "EmulationPageTable::allocate: addr %#x already
mapped",
+ vaddr);
} else {
- it = pTable.emplace(vaddr, nullptr).first;
+ it = pTable.emplace(vaddr, Entry()).first;
}

- it->second = new TheISA::TlbEntry(pid, vaddr, paddr,
- flags & Uncacheable,
- flags & ReadOnly);
+ it->second = Entry(paddr, flags);
eraseCacheEntry(vaddr);
- updateCache(vaddr, pTable[vaddr]);
+ updateCache(vaddr, &(it->second));
size -= pageSize;
vaddr += pageSize;
paddr += pageSize;
@@ -105,8 +92,7 @@
new_it->second = old_it->second;
pTable.erase(old_it);
eraseCacheEntry(vaddr);
- new_it->second->updateVaddr(new_vaddr);
- updateCache(new_vaddr, new_it->second);
+ updateCache(new_vaddr, &(new_it->second));
size -= pageSize;
vaddr += pageSize;
new_vaddr += pageSize;
@@ -117,7 +103,7 @@
EmulationPageTable::getMappings(std::vector<std::pair<Addr, Addr>>
*addr_maps)
{
for (auto &iter : pTable)
- addr_maps->push_back(make_pair(iter.first,
iter.second->pageStart()));
+ addr_maps->push_back(make_pair(iter.first, iter.second.paddr));
}

void
@@ -125,13 +111,12 @@
{
assert(pageOffset(vaddr) == 0);

- DPRINTF(MMU, "Unmapping page: %#x-%#x\n", vaddr, vaddr+ size);
+ DPRINTF(MMU, "Unmapping page: %#x-%#x\n", vaddr, vaddr + size);

while (size > 0) {
auto it = pTable.find(vaddr);
assert(it != pTable.end());
eraseCacheEntry(vaddr);
- delete it->second;
pTable.erase(it);
size -= pageSize;
vaddr += pageSize;
@@ -151,7 +136,7 @@
return true;
}

-TheISA::TlbEntry *
+const EmulationPageTable::Entry *
EmulationPageTable::lookup(Addr vaddr)
{
Addr page_addr = pageAlign(vaddr);
@@ -168,19 +153,19 @@
if (iter == pTable.end())
return nullptr;

- updateCache(page_addr, iter->second);
- return iter->second;
+ updateCache(page_addr, &(iter->second));
+ return &(iter->second);
}

bool
EmulationPageTable::translate(Addr vaddr, Addr &paddr)
{
- TheISA::TlbEntry *entry = lookup(vaddr);
+ const Entry *entry = lookup(vaddr);
if (!entry) {
DPRINTF(MMU, "Couldn't Translate: %#x\n", vaddr);
return false;
}
- paddr = pageOffset(vaddr) + entry->pageStart();
+ paddr = pageOffset(vaddr) + entry->paddr;
DPRINTF(MMU, "Translating: %#x->%#x\n", vaddr, paddr);
return true;
}
@@ -211,7 +196,8 @@
ScopedCheckpointSection sec(cp, csprintf("Entry%d", count++));

paramOut(cp, "vaddr", pte.first);
- pte.second->serialize(cp);
+ paramOut(cp, "paddr", pte.second.paddr);
+ paramOut(cp, "flags", pte.second.flags);
}
assert(count == pTable.size());
}
@@ -225,13 +211,14 @@
for (int i = 0; i < count; ++i) {
ScopedCheckpointSection sec(cp, csprintf("Entry%d", i));

- TheISA::TlbEntry *entry = new TheISA::TlbEntry();
- entry->unserialize(cp);
-
Addr vaddr;
- paramIn(cp, "vaddr", vaddr);
+ UNSERIALIZE_SCALAR(vaddr);
+ Addr paddr;
+ uint64_t flags;
+ UNSERIALIZE_SCALAR(paddr);
+ UNSERIALIZE_SCALAR(flags);

- pTable[vaddr] = entry;
+ pTable.emplace(vaddr, Entry(paddr, flags));
}
}

diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 4baac67..0891c1a 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -40,11 +40,8 @@
#include <string>
#include <unordered_map>

-#include "arch/isa_traits.hh"
-#include "arch/tlb.hh"
#include "base/intmath.hh"
#include "base/types.hh"
-#include "config/the_isa.hh"
#include "mem/request.hh"
#include "sim/serialize.hh"

@@ -52,8 +49,18 @@

class EmulationPageTable : public Serializable
{
+ public:
+ struct Entry
+ {
+ Addr paddr;
+ uint64_t flags;
+
+ Entry(Addr paddr, uint64_t flags) : paddr(paddr), flags(flags) {}
+ Entry() {}
+ };
+
protected:
- typedef std::unordered_map<Addr, TheISA::TlbEntry *> PTable;
+ typedef std::unordered_map<Addr, Entry> PTable;
typedef PTable::iterator PTableItr;
PTable pTable;

@@ -62,7 +69,7 @@
CacheElement() { entry = nullptr; }

Addr vaddr;
- TheISA::TlbEntry *entry;
+ Entry *entry;
};
CacheElement pTableCache[3];

@@ -72,7 +79,7 @@
* @param pte page table entry to return
*/
void
- updateCache(Addr vaddr, TheISA::TlbEntry *entry)
+ updateCache(Addr vaddr, Entry *entry)
{
pTableCache[2].entry = pTableCache[1].entry;
pTableCache[2].vaddr = pTableCache[1].vaddr;
@@ -103,7 +110,7 @@
const Addr pageSize;
const Addr offsetMask;

- const uint64_t pid;
+ const uint64_t _pid;
const std::string _name;

public:
@@ -111,12 +118,14 @@
EmulationPageTable(
const std::string &__name, uint64_t _pid, Addr _pageSize) :
pageSize(_pageSize), offsetMask(mask(floorLog2(_pageSize))),
- pid(_pid), _name(__name)
+ _pid(_pid), _name(__name)
{
assert(isPowerOf2(pageSize));
}

- virtual ~EmulationPageTable();
+ uint64_t pid() const { return _pid; };
+
+ virtual ~EmulationPageTable() {};

/* generic page table mapping flags
* unset | set
@@ -163,7 +172,7 @@
* @param vaddr The virtual address.
* @return The page table entry corresponding to vaddr.
*/
- virtual TheISA::TlbEntry *lookup(Addr vaddr);
+ const Entry *lookup(Addr vaddr);

/**
* Translate function
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index ef61299..eaa5e54 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -93,6 +93,7 @@
#include <memory>
#include <string>

+#include "arch/generic/tlb.hh"
#include "arch/utility.hh"
#include "base/intmath.hh"
#include "base/loader/object_file.hh"
--
To view, visit https://gem5-review.googlesource.com/7350
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
Gerrit-Change-Number: 7350
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <***@google.com>
Gabe Black (Gerrit)
2018-01-19 10:39:16 UTC
Permalink
Hello Jason Lowe-Power, Alexandru Duțu, Andreas Sandberg, Brandon Potter,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/7350

to look at the new patch set (#3).

Change subject: arch, mem: Abstract the data stored in the SE page tables.
......................................................................

arch, mem: Abstract the data stored in the SE page tables.

Rather than store the actual TLB entry that corresponds to a mapping,
we can just store some abstracted information (address, a few flags)
and then let the caller turn that into the appropriate entry. There
could potentially be some small amount of overhead from creating
entries vs. storing them and just installing them, but it's likely
pretty minimal since that only happens on a TLB miss (ideally rare),
and, if it is problematic, there could be some preallocated TLB
entries which are just minimally filled in as necessary.

This has the nice effect of finally making the page tables ISA
agnostic.

Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
---
M src/arch/alpha/faults.cc
M src/arch/sparc/faults.cc
M src/arch/x86/tlb.cc
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
M src/sim/syscall_emul.hh
7 files changed, 105 insertions(+), 87 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/7350
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
Gerrit-Change-Number: 7350
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-23 20:38:35 UTC
Permalink
Hello Jason Lowe-Power, Alexandru Duțu, Andreas Sandberg, Brandon Potter,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/7350

to look at the new patch set (#5).

Change subject: tarch, mem: Abstract the data stored in the SE page tables.
......................................................................

tarch, mem: Abstract the data stored in the SE page tables.

Rather than store the actual TLB entry that corresponds to a mapping,
we can just store some abstracted information (address, a few flags)
and then let the caller turn that into the appropriate entry. There
could potentially be some small amount of overhead from creating
entries vs. storing them and just installing them, but it's likely
pretty minimal since that only happens on a TLB miss (ideally rare),
and, if it is problematic, there could be some preallocated TLB
entries which are just minimally filled in as necessary.

This has the nice effect of finally making the page tables ISA
agnostic.

Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
---
M src/arch/alpha/faults.cc
M src/arch/sparc/faults.cc
M src/arch/x86/tlb.cc
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
M src/sim/syscall_emul.hh
7 files changed, 105 insertions(+), 87 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/7350
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
Gerrit-Change-Number: 7350
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-23 20:39:18 UTC
Permalink
Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/7350 )

Change subject: tarch, mem: Abstract the data stored in the SE page tables.
......................................................................

tarch, mem: Abstract the data stored in the SE page tables.

Rather than store the actual TLB entry that corresponds to a mapping,
we can just store some abstracted information (address, a few flags)
and then let the caller turn that into the appropriate entry. There
could potentially be some small amount of overhead from creating
entries vs. storing them and just installing them, but it's likely
pretty minimal since that only happens on a TLB miss (ideally rare),
and, if it is problematic, there could be some preallocated TLB
entries which are just minimally filled in as necessary.

This has the nice effect of finally making the page tables ISA
agnostic.

Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
Reviewed-on: https://gem5-review.googlesource.com/7350
Reviewed-by: Gabe Black <***@google.com>
Maintainer: Gabe Black <***@google.com>
---
M src/arch/alpha/faults.cc
M src/arch/sparc/faults.cc
M src/arch/x86/tlb.cc
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
M src/sim/syscall_emul.hh
7 files changed, 105 insertions(+), 87 deletions(-)

Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved



diff --git a/src/arch/alpha/faults.cc b/src/arch/alpha/faults.cc
index 89b0ece..3433844 100644
--- a/src/arch/alpha/faults.cc
+++ b/src/arch/alpha/faults.cc
@@ -196,11 +196,14 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(pc);
- panic_if(!entry, "Tried to execute unmapped address %#x.\n", pc);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(pc);
+ panic_if(!pte, "Tried to execute unmapped address %#x.\n", pc);

VAddr vaddr(pc);
- dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), *entry);
+ TlbEntry entry(p->pTable->pid(), vaddr.page(), pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+ dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), entry);
}

void
@@ -212,11 +215,14 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- if (!entry && p->fixupStackFault(vaddr))
- entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to access unmapped address %#x.\n",
(Addr)vaddr);
- dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), *entry);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ if (!pte && p->fixupStackFault(vaddr))
+ pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to access unmapped address %#x.\n", (Addr)vaddr);
+ TlbEntry entry(p->pTable->pid(), vaddr.page(), pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+ dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), entry);
}

} // namespace AlphaISA
diff --git a/src/arch/sparc/faults.cc b/src/arch/sparc/faults.cc
index 0b6a289..0f042b4 100644
--- a/src/arch/sparc/faults.cc
+++ b/src/arch/sparc/faults.cc
@@ -35,6 +35,7 @@

#include "arch/sparc/isa_traits.hh"
#include "arch/sparc/process.hh"
+#include "arch/sparc/tlb.hh"
#include "arch/sparc/types.hh"
#include "base/bitfield.hh"
#include "base/trace.hh"
@@ -629,8 +630,8 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to execute unmapped address %#x.\n", vaddr);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to execute unmapped address %#x.\n", vaddr);

Addr alignedvaddr = p->pTable->pageAlign(vaddr);

@@ -662,13 +663,17 @@
// the logic works out to the following for the context.
int context_id = (is_real_address || trapped) ? 0 : primary_context;

+ TlbEntry entry(p->pTable->pid(), alignedvaddr, pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+
// Insert the TLB entry.
// The entry specifying whether the address is "real" is set to
// false for syscall emulation mode regardless of whether the
// address is real in preceding code. Not sure sure that this is
// correct, but also not sure if it matters at all.
dynamic_cast<TLB *>(tc->getITBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry->pte);
+ insert(alignedvaddr, partition_id, context_id, false, entry.pte);
}

void
@@ -680,10 +685,10 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry *entry = p->pTable->lookup(vaddr);
- if (!entry && p->fixupStackFault(vaddr))
- entry = p->pTable->lookup(vaddr);
- panic_if(!entry, "Tried to access unmapped address %#x.\n", vaddr);
+ const EmulationPageTable::Entry *pte = p->pTable->lookup(vaddr);
+ if (!pte && p->fixupStackFault(vaddr))
+ pte = p->pTable->lookup(vaddr);
+ panic_if(!pte, "Tried to access unmapped address %#x.\n", vaddr);

Addr alignedvaddr = p->pTable->pageAlign(vaddr);

@@ -745,13 +750,17 @@
// The partition id distinguishes between virtualized environments.
int const partition_id = 0;

+ TlbEntry entry(p->pTable->pid(), alignedvaddr, pte->paddr,
+ pte->flags & EmulationPageTable::Uncacheable,
+ pte->flags & EmulationPageTable::ReadOnly);
+
// Insert the TLB entry.
// The entry specifying whether the address is "real" is set to
// false for syscall emulation mode regardless of whether the
// address is real in preceding code. Not sure sure that this is
// correct, but also not sure if it matters at all.
dynamic_cast<TLB *>(tc->getDTBPtr())->
- insert(alignedvaddr, partition_id, context_id, false, entry->pte);
+ insert(alignedvaddr, partition_id, context_id, false, entry.pte);
}

void
diff --git a/src/arch/x86/tlb.cc b/src/arch/x86/tlb.cc
index 9336949..a3aec16 100644
--- a/src/arch/x86/tlb.cc
+++ b/src/arch/x86/tlb.cc
@@ -357,22 +357,26 @@
assert(entry);
} else {
Process *p = tc->getProcessPtr();
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && mode != Execute) {
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);
+ if (!pte && mode != Execute) {
// Check if we just need to grow the stack.
if (p->fixupStackFault(vaddr)) {
// If we did, lookup the entry for the new
page.
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}
}
- if (!newEntry) {
+ if (!pte) {
return std::make_shared<PageFault>(vaddr, true,
mode,
true, false);
} else {
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
DPRINTF(TLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
- entry = insert(alignedVaddr, *newEntry);
+ pte->paddr);
+ entry = insert(alignedVaddr, TlbEntry(
+ p->pTable->pid(), alignedVaddr, pte->paddr,
+ pte->flags &
EmulationPageTable::Uncacheable,
+ pte->flags &
EmulationPageTable::ReadOnly));
}
DPRINTF(TLB, "Miss was serviced.\n");
}
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index 9cbf3e8..05d22da 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -808,18 +808,19 @@
"at pc %#x.\n", vaddr, tc->instAddr());

Process *p = tc->getProcessPtr();
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);

- if (!newEntry && mode != BaseTLB::Execute) {
+ if (!pte && mode != BaseTLB::Execute) {
// penalize a "page fault" more
if (timing)
latency += missLatency2;

if (p->fixupStackFault(vaddr))
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

- if (!newEntry) {
+ if (!pte) {
return std::make_shared<PageFault>(vaddr, true,
mode, true,
false);
@@ -827,11 +828,11 @@
Addr alignedVaddr =
p->pTable->pageAlign(vaddr);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
- alignedVaddr, newEntry->pageStart());
+ alignedVaddr, pte->paddr);

- GpuTlbEntry gpuEntry;
- *(TlbEntry *)&gpuEntry = *newEntry;
- gpuEntry.valid = true;
+ GpuTlbEntry gpuEntry(
+ p->pTable->pid(), alignedVaddr,
+ pte->paddr, true);
entry = insert(alignedVaddr, gpuEntry);
}

@@ -1335,18 +1336,18 @@
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
assert(alignedVaddr == virtPageAddr);
#endif
- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+ const EmulationPageTable::Entry *pte =
p->pTable->lookup(vaddr);
+ if (!pte && sender_state->tlbMode != BaseTLB::Execute &&
p->fixupStackFault(vaddr)) {
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

- if (newEntry) {
+ if (pte) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr, newEntry->paddr,
true);
+ new GpuTlbEntry(0, virtPageAddr, pte->paddr, true);
} else {
sender_state->tlbEntry =
new GpuTlbEntry(0, 0, 0, false);
@@ -1533,10 +1534,11 @@
assert(alignedVaddr == virt_page_addr);
#endif

- TlbEntry *newEntry = p->pTable->lookup(vaddr);
- if (!newEntry && sender_state->tlbMode != BaseTLB::Execute
&&
+ const EmulationPageTable::Entry *pte =
+ p->pTable->lookup(vaddr);
+ if (!pte && sender_state->tlbMode != BaseTLB::Execute &&
p->fixupStackFault(vaddr)) {
- newEntry = p->pTable->lookup(vaddr);
+ pte = p->pTable->lookup(vaddr);
}

if (!sender_state->prefetch) {
@@ -1545,23 +1547,23 @@
assert(success);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr,
- newEntry->paddr, success);
+ new GpuTlbEntry(0, virt_page_addr,
+ pte->paddr, success);
} else {
// If this was a prefetch, then do the normal thing if
it
// was a successful translation. Otherwise, send an
empty
// TLB entry back so that it can be figured out as
empty and
// handled accordingly.
- if (newEntry) {
+ if (pte) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
alignedVaddr,
- newEntry->pageStart());
+ pte->paddr);

sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry->vaddr,
- newEntry->paddr, success);
+ new GpuTlbEntry(0, virt_page_addr,
+ pte->paddr, success);
} else {
DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
alignedVaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index ee50419..8a11ada 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -40,19 +40,11 @@
#include <string>

#include "base/trace.hh"
-#include "config/the_isa.hh"
#include "debug/MMU.hh"
#include "sim/faults.hh"
#include "sim/serialize.hh"

using namespace std;
-using namespace TheISA;
-
-EmulationPageTable::~EmulationPageTable()
-{
- for (auto &iter : pTable)
- delete iter.second;
-}

void
EmulationPageTable::map(Addr vaddr, Addr paddr, int64_t size, uint64_t
flags)
@@ -66,20 +58,15 @@
while (size > 0) {
auto it = pTable.find(vaddr);
if (it != pTable.end()) {
- if (clobber) {
- delete it->second;
- } else {
- // already mapped
- panic("EmulationPageTable::allocate: addr %#x already
mapped",
- vaddr);
- }
+ // already mapped
+ panic_if(!clobber,
+ "EmulationPageTable::allocate: addr %#x already
mapped",
+ vaddr);
+ it->second = Entry(paddr, flags);
} else {
- it = pTable.emplace(vaddr, nullptr).first;
+ pTable.emplace(vaddr, Entry(paddr, flags));
}

- it->second = new TheISA::TlbEntry(pid, vaddr, paddr,
- flags & Uncacheable,
- flags & ReadOnly);
size -= pageSize;
vaddr += pageSize;
paddr += pageSize;
@@ -102,7 +89,6 @@

new_it->second = old_it->second;
pTable.erase(old_it);
- new_it->second->updateVaddr(new_vaddr);
size -= pageSize;
vaddr += pageSize;
new_vaddr += pageSize;
@@ -113,7 +99,7 @@
EmulationPageTable::getMappings(std::vector<std::pair<Addr, Addr>>
*addr_maps)
{
for (auto &iter : pTable)
- addr_maps->push_back(make_pair(iter.first,
iter.second->pageStart()));
+ addr_maps->push_back(make_pair(iter.first, iter.second.paddr));
}

void
@@ -121,12 +107,11 @@
{
assert(pageOffset(vaddr) == 0);

- DPRINTF(MMU, "Unmapping page: %#x-%#x\n", vaddr, vaddr+ size);
+ DPRINTF(MMU, "Unmapping page: %#x-%#x\n", vaddr, vaddr + size);

while (size > 0) {
auto it = pTable.find(vaddr);
assert(it != pTable.end());
- delete it->second;
pTable.erase(it);
size -= pageSize;
vaddr += pageSize;
@@ -146,25 +131,25 @@
return true;
}

-TheISA::TlbEntry *
+const EmulationPageTable::Entry *
EmulationPageTable::lookup(Addr vaddr)
{
Addr page_addr = pageAlign(vaddr);
PTableItr iter = pTable.find(page_addr);
if (iter == pTable.end())
return nullptr;
- return iter->second;
+ return &(iter->second);
}

bool
EmulationPageTable::translate(Addr vaddr, Addr &paddr)
{
- TheISA::TlbEntry *entry = lookup(vaddr);
+ const Entry *entry = lookup(vaddr);
if (!entry) {
DPRINTF(MMU, "Couldn't Translate: %#x\n", vaddr);
return false;
}
- paddr = pageOffset(vaddr) + entry->pageStart();
+ paddr = pageOffset(vaddr) + entry->paddr;
DPRINTF(MMU, "Translating: %#x->%#x\n", vaddr, paddr);
return true;
}
@@ -195,7 +180,8 @@
ScopedCheckpointSection sec(cp, csprintf("Entry%d", count++));

paramOut(cp, "vaddr", pte.first);
- pte.second->serialize(cp);
+ paramOut(cp, "paddr", pte.second.paddr);
+ paramOut(cp, "flags", pte.second.flags);
}
assert(count == pTable.size());
}
@@ -209,13 +195,14 @@
for (int i = 0; i < count; ++i) {
ScopedCheckpointSection sec(cp, csprintf("Entry%d", i));

- TheISA::TlbEntry *entry = new TheISA::TlbEntry();
- entry->unserialize(cp);
-
Addr vaddr;
- paramIn(cp, "vaddr", vaddr);
+ UNSERIALIZE_SCALAR(vaddr);
+ Addr paddr;
+ uint64_t flags;
+ UNSERIALIZE_SCALAR(paddr);
+ UNSERIALIZE_SCALAR(flags);

- pTable[vaddr] = entry;
+ pTable.emplace(vaddr, Entry(paddr, flags));
}
}

diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 733cdd2..fc0c092 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -40,11 +40,8 @@
#include <string>
#include <unordered_map>

-#include "arch/isa_traits.hh"
-#include "arch/tlb.hh"
#include "base/intmath.hh"
#include "base/types.hh"
-#include "config/the_isa.hh"
#include "mem/request.hh"
#include "sim/serialize.hh"

@@ -52,15 +49,25 @@

class EmulationPageTable : public Serializable
{
+ public:
+ struct Entry
+ {
+ Addr paddr;
+ uint64_t flags;
+
+ Entry(Addr paddr, uint64_t flags) : paddr(paddr), flags(flags) {}
+ Entry() {}
+ };
+
protected:
- typedef std::unordered_map<Addr, TheISA::TlbEntry *> PTable;
+ typedef std::unordered_map<Addr, Entry> PTable;
typedef PTable::iterator PTableItr;
PTable pTable;

const Addr pageSize;
const Addr offsetMask;

- const uint64_t pid;
+ const uint64_t _pid;
const std::string _name;

public:
@@ -68,12 +75,14 @@
EmulationPageTable(
const std::string &__name, uint64_t _pid, Addr _pageSize) :
pageSize(_pageSize), offsetMask(mask(floorLog2(_pageSize))),
- pid(_pid), _name(__name)
+ _pid(_pid), _name(__name)
{
assert(isPowerOf2(pageSize));
}

- virtual ~EmulationPageTable();
+ uint64_t pid() const { return _pid; };
+
+ virtual ~EmulationPageTable() {};

/* generic page table mapping flags
* unset | set
@@ -120,7 +129,7 @@
* @param vaddr The virtual address.
* @return The page table entry corresponding to vaddr.
*/
- virtual TheISA::TlbEntry *lookup(Addr vaddr);
+ const Entry *lookup(Addr vaddr);

/**
* Translate function
diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index ef61299..eaa5e54 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -93,6 +93,7 @@
#include <memory>
#include <string>

+#include "arch/generic/tlb.hh"
#include "arch/utility.hh"
#include "base/intmath.hh"
#include "base/loader/object_file.hh"

--
To view, visit https://gem5-review.googlesource.com/7350
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I11e630f60682f0a0029b0683eb8ff0135fbd4317
Gerrit-Change-Number: 7350
Gerrit-PatchSet: 6
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>

Gabe Black (Gerrit)
2018-01-11 06:26:45 UTC
Permalink
Gabe Black has uploaded this change for review. (
https://gem5-review.googlesource.com/7343


Change subject: arch, mem: Make the page table lookup function return a
pointer.
......................................................................

arch, mem: Make the page table lookup function return a pointer.

This avoids having a copy in the lookup function itself, and the
declaration of a lot of temporary TLB entry pointers in callers. The
gpu TLB seems to have had the most dependence on the original signature
of the lookup function, partially because it was relying on a somewhat
unsafe copy to a TLB entry using a base class pointer type.

Change-Id: I8b1cf494468163deee000002d243541657faf57f
---
M src/arch/alpha/faults.cc
M src/arch/arm/remote_gdb.cc
M src/arch/mips/remote_gdb.cc
M src/arch/power/remote_gdb.cc
M src/arch/riscv/remote_gdb.cc
M src/arch/sparc/faults.cc
M src/arch/sparc/remote_gdb.cc
M src/arch/x86/remote_gdb.cc
M src/arch/x86/tlb.cc
M src/arch/x86/tlb.hh
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
13 files changed, 183 insertions(+), 226 deletions(-)



diff --git a/src/arch/alpha/faults.cc b/src/arch/alpha/faults.cc
index 4a829cd..89b0ece 100644
--- a/src/arch/alpha/faults.cc
+++ b/src/arch/alpha/faults.cc
@@ -196,14 +196,11 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(pc, entry);
- if (!success) {
- panic("Tried to execute unmapped address %#x.\n", pc);
- } else {
- VAddr vaddr(pc);
- dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), entry);
- }
+ TlbEntry *entry = p->pTable->lookup(pc);
+ panic_if(!entry, "Tried to execute unmapped address %#x.\n", pc);
+
+ VAddr vaddr(pc);
+ dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), *entry);
}

void
@@ -215,17 +212,11 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, entry);
- }
- if (!success) {
- panic("Tried to access unmapped address %#x.\n", (Addr)vaddr);
- } else {
- dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), entry);
- }
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ if (!entry && p->fixupStackFault(vaddr))
+ entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to access unmapped address %#x.\n",
(Addr)vaddr);
+ dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), *entry);
}

} // namespace AlphaISA
diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index 6dc68b1..333ba70 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -186,12 +186,9 @@
DPRINTF(GDBAcc, "acc: %#x mapping is valid\n", va);
return true;
} else {
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space.
- if (context->getProcessPtr()->pTable->lookup(va, entry))
- return true;
- return false;
+ // Check to make sure the first byte is mapped into the processes
+ // address space.
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/mips/remote_gdb.cc b/src/arch/mips/remote_gdb.cc
index 2cc2d77..dc0c27e 100644
--- a/src/arch/mips/remote_gdb.cc
+++ b/src/arch/mips/remote_gdb.cc
@@ -162,13 +162,10 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space.
- if (FullSystem)
- panic("acc not implemented for MIPS FS!");
- else
- return context->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes
address
+ // space.
+ panic_if(FullSystem, "acc not implemented for MIPS FS!");
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/power/remote_gdb.cc b/src/arch/power/remote_gdb.cc
index c85aa38..30deba7 100644
--- a/src/arch/power/remote_gdb.cc
+++ b/src/arch/power/remote_gdb.cc
@@ -162,16 +162,13 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space. At the time of this writing, the acc() check is used when
- //processing the MemR/MemW packets before actually asking the
translating
- //port proxy to read/writeBlob. I (bgs) am not convinced the first
byte
- //check is enough.
- if (FullSystem)
- panic("acc not implemented for POWER FS!");
- else
- return context->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes
address
+ // space. At the time of this writing, the acc() check is used when
+ // processing the MemR/MemW packets before actually asking the
translating
+ // port proxy to read/writeBlob. I (bgs) am not convinced the first
byte
+ // check is enough.
+ panic_if(FullSystem, "acc not implemented for POWER FS!");
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/riscv/remote_gdb.cc b/src/arch/riscv/remote_gdb.cc
index 3488c81..e3dba4a 100644
--- a/src/arch/riscv/remote_gdb.cc
+++ b/src/arch/riscv/remote_gdb.cc
@@ -156,11 +156,8 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- if (FullSystem)
- panic("acc not implemented for RISCV FS!");
- else
- return context->getProcessPtr()->pTable->lookup(va, entry);
+ panic_if(FullSystem, "acc not implemented for RISCV FS!");
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/sparc/faults.cc b/src/arch/sparc/faults.cc
index 5466115..0b6a289 100644
--- a/src/arch/sparc/faults.cc
+++ b/src/arch/sparc/faults.cc
@@ -629,49 +629,46 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- panic("Tried to execute unmapped address %#x.\n", vaddr);
- } else {
- Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to execute unmapped address %#x.\n", vaddr);

- // Grab fields used during instruction translation to figure out
- // which context to use.
- uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+ Addr alignedvaddr = p->pTable->pageAlign(vaddr);

- // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical
address,
- // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the
context is
- // zero (kernel-level) if real addressing is being used.
- bool is_real_address = !bits(tlbdata, 4);
+ // Grab fields used during instruction translation to figure out
+ // which context to use.
+ uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

- // The SPARC ITLB code assumes that traps are executed in context
- // zero so we carry that assumption through here.
- bool trapped = bits(tlbdata, 18, 16) > 0;
+ // Inside a VM, a real address is the address that guest OS would
+ // interpret to be a physical address. To map to the physical address,
+ // it still needs to undergo a translation. The instruction
+ // translation code in the SPARC ITLB code assumes that the context is
+ // zero (kernel-level) if real addressing is being used.
+ bool is_real_address = !bits(tlbdata, 4);

- // The primary context acts as a PASID. It allows the MMU to
- // distinguish between virtual addresses that would alias to the
- // same physical address (if two or more processes shared the same
- // virtual address mapping).
- int primary_context = bits(tlbdata, 47, 32);
+ // The SPARC ITLB code assumes that traps are executed in context
+ // zero so we carry that assumption through here.
+ bool trapped = bits(tlbdata, 18, 16) > 0;

- // The partition id distinguishes between virtualized environments.
- int const partition_id = 0;
+ // The primary context acts as a PASID. It allows the MMU to
+ // distinguish between virtual addresses that would alias to the
+ // same physical address (if two or more processes shared the same
+ // virtual address mapping).
+ int primary_context = bits(tlbdata, 47, 32);

- // Given the assumptions in the translateInst code in the SPARC
ITLB,
- // the logic works out to the following for the context.
- int context_id = (is_real_address || trapped) ? 0 :
primary_context;
+ // The partition id distinguishes between virtualized environments.
+ int const partition_id = 0;

- // Insert the TLB entry.
- // The entry specifying whether the address is "real" is set to
- // false for syscall emulation mode regardless of whether the
- // address is real in preceding code. Not sure sure that this is
- // correct, but also not sure if it matters at all.
- dynamic_cast<TLB *>(tc->getITBPtr())->
- insert(alignedvaddr, partition_id, context_id, false,
entry.pte);
- }
+ // Given the assumptions in the translateInst code in the SPARC ITLB,
+ // the logic works out to the following for the context.
+ int context_id = (is_real_address || trapped) ? 0 : primary_context;
+
+ // Insert the TLB entry.
+ // The entry specifying whether the address is "real" is set to
+ // false for syscall emulation mode regardless of whether the
+ // address is real in preceding code. Not sure sure that this is
+ // correct, but also not sure if it matters at all.
+ dynamic_cast<TLB *>(tc->getITBPtr())->
+ insert(alignedvaddr, partition_id, context_id, false, entry->pte);
}

void
@@ -683,83 +680,78 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, entry);
- }
- if (!success) {
- panic("Tried to access unmapped address %#x.\n", vaddr);
- } else {
- Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ if (!entry && p->fixupStackFault(vaddr))
+ entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to access unmapped address %#x.\n", vaddr);

- // Grab fields used during data translation to figure out
- // which context to use.
- uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+ Addr alignedvaddr = p->pTable->pageAlign(vaddr);

- // The primary context acts as a PASID. It allows the MMU to
- // distinguish between virtual addresses that would alias to the
- // same physical address (if two or more processes shared the same
- // virtual address mapping). There's a secondary context used in
the
- // DTLB translation code, but it should __probably__ be zero for
- // syscall emulation code. (The secondary context is used by
Solaris
- // to allow kernel privilege code to access user space code:
- // [ISBN 0-13-022496-0]:PG199.)
- int primary_context = bits(tlbdata, 47, 32);
+ // Grab fields used during data translation to figure out
+ // which context to use.
+ uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

- // "Hyper-Privileged Mode" is in use. There are three main modes of
- // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
- // User Mode.
- int hpriv = bits(tlbdata, 0);
+ // The primary context acts as a PASID. It allows the MMU to
+ // distinguish between virtual addresses that would alias to the
+ // same physical address (if two or more processes shared the same
+ // virtual address mapping). There's a secondary context used in the
+ // DTLB translation code, but it should __probably__ be zero for
+ // syscall emulation code. (The secondary context is used by Solaris
+ // to allow kernel privilege code to access user space code:
+ // [ISBN 0-13-022496-0]:PG199.)
+ int primary_context = bits(tlbdata, 47, 32);

- // Reset, Error and Debug state is in use. Something horrible has
- // happened or the system is operating in Reset Mode.
- int red = bits(tlbdata, 1);
+ // "Hyper-Privileged Mode" is in use. There are three main modes of
+ // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
+ // User Mode.
+ int hpriv = bits(tlbdata, 0);

- // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical
address,
- // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the
context is
- // zero (kernel-level) if real addressing is being used.
- int is_real_address = !bits(tlbdata, 5);
+ // Reset, Error and Debug state is in use. Something horrible has
+ // happened or the system is operating in Reset Mode.
+ int red = bits(tlbdata, 1);

- // Grab the address space identifier register from the thread
context.
- // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
- // MISCREG_ASI causes me to think that the ASI register
implementation
- // might be bugged. The NoEffect variant changes the ASI register
- // value in the architectural state while the normal variant
changes
- // the context field in the thread context's currently decoded
request
- // but does not directly affect the ASI register value in the
- // architectural state. The ASI values and the context field in the
- // request packet seem to have completely different uses.
- MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
- ASI asi = static_cast<ASI>(reg_asi);
+ // Inside a VM, a real address is the address that guest OS would
+ // interpret to be a physical address. To map to the physical address,
+ // it still needs to undergo a translation. The instruction
+ // translation code in the SPARC ITLB code assumes that the context is
+ // zero (kernel-level) if real addressing is being used.
+ int is_real_address = !bits(tlbdata, 5);

- // The SPARC DTLB code assumes that traps are executed in context
- // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's
also
- // an assumption that the nucleus address space is being used, but
- // the context is the relevant issue since we need to pass it to
TLB.
- bool trapped = bits(tlbdata, 18, 16) > 0;
+ // Grab the address space identifier register from the thread context.
+ // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
+ // MISCREG_ASI causes me to think that the ASI register implementation
+ // might be bugged. The NoEffect variant changes the ASI register
+ // value in the architectural state while the normal variant changes
+ // the context field in the thread context's currently decoded request
+ // but does not directly affect the ASI register value in the
+ // architectural state. The ASI values and the context field in the
+ // request packet seem to have completely different uses.
+ MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
+ ASI asi = static_cast<ASI>(reg_asi);

- // Given the assumptions in the translateData code in the SPARC
DTLB,
- // the logic works out to the following for the context.
- int context_id = ((!hpriv && !red && is_real_address) ||
- asiIsReal(asi) ||
- (trapped && asi == ASI_IMPLICIT))
- ? 0 : primary_context;
+ // The SPARC DTLB code assumes that traps are executed in context
+ // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's also
+ // an assumption that the nucleus address space is being used, but
+ // the context is the relevant issue since we need to pass it to TLB.
+ bool trapped = bits(tlbdata, 18, 16) > 0;

- // The partition id distinguishes between virtualized environments.
- int const partition_id = 0;
+ // Given the assumptions in the translateData code in the SPARC DTLB,
+ // the logic works out to the following for the context.
+ int context_id = ((!hpriv && !red && is_real_address) ||
+ asiIsReal(asi) ||
+ (trapped && asi == ASI_IMPLICIT))
+ ? 0 : primary_context;

- // Insert the TLB entry.
- // The entry specifying whether the address is "real" is set to
- // false for syscall emulation mode regardless of whether the
- // address is real in preceding code. Not sure sure that this is
- // correct, but also not sure if it matters at all.
- dynamic_cast<TLB *>(tc->getDTBPtr())->
- insert(alignedvaddr, partition_id, context_id, false,
entry.pte);
- }
+ // The partition id distinguishes between virtualized environments.
+ int const partition_id = 0;
+
+ // Insert the TLB entry.
+ // The entry specifying whether the address is "real" is set to
+ // false for syscall emulation mode regardless of whether the
+ // address is real in preceding code. Not sure sure that this is
+ // correct, but also not sure if it matters at all.
+ dynamic_cast<TLB *>(tc->getDTBPtr())->
+ insert(alignedvaddr, partition_id, context_id, false, entry->pte);
}

void
diff --git a/src/arch/sparc/remote_gdb.cc b/src/arch/sparc/remote_gdb.cc
index 3f4df0d..778bb24 100644
--- a/src/arch/sparc/remote_gdb.cc
+++ b/src/arch/sparc/remote_gdb.cc
@@ -163,16 +163,11 @@
// from va to va + len have valid page map entries. Not
// sure how this will work for other OSes or in general.
if (FullSystem) {
- if (va)
- return true;
- return false;
+ return va != 0;
} else {
- TlbEntry entry;
// Check to make sure the first byte is mapped into the processes
// address space.
- if (context->getProcessPtr()->pTable->lookup(va, entry))
- return true;
- return false;
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/x86/remote_gdb.cc b/src/arch/x86/remote_gdb.cc
index a6fdabd..45265d1 100644
--- a/src/arch/x86/remote_gdb.cc
+++ b/src/arch/x86/remote_gdb.cc
@@ -88,8 +88,7 @@
BaseTLB::Read);
return fault == NoFault;
} else {
- TlbEntry entry;
- return context->getProcessPtr()->pTable->lookup(va, entry);
+ return context->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/x86/tlb.cc b/src/arch/x86/tlb.cc
index 0b1df9e..9336949 100644
--- a/src/arch/x86/tlb.cc
+++ b/src/arch/x86/tlb.cc
@@ -94,7 +94,7 @@
}

TlbEntry *
-TLB::insert(Addr vpn, TlbEntry &entry)
+TLB::insert(Addr vpn, const TlbEntry &entry)
{
// If somebody beat us to it, just use that existing entry.
TlbEntry *newEntry = trie.lookup(vpn);
@@ -357,23 +357,22 @@
assert(entry);
} else {
Process *p = tc->getProcessPtr();
- TlbEntry newEntry;
- bool success = p->pTable->lookup(vaddr, newEntry);
- if (!success && mode != Execute) {
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && mode != Execute) {
// Check if we just need to grow the stack.
if (p->fixupStackFault(vaddr)) {
// If we did, lookup the entry for the new
page.
- success = p->pTable->lookup(vaddr, newEntry);
+ newEntry = p->pTable->lookup(vaddr);
}
}
- if (!success) {
+ if (!newEntry) {
return std::make_shared<PageFault>(vaddr, true,
mode,
true, false);
} else {
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
DPRINTF(TLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
- entry = insert(alignedVaddr, newEntry);
+ newEntry->pageStart());
+ entry = insert(alignedVaddr, *newEntry);
}
DPRINTF(TLB, "Miss was serviced.\n");
}
diff --git a/src/arch/x86/tlb.hh b/src/arch/x86/tlb.hh
index c3dc83b..08804a4 100644
--- a/src/arch/x86/tlb.hh
+++ b/src/arch/x86/tlb.hh
@@ -144,7 +144,7 @@
Fault finalizePhysical(RequestPtr req, ThreadContext *tc,
Mode mode) const override;

- TlbEntry * insert(Addr vpn, TlbEntry &entry);
+ TlbEntry *insert(Addr vpn, const TlbEntry &entry);

/*
* Function to register Stats
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index b5411f8..9cbf3e8 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -808,31 +808,31 @@
"at pc %#x.\n", vaddr, tc->instAddr());

Process *p = tc->getProcessPtr();
- GpuTlbEntry newEntry;
- bool success = p->pTable->lookup(vaddr, newEntry);
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);

- if (!success && mode != BaseTLB::Execute) {
+ if (!newEntry && mode != BaseTLB::Execute) {
// penalize a "page fault" more
- if (timing) {
+ if (timing)
latency += missLatency2;
- }

if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr,
newEntry);
+ newEntry = p->pTable->lookup(vaddr);
}

- if (!success) {
+ if (!newEntry) {
return std::make_shared<PageFault>(vaddr, true,
mode, true,
false);
} else {
- newEntry.valid = success;
Addr alignedVaddr =
p->pTable->pageAlign(vaddr);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
- alignedVaddr, newEntry.pageStart());
+ alignedVaddr, newEntry->pageStart());

- entry = insert(alignedVaddr, newEntry);
+ GpuTlbEntry gpuEntry;
+ *(TlbEntry *)&gpuEntry = *newEntry;
+ gpuEntry.valid = true;
+ entry = insert(alignedVaddr, gpuEntry);
}

DPRINTF(GPUTLB, "Miss was serviced.\n");
@@ -1330,25 +1330,27 @@
safe_cast<TranslationState*>(pkt->senderState);

Process *p = sender_state->tc->getProcessPtr();
- TlbEntry newEntry;
Addr vaddr = pkt->req->getVaddr();
#ifndef NDEBUG
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
assert(alignedVaddr == virtPageAddr);
#endif
- bool success;
- success = p->pTable->lookup(vaddr, newEntry);
- if (!success && sender_state->tlbMode != BaseTLB::Execute) {
- if (p->fixupStackFault(vaddr)) {
- success = p->pTable->lookup(vaddr, newEntry);
- }
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+ p->fixupStackFault(vaddr)) {
+ newEntry = p->pTable->lookup(vaddr);
}

- DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
+ if (newEntry) {
+ DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
+ newEntry->pageStart());

- sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry.vaddr, newEntry.paddr,
success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr, newEntry->paddr,
true);
+ } else {
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, 0, 0, false);
+ }

handleTranslationReturn(virtPageAddr, TLB_MISS, pkt);
} else if (outcome == MISS_RETURN) {
@@ -1524,7 +1526,6 @@
virt_page_addr);

Process *p = tc->getProcessPtr();
- TlbEntry newEntry;

Addr vaddr = pkt->req->getVaddr();
#ifndef NDEBUG
@@ -1532,10 +1533,10 @@
assert(alignedVaddr == virt_page_addr);
#endif

- bool success = p->pTable->lookup(vaddr, newEntry);
- if (!success && sender_state->tlbMode != BaseTLB::Execute)
{
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, newEntry);
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && sender_state->tlbMode != BaseTLB::Execute
&&
+ p->fixupStackFault(vaddr)) {
+ newEntry = p->pTable->lookup(vaddr);
}

if (!sender_state->prefetch) {
@@ -1544,24 +1545,23 @@
assert(success);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
+ newEntry->pageStart());

- sender_state->tlbEntry = new GpuTlbEntry(0,
newEntry.vaddr,
-
newEntry.paddr,
- success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr,
+ newEntry->paddr, success);
} else {
// If this was a prefetch, then do the normal thing if
it
// was a successful translation. Otherwise, send an
empty
// TLB entry back so that it can be figured out as
empty and
// handled accordingly.
- if (success) {
+ if (newEntry) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
alignedVaddr,
- newEntry.pageStart());
+ newEntry->pageStart());

- sender_state->tlbEntry = new GpuTlbEntry(0,
-
newEntry.vaddr,
-
newEntry.paddr,
- success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr,
+ newEntry->paddr, success);
} else {
DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
alignedVaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index 4d62b80..3031c2a 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -151,43 +151,36 @@
return true;
}

-bool
-EmulationPageTable::lookup(Addr vaddr, TheISA::TlbEntry &entry)
+TheISA::TlbEntry *
+EmulationPageTable::lookup(Addr vaddr)
{
Addr page_addr = pageAlign(vaddr);

- if (pTableCache[0].entry && pTableCache[0].vaddr == page_addr) {
- entry = *pTableCache[0].entry;
- return true;
- }
- if (pTableCache[1].entry && pTableCache[1].vaddr == page_addr) {
- entry = *pTableCache[1].entry;
- return true;
- }
- if (pTableCache[2].entry && pTableCache[2].vaddr == page_addr) {
- entry = *pTableCache[2].entry;
- return true;
- }
+ if (pTableCache[0].entry && pTableCache[0].vaddr == page_addr)
+ return pTableCache[0].entry;
+ if (pTableCache[1].entry && pTableCache[1].vaddr == page_addr)
+ return pTableCache[1].entry;
+ if (pTableCache[2].entry && pTableCache[2].vaddr == page_addr)
+ return pTableCache[2].entry;

PTableItr iter = pTable.find(page_addr);

if (iter == pTable.end())
- return false;
+ return nullptr;

updateCache(page_addr, iter->second);
- entry = *iter->second;
- return true;
+ return iter->second;
}

bool
EmulationPageTable::translate(Addr vaddr, Addr &paddr)
{
- TheISA::TlbEntry entry;
- if (!lookup(vaddr, entry)) {
+ TheISA::TlbEntry *entry = lookup(vaddr);
+ if (!entry) {
DPRINTF(MMU, "Couldn't Translate: %#x\n", vaddr);
return false;
}
- paddr = pageOffset(vaddr) + entry.pageStart();
+ paddr = pageOffset(vaddr) + entry->pageStart();
DPRINTF(MMU, "Translating: %#x->%#x\n", vaddr, paddr);
return true;
}
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index de16740..cf3c378 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -164,9 +164,9 @@
/**
* Lookup function
* @param vaddr The virtual address.
- * @return entry The page table entry corresponding to vaddr.
+ * @return The page table entry corresponding to vaddr.
*/
- virtual bool lookup(Addr vaddr, TheISA::TlbEntry &entry);
+ virtual TheISA::TlbEntry *lookup(Addr vaddr);

/**
* Translate function
--
To view, visit https://gem5-review.googlesource.com/7343
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b1cf494468163deee000002d243541657faf57f
Gerrit-Change-Number: 7343
Gerrit-PatchSet: 1
Gerrit-Owner: Gabe Black <***@google.com>
Gabe Black (Gerrit)
2018-01-19 10:39:16 UTC
Permalink
Hello Jason Lowe-Power, Alexandru Duțu, Andreas Sandberg, Brandon Potter,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/7343

to look at the new patch set (#3).

Change subject: arch, mem: Make the page table lookup function return a
pointer.
......................................................................

arch, mem: Make the page table lookup function return a pointer.

This avoids having a copy in the lookup function itself, and the
declaration of a lot of temporary TLB entry pointers in callers. The
gpu TLB seems to have had the most dependence on the original signature
of the lookup function, partially because it was relying on a somewhat
unsafe copy to a TLB entry using a base class pointer type.

Change-Id: I8b1cf494468163deee000002d243541657faf57f
---
M src/arch/alpha/faults.cc
M src/arch/arm/remote_gdb.cc
M src/arch/mips/remote_gdb.cc
M src/arch/power/remote_gdb.cc
M src/arch/riscv/remote_gdb.cc
M src/arch/sparc/faults.cc
M src/arch/sparc/remote_gdb.cc
M src/arch/x86/remote_gdb.cc
M src/arch/x86/tlb.cc
M src/arch/x86/tlb.hh
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
13 files changed, 177 insertions(+), 217 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/7343
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1cf494468163deee000002d243541657faf57f
Gerrit-Change-Number: 7343
Gerrit-PatchSet: 3
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-20 08:05:51 UTC
Permalink
Hello Jason Lowe-Power, Alexandru Duțu, Andreas Sandberg, Brandon Potter,

I'd like you to reexamine a change. Please visit

https://gem5-review.googlesource.com/7343

to look at the new patch set (#4).

Change subject: arch, mem: Make the page table lookup function return a
pointer.
......................................................................

arch, mem: Make the page table lookup function return a pointer.

This avoids having a copy in the lookup function itself, and the
declaration of a lot of temporary TLB entry pointers in callers. The
gpu TLB seems to have had the most dependence on the original signature
of the lookup function, partially because it was relying on a somewhat
unsafe copy to a TLB entry using a base class pointer type.

Change-Id: I8b1cf494468163deee000002d243541657faf57f
---
M src/arch/alpha/faults.cc
M src/arch/arm/remote_gdb.cc
M src/arch/mips/remote_gdb.cc
M src/arch/power/remote_gdb.cc
M src/arch/riscv/remote_gdb.cc
M src/arch/sparc/faults.cc
M src/arch/sparc/remote_gdb.cc
M src/arch/x86/remote_gdb.cc
M src/arch/x86/tlb.cc
M src/arch/x86/tlb.hh
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
13 files changed, 177 insertions(+), 217 deletions(-)


--
To view, visit https://gem5-review.googlesource.com/7343
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1cf494468163deee000002d243541657faf57f
Gerrit-Change-Number: 7343
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-20 08:06:57 UTC
Permalink
Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/7343 )

Change subject: arch, mem: Make the page table lookup function return a
pointer.
......................................................................

arch, mem: Make the page table lookup function return a pointer.

This avoids having a copy in the lookup function itself, and the
declaration of a lot of temporary TLB entry pointers in callers. The
gpu TLB seems to have had the most dependence on the original signature
of the lookup function, partially because it was relying on a somewhat
unsafe copy to a TLB entry using a base class pointer type.

Change-Id: I8b1cf494468163deee000002d243541657faf57f
Reviewed-on: https://gem5-review.googlesource.com/7343
Reviewed-by: Gabe Black <***@google.com>
Maintainer: Gabe Black <***@google.com>
---
M src/arch/alpha/faults.cc
M src/arch/arm/remote_gdb.cc
M src/arch/mips/remote_gdb.cc
M src/arch/power/remote_gdb.cc
M src/arch/riscv/remote_gdb.cc
M src/arch/sparc/faults.cc
M src/arch/sparc/remote_gdb.cc
M src/arch/x86/remote_gdb.cc
M src/arch/x86/tlb.cc
M src/arch/x86/tlb.hh
M src/gpu-compute/gpu_tlb.cc
M src/mem/page_table.cc
M src/mem/page_table.hh
13 files changed, 177 insertions(+), 217 deletions(-)

Approvals:
Gabe Black: Looks good to me, approved; Looks good to me, approved



diff --git a/src/arch/alpha/faults.cc b/src/arch/alpha/faults.cc
index 4a829cd..89b0ece 100644
--- a/src/arch/alpha/faults.cc
+++ b/src/arch/alpha/faults.cc
@@ -196,14 +196,11 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(pc, entry);
- if (!success) {
- panic("Tried to execute unmapped address %#x.\n", pc);
- } else {
- VAddr vaddr(pc);
- dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), entry);
- }
+ TlbEntry *entry = p->pTable->lookup(pc);
+ panic_if(!entry, "Tried to execute unmapped address %#x.\n", pc);
+
+ VAddr vaddr(pc);
+ dynamic_cast<TLB *>(tc->getITBPtr())->insert(vaddr.page(), *entry);
}

void
@@ -215,17 +212,11 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, entry);
- }
- if (!success) {
- panic("Tried to access unmapped address %#x.\n", (Addr)vaddr);
- } else {
- dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), entry);
- }
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ if (!entry && p->fixupStackFault(vaddr))
+ entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to access unmapped address %#x.\n",
(Addr)vaddr);
+ dynamic_cast<TLB *>(tc->getDTBPtr())->insert(vaddr.page(), *entry);
}

} // namespace AlphaISA
diff --git a/src/arch/arm/remote_gdb.cc b/src/arch/arm/remote_gdb.cc
index f127341..f191937 100644
--- a/src/arch/arm/remote_gdb.cc
+++ b/src/arch/arm/remote_gdb.cc
@@ -186,12 +186,9 @@
DPRINTF(GDBAcc, "acc: %#x mapping is valid\n", va);
return true;
} else {
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space.
- if (context()->getProcessPtr()->pTable->lookup(va, entry))
- return true;
- return false;
+ // Check to make sure the first byte is mapped into the processes
+ // address space.
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/mips/remote_gdb.cc b/src/arch/mips/remote_gdb.cc
index e17f8fd..d490fa5 100644
--- a/src/arch/mips/remote_gdb.cc
+++ b/src/arch/mips/remote_gdb.cc
@@ -162,13 +162,10 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space.
- if (FullSystem)
- panic("acc not implemented for MIPS FS!");
- else
- return context()->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes
address
+ // space.
+ panic_if(FullSystem, "acc not implemented for MIPS FS!");
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/power/remote_gdb.cc b/src/arch/power/remote_gdb.cc
index 7bdd3ba..b4082e0 100644
--- a/src/arch/power/remote_gdb.cc
+++ b/src/arch/power/remote_gdb.cc
@@ -162,16 +162,13 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- //Check to make sure the first byte is mapped into the processes
address
- //space. At the time of this writing, the acc() check is used when
- //processing the MemR/MemW packets before actually asking the
translating
- //port proxy to read/writeBlob. I (bgs) am not convinced the first
byte
- //check is enough.
- if (FullSystem)
- panic("acc not implemented for POWER FS!");
- else
- return context()->getProcessPtr()->pTable->lookup(va, entry);
+ // Check to make sure the first byte is mapped into the processes
address
+ // space. At the time of this writing, the acc() check is used when
+ // processing the MemR/MemW packets before actually asking the
translating
+ // port proxy to read/writeBlob. I (bgs) am not convinced the first
byte
+ // check is enough.
+ panic_if(FullSystem, "acc not implemented for POWER FS!");
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/riscv/remote_gdb.cc b/src/arch/riscv/remote_gdb.cc
index 4f423fc..f4e606e 100644
--- a/src/arch/riscv/remote_gdb.cc
+++ b/src/arch/riscv/remote_gdb.cc
@@ -156,11 +156,8 @@
bool
RemoteGDB::acc(Addr va, size_t len)
{
- TlbEntry entry;
- if (FullSystem)
- panic("acc not implemented for RISCV FS!");
- else
- return context()->getProcessPtr()->pTable->lookup(va, entry);
+ panic_if(FullSystem, "acc not implemented for RISCV FS!");
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}

void
diff --git a/src/arch/sparc/faults.cc b/src/arch/sparc/faults.cc
index 5466115..0b6a289 100644
--- a/src/arch/sparc/faults.cc
+++ b/src/arch/sparc/faults.cc
@@ -629,49 +629,46 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- panic("Tried to execute unmapped address %#x.\n", vaddr);
- } else {
- Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to execute unmapped address %#x.\n", vaddr);

- // Grab fields used during instruction translation to figure out
- // which context to use.
- uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+ Addr alignedvaddr = p->pTable->pageAlign(vaddr);

- // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical
address,
- // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the
context is
- // zero (kernel-level) if real addressing is being used.
- bool is_real_address = !bits(tlbdata, 4);
+ // Grab fields used during instruction translation to figure out
+ // which context to use.
+ uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

- // The SPARC ITLB code assumes that traps are executed in context
- // zero so we carry that assumption through here.
- bool trapped = bits(tlbdata, 18, 16) > 0;
+ // Inside a VM, a real address is the address that guest OS would
+ // interpret to be a physical address. To map to the physical address,
+ // it still needs to undergo a translation. The instruction
+ // translation code in the SPARC ITLB code assumes that the context is
+ // zero (kernel-level) if real addressing is being used.
+ bool is_real_address = !bits(tlbdata, 4);

- // The primary context acts as a PASID. It allows the MMU to
- // distinguish between virtual addresses that would alias to the
- // same physical address (if two or more processes shared the same
- // virtual address mapping).
- int primary_context = bits(tlbdata, 47, 32);
+ // The SPARC ITLB code assumes that traps are executed in context
+ // zero so we carry that assumption through here.
+ bool trapped = bits(tlbdata, 18, 16) > 0;

- // The partition id distinguishes between virtualized environments.
- int const partition_id = 0;
+ // The primary context acts as a PASID. It allows the MMU to
+ // distinguish between virtual addresses that would alias to the
+ // same physical address (if two or more processes shared the same
+ // virtual address mapping).
+ int primary_context = bits(tlbdata, 47, 32);

- // Given the assumptions in the translateInst code in the SPARC
ITLB,
- // the logic works out to the following for the context.
- int context_id = (is_real_address || trapped) ? 0 :
primary_context;
+ // The partition id distinguishes between virtualized environments.
+ int const partition_id = 0;

- // Insert the TLB entry.
- // The entry specifying whether the address is "real" is set to
- // false for syscall emulation mode regardless of whether the
- // address is real in preceding code. Not sure sure that this is
- // correct, but also not sure if it matters at all.
- dynamic_cast<TLB *>(tc->getITBPtr())->
- insert(alignedvaddr, partition_id, context_id, false,
entry.pte);
- }
+ // Given the assumptions in the translateInst code in the SPARC ITLB,
+ // the logic works out to the following for the context.
+ int context_id = (is_real_address || trapped) ? 0 : primary_context;
+
+ // Insert the TLB entry.
+ // The entry specifying whether the address is "real" is set to
+ // false for syscall emulation mode regardless of whether the
+ // address is real in preceding code. Not sure sure that this is
+ // correct, but also not sure if it matters at all.
+ dynamic_cast<TLB *>(tc->getITBPtr())->
+ insert(alignedvaddr, partition_id, context_id, false, entry->pte);
}

void
@@ -683,83 +680,78 @@
}

Process *p = tc->getProcessPtr();
- TlbEntry entry;
- bool success = p->pTable->lookup(vaddr, entry);
- if (!success) {
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, entry);
- }
- if (!success) {
- panic("Tried to access unmapped address %#x.\n", vaddr);
- } else {
- Addr alignedvaddr = p->pTable->pageAlign(vaddr);
+ TlbEntry *entry = p->pTable->lookup(vaddr);
+ if (!entry && p->fixupStackFault(vaddr))
+ entry = p->pTable->lookup(vaddr);
+ panic_if(!entry, "Tried to access unmapped address %#x.\n", vaddr);

- // Grab fields used during data translation to figure out
- // which context to use.
- uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);
+ Addr alignedvaddr = p->pTable->pageAlign(vaddr);

- // The primary context acts as a PASID. It allows the MMU to
- // distinguish between virtual addresses that would alias to the
- // same physical address (if two or more processes shared the same
- // virtual address mapping). There's a secondary context used in
the
- // DTLB translation code, but it should __probably__ be zero for
- // syscall emulation code. (The secondary context is used by
Solaris
- // to allow kernel privilege code to access user space code:
- // [ISBN 0-13-022496-0]:PG199.)
- int primary_context = bits(tlbdata, 47, 32);
+ // Grab fields used during data translation to figure out
+ // which context to use.
+ uint64_t tlbdata = tc->readMiscRegNoEffect(MISCREG_TLB_DATA);

- // "Hyper-Privileged Mode" is in use. There are three main modes of
- // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
- // User Mode.
- int hpriv = bits(tlbdata, 0);
+ // The primary context acts as a PASID. It allows the MMU to
+ // distinguish between virtual addresses that would alias to the
+ // same physical address (if two or more processes shared the same
+ // virtual address mapping). There's a secondary context used in the
+ // DTLB translation code, but it should __probably__ be zero for
+ // syscall emulation code. (The secondary context is used by Solaris
+ // to allow kernel privilege code to access user space code:
+ // [ISBN 0-13-022496-0]:PG199.)
+ int primary_context = bits(tlbdata, 47, 32);

- // Reset, Error and Debug state is in use. Something horrible has
- // happened or the system is operating in Reset Mode.
- int red = bits(tlbdata, 1);
+ // "Hyper-Privileged Mode" is in use. There are three main modes of
+ // operation for Sparc: Hyper-Privileged Mode, Privileged Mode, and
+ // User Mode.
+ int hpriv = bits(tlbdata, 0);

- // Inside a VM, a real address is the address that guest OS would
- // interpret to be a physical address. To map to the physical
address,
- // it still needs to undergo a translation. The instruction
- // translation code in the SPARC ITLB code assumes that the
context is
- // zero (kernel-level) if real addressing is being used.
- int is_real_address = !bits(tlbdata, 5);
+ // Reset, Error and Debug state is in use. Something horrible has
+ // happened or the system is operating in Reset Mode.
+ int red = bits(tlbdata, 1);

- // Grab the address space identifier register from the thread
context.
- // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
- // MISCREG_ASI causes me to think that the ASI register
implementation
- // might be bugged. The NoEffect variant changes the ASI register
- // value in the architectural state while the normal variant
changes
- // the context field in the thread context's currently decoded
request
- // but does not directly affect the ASI register value in the
- // architectural state. The ASI values and the context field in the
- // request packet seem to have completely different uses.
- MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
- ASI asi = static_cast<ASI>(reg_asi);
+ // Inside a VM, a real address is the address that guest OS would
+ // interpret to be a physical address. To map to the physical address,
+ // it still needs to undergo a translation. The instruction
+ // translation code in the SPARC ITLB code assumes that the context is
+ // zero (kernel-level) if real addressing is being used.
+ int is_real_address = !bits(tlbdata, 5);

- // The SPARC DTLB code assumes that traps are executed in context
- // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's
also
- // an assumption that the nucleus address space is being used, but
- // the context is the relevant issue since we need to pass it to
TLB.
- bool trapped = bits(tlbdata, 18, 16) > 0;
+ // Grab the address space identifier register from the thread context.
+ // XXX: Inspecting how setMiscReg and setMiscRegNoEffect behave for
+ // MISCREG_ASI causes me to think that the ASI register implementation
+ // might be bugged. The NoEffect variant changes the ASI register
+ // value in the architectural state while the normal variant changes
+ // the context field in the thread context's currently decoded request
+ // but does not directly affect the ASI register value in the
+ // architectural state. The ASI values and the context field in the
+ // request packet seem to have completely different uses.
+ MiscReg reg_asi = tc->readMiscRegNoEffect(MISCREG_ASI);
+ ASI asi = static_cast<ASI>(reg_asi);

- // Given the assumptions in the translateData code in the SPARC
DTLB,
- // the logic works out to the following for the context.
- int context_id = ((!hpriv && !red && is_real_address) ||
- asiIsReal(asi) ||
- (trapped && asi == ASI_IMPLICIT))
- ? 0 : primary_context;
+ // The SPARC DTLB code assumes that traps are executed in context
+ // zero if the asi value is ASI_IMPLICIT (which is 0x0). There's also
+ // an assumption that the nucleus address space is being used, but
+ // the context is the relevant issue since we need to pass it to TLB.
+ bool trapped = bits(tlbdata, 18, 16) > 0;

- // The partition id distinguishes between virtualized environments.
- int const partition_id = 0;
+ // Given the assumptions in the translateData code in the SPARC DTLB,
+ // the logic works out to the following for the context.
+ int context_id = ((!hpriv && !red && is_real_address) ||
+ asiIsReal(asi) ||
+ (trapped && asi == ASI_IMPLICIT))
+ ? 0 : primary_context;

- // Insert the TLB entry.
- // The entry specifying whether the address is "real" is set to
- // false for syscall emulation mode regardless of whether the
- // address is real in preceding code. Not sure sure that this is
- // correct, but also not sure if it matters at all.
- dynamic_cast<TLB *>(tc->getDTBPtr())->
- insert(alignedvaddr, partition_id, context_id, false,
entry.pte);
- }
+ // The partition id distinguishes between virtualized environments.
+ int const partition_id = 0;
+
+ // Insert the TLB entry.
+ // The entry specifying whether the address is "real" is set to
+ // false for syscall emulation mode regardless of whether the
+ // address is real in preceding code. Not sure sure that this is
+ // correct, but also not sure if it matters at all.
+ dynamic_cast<TLB *>(tc->getDTBPtr())->
+ insert(alignedvaddr, partition_id, context_id, false, entry->pte);
}

void
diff --git a/src/arch/sparc/remote_gdb.cc b/src/arch/sparc/remote_gdb.cc
index baec0e7..057cea1 100644
--- a/src/arch/sparc/remote_gdb.cc
+++ b/src/arch/sparc/remote_gdb.cc
@@ -163,16 +163,11 @@
// from va to va + len have valid page map entries. Not
// sure how this will work for other OSes or in general.
if (FullSystem) {
- if (va)
- return true;
- return false;
+ return va != 0;
} else {
- TlbEntry entry;
// Check to make sure the first byte is mapped into the processes
// address space.
- if (context()->getProcessPtr()->pTable->lookup(va, entry))
- return true;
- return false;
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/x86/remote_gdb.cc b/src/arch/x86/remote_gdb.cc
index 175cabb..9564593 100644
--- a/src/arch/x86/remote_gdb.cc
+++ b/src/arch/x86/remote_gdb.cc
@@ -88,8 +88,7 @@
BaseTLB::Read);
return fault == NoFault;
} else {
- TlbEntry entry;
- return context()->getProcessPtr()->pTable->lookup(va, entry);
+ return context()->getProcessPtr()->pTable->lookup(va) != nullptr;
}
}

diff --git a/src/arch/x86/tlb.cc b/src/arch/x86/tlb.cc
index 0b1df9e..9336949 100644
--- a/src/arch/x86/tlb.cc
+++ b/src/arch/x86/tlb.cc
@@ -94,7 +94,7 @@
}

TlbEntry *
-TLB::insert(Addr vpn, TlbEntry &entry)
+TLB::insert(Addr vpn, const TlbEntry &entry)
{
// If somebody beat us to it, just use that existing entry.
TlbEntry *newEntry = trie.lookup(vpn);
@@ -357,23 +357,22 @@
assert(entry);
} else {
Process *p = tc->getProcessPtr();
- TlbEntry newEntry;
- bool success = p->pTable->lookup(vaddr, newEntry);
- if (!success && mode != Execute) {
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && mode != Execute) {
// Check if we just need to grow the stack.
if (p->fixupStackFault(vaddr)) {
// If we did, lookup the entry for the new
page.
- success = p->pTable->lookup(vaddr, newEntry);
+ newEntry = p->pTable->lookup(vaddr);
}
}
- if (!success) {
+ if (!newEntry) {
return std::make_shared<PageFault>(vaddr, true,
mode,
true, false);
} else {
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
DPRINTF(TLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
- entry = insert(alignedVaddr, newEntry);
+ newEntry->pageStart());
+ entry = insert(alignedVaddr, *newEntry);
}
DPRINTF(TLB, "Miss was serviced.\n");
}
diff --git a/src/arch/x86/tlb.hh b/src/arch/x86/tlb.hh
index c3dc83b..08804a4 100644
--- a/src/arch/x86/tlb.hh
+++ b/src/arch/x86/tlb.hh
@@ -144,7 +144,7 @@
Fault finalizePhysical(RequestPtr req, ThreadContext *tc,
Mode mode) const override;

- TlbEntry * insert(Addr vpn, TlbEntry &entry);
+ TlbEntry *insert(Addr vpn, const TlbEntry &entry);

/*
* Function to register Stats
diff --git a/src/gpu-compute/gpu_tlb.cc b/src/gpu-compute/gpu_tlb.cc
index b5411f8..9cbf3e8 100644
--- a/src/gpu-compute/gpu_tlb.cc
+++ b/src/gpu-compute/gpu_tlb.cc
@@ -808,31 +808,31 @@
"at pc %#x.\n", vaddr, tc->instAddr());

Process *p = tc->getProcessPtr();
- GpuTlbEntry newEntry;
- bool success = p->pTable->lookup(vaddr, newEntry);
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);

- if (!success && mode != BaseTLB::Execute) {
+ if (!newEntry && mode != BaseTLB::Execute) {
// penalize a "page fault" more
- if (timing) {
+ if (timing)
latency += missLatency2;
- }

if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr,
newEntry);
+ newEntry = p->pTable->lookup(vaddr);
}

- if (!success) {
+ if (!newEntry) {
return std::make_shared<PageFault>(vaddr, true,
mode, true,
false);
} else {
- newEntry.valid = success;
Addr alignedVaddr =
p->pTable->pageAlign(vaddr);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
- alignedVaddr, newEntry.pageStart());
+ alignedVaddr, newEntry->pageStart());

- entry = insert(alignedVaddr, newEntry);
+ GpuTlbEntry gpuEntry;
+ *(TlbEntry *)&gpuEntry = *newEntry;
+ gpuEntry.valid = true;
+ entry = insert(alignedVaddr, gpuEntry);
}

DPRINTF(GPUTLB, "Miss was serviced.\n");
@@ -1330,25 +1330,27 @@
safe_cast<TranslationState*>(pkt->senderState);

Process *p = sender_state->tc->getProcessPtr();
- TlbEntry newEntry;
Addr vaddr = pkt->req->getVaddr();
#ifndef NDEBUG
Addr alignedVaddr = p->pTable->pageAlign(vaddr);
assert(alignedVaddr == virtPageAddr);
#endif
- bool success;
- success = p->pTable->lookup(vaddr, newEntry);
- if (!success && sender_state->tlbMode != BaseTLB::Execute) {
- if (p->fixupStackFault(vaddr)) {
- success = p->pTable->lookup(vaddr, newEntry);
- }
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && sender_state->tlbMode != BaseTLB::Execute &&
+ p->fixupStackFault(vaddr)) {
+ newEntry = p->pTable->lookup(vaddr);
}

- DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
+ if (newEntry) {
+ DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
+ newEntry->pageStart());

- sender_state->tlbEntry =
- new GpuTlbEntry(0, newEntry.vaddr, newEntry.paddr,
success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr, newEntry->paddr,
true);
+ } else {
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, 0, 0, false);
+ }

handleTranslationReturn(virtPageAddr, TLB_MISS, pkt);
} else if (outcome == MISS_RETURN) {
@@ -1524,7 +1526,6 @@
virt_page_addr);

Process *p = tc->getProcessPtr();
- TlbEntry newEntry;

Addr vaddr = pkt->req->getVaddr();
#ifndef NDEBUG
@@ -1532,10 +1533,10 @@
assert(alignedVaddr == virt_page_addr);
#endif

- bool success = p->pTable->lookup(vaddr, newEntry);
- if (!success && sender_state->tlbMode != BaseTLB::Execute)
{
- if (p->fixupStackFault(vaddr))
- success = p->pTable->lookup(vaddr, newEntry);
+ TlbEntry *newEntry = p->pTable->lookup(vaddr);
+ if (!newEntry && sender_state->tlbMode != BaseTLB::Execute
&&
+ p->fixupStackFault(vaddr)) {
+ newEntry = p->pTable->lookup(vaddr);
}

if (!sender_state->prefetch) {
@@ -1544,24 +1545,23 @@
assert(success);

DPRINTF(GPUTLB, "Mapping %#x to %#x\n", alignedVaddr,
- newEntry.pageStart());
+ newEntry->pageStart());

- sender_state->tlbEntry = new GpuTlbEntry(0,
newEntry.vaddr,
-
newEntry.paddr,
- success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr,
+ newEntry->paddr, success);
} else {
// If this was a prefetch, then do the normal thing if
it
// was a successful translation. Otherwise, send an
empty
// TLB entry back so that it can be figured out as
empty and
// handled accordingly.
- if (success) {
+ if (newEntry) {
DPRINTF(GPUTLB, "Mapping %#x to %#x\n",
alignedVaddr,
- newEntry.pageStart());
+ newEntry->pageStart());

- sender_state->tlbEntry = new GpuTlbEntry(0,
-
newEntry.vaddr,
-
newEntry.paddr,
- success);
+ sender_state->tlbEntry =
+ new GpuTlbEntry(0, newEntry->vaddr,
+ newEntry->paddr, success);
} else {
DPRINTF(GPUPrefetch, "Prefetch failed %#x\n",
alignedVaddr);
diff --git a/src/mem/page_table.cc b/src/mem/page_table.cc
index 3854a8c..26cc6e5 100644
--- a/src/mem/page_table.cc
+++ b/src/mem/page_table.cc
@@ -146,29 +146,25 @@
return true;
}

-bool
-EmulationPageTable::lookup(Addr vaddr, TheISA::TlbEntry &entry)
+TheISA::TlbEntry *
+EmulationPageTable::lookup(Addr vaddr)
{
Addr page_addr = pageAlign(vaddr);
-
PTableItr iter = pTable.find(page_addr);
-
if (iter == pTable.end())
- return false;
-
- entry = *iter->second;
- return true;
+ return nullptr;
+ return iter->second;
}

bool
EmulationPageTable::translate(Addr vaddr, Addr &paddr)
{
- TheISA::TlbEntry entry;
- if (!lookup(vaddr, entry)) {
+ TheISA::TlbEntry *entry = lookup(vaddr);
+ if (!entry) {
DPRINTF(MMU, "Couldn't Translate: %#x\n", vaddr);
return false;
}
- paddr = pageOffset(vaddr) + entry.pageStart();
+ paddr = pageOffset(vaddr) + entry->pageStart();
DPRINTF(MMU, "Translating: %#x->%#x\n", vaddr, paddr);
return true;
}
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 30437a6..470a3e7 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -121,9 +121,9 @@
/**
* Lookup function
* @param vaddr The virtual address.
- * @return entry The page table entry corresponding to vaddr.
+ * @return The page table entry corresponding to vaddr.
*/
- virtual bool lookup(Addr vaddr, TheISA::TlbEntry &entry);
+ virtual TheISA::TlbEntry *lookup(Addr vaddr);

/**
* Translate function

--
To view, visit https://gem5-review.googlesource.com/7343
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b1cf494468163deee000002d243541657faf57f
Gerrit-Change-Number: 7343
Gerrit-PatchSet: 5
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Gabe Black (Gerrit)
2018-01-11 06:29:59 UTC
Permalink
Gabe Black has submitted this change and it was merged. (
https://gem5-review.googlesource.com/6981 )

Change subject: arch,mem: Move page table construction into the arch
classes.
......................................................................

arch,mem: Move page table construction into the arch classes.

This gets rid of an awkward NoArchPageTable class, and also gives the
arch a place to inject ISA specific parameters (specifically page size)
without having to have TheISA:: in the generic version of these types.

Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
Reviewed-on: https://gem5-review.googlesource.com/6981
Reviewed-by: Alexandru Duțu <***@amd.com>
Maintainer: Gabe Black <***@google.com>
---
M src/arch/alpha/process.cc
M src/arch/alpha/process.hh
M src/arch/arm/process.cc
M src/arch/arm/process.hh
M src/arch/mips/process.cc
M src/arch/mips/process.hh
M src/arch/power/process.cc
M src/arch/power/process.hh
M src/arch/riscv/process.cc
M src/arch/riscv/process.hh
M src/arch/sparc/process.cc
M src/arch/sparc/process.hh
M src/arch/x86/process.cc
M src/arch/x86/process.hh
M src/mem/page_table.hh
M src/sim/process.cc
M src/sim/process.hh
17 files changed, 45 insertions(+), 61 deletions(-)

Approvals:
Alexandru Duțu: Looks good to me, approved
Gabe Black: Looks good to me, approved



diff --git a/src/arch/alpha/process.cc b/src/arch/alpha/process.cc
index 58fe0bd..3cc0b0d 100644
--- a/src/arch/alpha/process.cc
+++ b/src/arch/alpha/process.cc
@@ -38,6 +38,7 @@
#include "cpu/thread_context.hh"
#include "debug/Loader.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/byteswap.hh"
#include "sim/process_impl.hh"
@@ -48,8 +49,9 @@
using namespace std;

AlphaProcess::AlphaProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
objFile->bssSize();
brk_point = roundUp(brk_point, PageBytes);
diff --git a/src/arch/alpha/process.hh b/src/arch/alpha/process.hh
index a02b8ce..28ecd68 100644
--- a/src/arch/alpha/process.hh
+++ b/src/arch/alpha/process.hh
@@ -61,7 +61,4 @@
virtual bool mmapGrowsDown() const override { return false; }
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __ARCH_ALPHA_PROCESS_HH__
diff --git a/src/arch/arm/process.cc b/src/arch/arm/process.cc
index dcc9145..b64579a 100644
--- a/src/arch/arm/process.cc
+++ b/src/arch/arm/process.cc
@@ -51,6 +51,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/byteswap.hh"
#include "sim/process_impl.hh"
@@ -62,8 +63,10 @@

ArmProcess::ArmProcess(ProcessParams *params, ObjectFile *objFile,
ObjectFile::Arch _arch)
- : Process(params, objFile), arch(_arch)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ arch(_arch)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
}

ArmProcess32::ArmProcess32(ProcessParams *params, ObjectFile *objFile,
diff --git a/src/arch/arm/process.hh b/src/arch/arm/process.hh
index f8e6542..f4f0874 100644
--- a/src/arch/arm/process.hh
+++ b/src/arch/arm/process.hh
@@ -95,8 +95,5 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __ARM_PROCESS_HH__

diff --git a/src/arch/mips/process.cc b/src/arch/mips/process.cc
index 4d0d5e3..f3b1108 100644
--- a/src/arch/mips/process.cc
+++ b/src/arch/mips/process.cc
@@ -39,6 +39,7 @@
#include "cpu/thread_context.hh"
#include "debug/Loader.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process.hh"
#include "sim/process_impl.hh"
@@ -48,9 +49,10 @@
using namespace std;
using namespace MipsISA;

-MipsProcess::MipsProcess(ProcessParams * params, ObjectFile *objFile)
- : Process(params, objFile)
+MipsProcess::MipsProcess(ProcessParams *params, ObjectFile *objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Set up stack. On MIPS, stack starts at the top of kuseg
// user address space. MIPS stack grows down from here
Addr stack_base = 0x7FFFFFFF;
diff --git a/src/arch/mips/process.hh b/src/arch/mips/process.hh
index ed6561c..e9e0585 100644
--- a/src/arch/mips/process.hh
+++ b/src/arch/mips/process.hh
@@ -58,8 +58,4 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
-
#endif // __MIPS_PROCESS_HH__
diff --git a/src/arch/power/process.cc b/src/arch/power/process.cc
index 4a34dec..87e5bac 100644
--- a/src/arch/power/process.cc
+++ b/src/arch/power/process.cc
@@ -40,6 +40,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_return.hh"
@@ -49,8 +50,9 @@
using namespace PowerISA;

PowerProcess::PowerProcess(ProcessParams *params, ObjectFile *objFile)
- : Process(params, objFile)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Set up break point (Top of Heap)
Addr brk_point = objFile->dataBase() + objFile->dataSize() +
objFile->bssSize();
diff --git a/src/arch/power/process.hh b/src/arch/power/process.hh
index 08efdfe..348e375 100644
--- a/src/arch/power/process.hh
+++ b/src/arch/power/process.hh
@@ -57,8 +57,5 @@
void setSyscallReturn(ThreadContext *tc, SyscallReturn return_value);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __POWER_PROCESS_HH__

diff --git a/src/arch/riscv/process.cc b/src/arch/riscv/process.cc
index b4fe1ee..88a093a 100644
--- a/src/arch/riscv/process.cc
+++ b/src/arch/riscv/process.cc
@@ -59,9 +59,10 @@
using namespace std;
using namespace RiscvISA;

-RiscvProcess::RiscvProcess(ProcessParams * params,
- ObjectFile *objFile) : Process(params, objFile)
+RiscvProcess::RiscvProcess(ProcessParams *params, ObjectFile *objFile) :
+ Process(params, new FuncPageTable(params->name, params->pid),
objFile)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
const Addr stack_base = 0x7FFFFFFFFFFFFFFFL;
const Addr max_stack_size = 8 * 1024 * 1024;
const Addr next_thread_stack_base = stack_base - max_stack_size;
diff --git a/src/arch/riscv/process.hh b/src/arch/riscv/process.hh
index 2a27f35..bda278e 100644
--- a/src/arch/riscv/process.hh
+++ b/src/arch/riscv/process.hh
@@ -65,8 +65,4 @@
virtual bool mmapGrowsDown() const override { return false; }
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
-
#endif // __RISCV_PROCESS_HH__
diff --git a/src/arch/sparc/process.cc b/src/arch/sparc/process.cc
index da7032f..fe91589 100644
--- a/src/arch/sparc/process.cc
+++ b/src/arch/sparc/process.cc
@@ -42,6 +42,7 @@
#include "cpu/thread_context.hh"
#include "debug/Stack.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_return.hh"
@@ -53,10 +54,12 @@
static const int FirstArgumentReg = 8;


-SparcProcess::SparcProcess(ProcessParams * params, ObjectFile *objFile,
+SparcProcess::SparcProcess(ProcessParams *params, ObjectFile *objFile,
Addr _StackBias)
- : Process(params, objFile), StackBias(_StackBias)
+ : Process(params, new FuncPageTable(params->name, params->pid),
objFile),
+ StackBias(_StackBias)
{
+ fatal_if(!params->useArchPT, "Arch page tables not implemented.");
// Initialize these to 0s
fillStart = 0;
spillStart = 0;
diff --git a/src/arch/sparc/process.hh b/src/arch/sparc/process.hh
index 6a203a4..d7e0967 100644
--- a/src/arch/sparc/process.hh
+++ b/src/arch/sparc/process.hh
@@ -160,7 +160,4 @@
void setSyscallArg(ThreadContext *tc, int i, SparcISA::IntReg val);
};

-/* No architectural page table defined for this ISA */
-typedef NoArchPageTable ArchPageTable;
-
#endif // __SPARC_PROCESS_HH__
diff --git a/src/arch/x86/process.cc b/src/arch/x86/process.cc
index 1ad1315..f11cc34 100644
--- a/src/arch/x86/process.cc
+++ b/src/arch/x86/process.cc
@@ -60,6 +60,7 @@
#include "debug/Stack.hh"
#include "mem/multi_level_page_table.hh"
#include "mem/page_table.hh"
+#include "params/Process.hh"
#include "sim/aux_vector.hh"
#include "sim/process_impl.hh"
#include "sim/syscall_desc.hh"
@@ -95,10 +96,16 @@
static const int NumArgumentRegs32 M5_VAR_USED =
sizeof(ArgumentReg) / sizeof(const int);

-X86Process::X86Process(ProcessParams * params, ObjectFile *objFile,
+X86Process::X86Process(ProcessParams *params, ObjectFile *objFile,
SyscallDesc *_syscallDescs, int _numSyscallDescs)
- : Process(params, objFile), syscallDescs(_syscallDescs),
- numSyscallDescs(_numSyscallDescs)
+ : Process(params, params->useArchPT ?
+ static_cast<PageTableBase *>(
+ new ArchPageTable(params->name, params->pid,
+ params->system)) :
+ static_cast<PageTableBase *>(
+ new FuncPageTable(params->name,
params->pid)),
+ objFile),
+ syscallDescs(_syscallDescs), numSyscallDescs(_numSyscallDescs)
{
}

diff --git a/src/arch/x86/process.hh b/src/arch/x86/process.hh
index 3eb9620..e5e1857 100644
--- a/src/arch/x86/process.hh
+++ b/src/arch/x86/process.hh
@@ -59,6 +59,14 @@
class X86Process : public Process
{
protected:
+ /**
+ * Declaration of architectural page table for x86.
+ *
+ * These page tables are stored in system memory and respect x86
+ * specification.
+ */
+ typedef MultiLevelPageTable<PageTableOps> ArchPageTable;
+
Addr _gdtStart;
Addr _gdtSize;

@@ -189,14 +197,6 @@
Process *process, TheISA::IntReg flags) override;
};

- /**
- * Declaration of architectural page table for x86.
- *
- * These page tables are stored in system memory and respect x86
- * specification.
- */
- typedef MultiLevelPageTable<PageTableOps> ArchPageTable;
-
}

#endif // __ARCH_X86_PROCESS_HH__
diff --git a/src/mem/page_table.hh b/src/mem/page_table.hh
index 0d0a75e..883b47c 100644
--- a/src/mem/page_table.hh
+++ b/src/mem/page_table.hh
@@ -246,19 +246,4 @@
void getMappings(std::vector<std::pair<Addr, Addr>> *addr_maps)
override;
};

-/**
- * Faux page table class indended to stop the usage of
- * an architectural page table, when there is none defined
- * for a particular ISA.
- */
-class NoArchPageTable : public FuncPageTable
-{
- public:
- NoArchPageTable(const std::string &__name, uint64_t _pid, System *_sys,
- Addr _pageSize = TheISA::PageBytes) : FuncPageTable(__name,
_pid)
- {
- fatal("No architectural page table defined for this ISA.\n");
- }
-};
-
#endif // __MEM_PAGE_TABLE_HH__
diff --git a/src/sim/process.cc b/src/sim/process.cc
index ee90667..77d7903 100644
--- a/src/sim/process.cc
+++ b/src/sim/process.cc
@@ -101,14 +101,12 @@
using namespace std;
using namespace TheISA;

-Process::Process(ProcessParams * params, ObjectFile * obj_file)
+Process::Process(ProcessParams *params, PageTableBase *pTable,
+ ObjectFile *obj_file)
: SimObject(params), system(params->system),
useArchPT(params->useArchPT),
kvmInSE(params->kvmInSE),
- pTable(useArchPT ?
- static_cast<PageTableBase *>(new ArchPageTable(name(), params->pid,
- system)) :
- static_cast<PageTableBase *>(new FuncPageTable(name(),
params->pid))),
+ pTable(pTable),
initVirtMem(system->getSystemPort(), this,
SETranslatingPortProxy::Always),
objFile(obj_file),
diff --git a/src/sim/process.hh b/src/sim/process.hh
index e4a52e3..6d465ac 100644
--- a/src/sim/process.hh
+++ b/src/sim/process.hh
@@ -63,7 +63,8 @@
class Process : public SimObject
{
public:
- Process(ProcessParams *params, ObjectFile *obj_file);
+ Process(ProcessParams *params, PageTableBase *pTable,
+ ObjectFile *obj_file);

void serialize(CheckpointOut &cp) const override;
void unserialize(CheckpointIn &cp) override;

--
To view, visit https://gem5-review.googlesource.com/6981
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1412f303460d5c43dafdb9b3cd07af81c908a441
Gerrit-Change-Number: 6981
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <***@google.com>
Gerrit-Reviewer: Alexandru Duțu <***@amd.com>
Gerrit-Reviewer: Andreas Sandberg <***@arm.com>
Gerrit-Reviewer: Brandon Potter <***@amd.com>
Gerrit-Reviewer: Gabe Black <***@google.com>
Gerrit-Reviewer: Jason Lowe-Power <***@lowepower.com>
Continue reading on narkive:
Loading...