Hello Andreas Sandberg,
I'd like you to do a code review. Please visit
https://gem5-review.googlesource.com/11196
to review the following change.
Change subject: arch-arm: BadMode checking if corresponding EL is
implemented
......................................................................
arch-arm: BadMode checking if corresponding EL is implemented
The old utility function called badMode was only checking if the mode
passed as an argument was a recognized mode. It was not checking if the
corresponding mode/EL was implemented. That function has been renamed to
unknownMode and a new badMode has been introduced. This is used by the
cpsrWriteByInstruction function. In this way any try to change the
execution mode won't succeed if the mode hasn't been implemented.
Change-Id: Ibfe385c5465b904acc0d2eb9647710891d72c9df
Signed-off-by: Giacomo Travaglini <giacomo.travagl...@arm.com>
Reviewed-by: Andreas Sandberg <andreas.sandb...@arm.com>
---
M src/arch/arm/insts/static_inst.cc
M src/arch/arm/insts/static_inst.hh
M src/arch/arm/isa/formats/mem.isa
M src/arch/arm/isa/formats/uncond.isa
M src/arch/arm/isa/insts/str.isa
M src/arch/arm/types.hh
M src/arch/arm/utility.cc
M src/arch/arm/utility.hh
8 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/src/arch/arm/insts/static_inst.cc
b/src/arch/arm/insts/static_inst.cc
index 40a1fe4..b7f235e 100644
--- a/src/arch/arm/insts/static_inst.cc
+++ b/src/arch/arm/insts/static_inst.cc
@@ -961,7 +961,7 @@
illegalExceptionReturn(ThreadContext *tc, CPSR cpsr, CPSR spsr)
{
const OperatingMode mode = (OperatingMode) (uint8_t)spsr.mode;
- if (badMode(mode))
+ if (unknownMode(mode))
return true;
const OperatingMode cur_mode = (OperatingMode) (uint8_t)cpsr.mode;
@@ -1000,7 +1000,7 @@
return true;
} else {
// aarch32
- return badMode32(mode);
+ return unknownMode32(mode);
}
return false;
@@ -1029,7 +1029,7 @@
}
} else {
new_cpsr.il = spsr.il;
- if (spsr.width && badMode32((OperatingMode)(uint8_t)spsr.mode)) {
+ if (spsr.width &&
unknownMode32((OperatingMode)(uint8_t)spsr.mode)) {
new_cpsr.il = 1;
} else if (spsr.width) {
new_cpsr.mode = spsr.mode;
diff --git a/src/arch/arm/insts/static_inst.hh
b/src/arch/arm/insts/static_inst.hh
index 69ae58e..873dfff 100644
--- a/src/arch/arm/insts/static_inst.hh
+++ b/src/arch/arm/insts/static_inst.hh
@@ -230,7 +230,7 @@
// Now check the new mode is allowed
OperatingMode newMode = (OperatingMode) (val & mask(5));
OperatingMode oldMode = (OperatingMode)(uint32_t)cpsr.mode;
- if (!badMode(newMode)) {
+ if (!badMode(tc, newMode)) {
bool validModeChange = true;
// Check for attempts to enter modes only permitted in
// Secure state from Non-secure state. These are
Monitor
diff --git a/src/arch/arm/isa/formats/mem.isa
b/src/arch/arm/isa/formats/mem.isa
index 1a30fbb..50e3e35 100644
--- a/src/arch/arm/isa/formats/mem.isa
+++ b/src/arch/arm/isa/formats/mem.isa
@@ -282,7 +282,12 @@
}
} else {
const uint32_t mode = bits(machInst, 4, 0);
- if (badMode32((OperatingMode)mode))
+ // We check at decode stage if the mode exists even
+ // if the checking is re-done by Srs::execute.
+ // This is done because we will otherwise panic if
+ // trying to read the banked stack pointer of an
+ // unrecognized mode.
+ if (unknownMode32((OperatingMode)mode))
return new Unknown(machInst);
if (!add && !wb) {
return new %(srs)s(machInst, mode,
diff --git a/src/arch/arm/isa/formats/uncond.isa
b/src/arch/arm/isa/formats/uncond.isa
index c376cd9..e0b07ab 100644
--- a/src/arch/arm/isa/formats/uncond.isa
+++ b/src/arch/arm/isa/formats/uncond.isa
@@ -166,7 +166,12 @@
const uint32_t val = ((machInst >> 20) & 0x5);
if (val == 0x4) {
const uint32_t mode = bits(machInst, 4, 0);
- if (badMode32((OperatingMode)mode))
+ // We check at decode stage if the mode exists even
+ // if the checking is re-done by Srs::execute.
+ // This is done because we will otherwise panic if
+ // trying to read the banked stack pointer of an
+ // unrecognized mode.
+ if (unknownMode32((OperatingMode)mode))
return new Unknown(machInst);
switch (bits(machInst, 24, 21)) {
case 0x2:
diff --git a/src/arch/arm/isa/insts/str.isa b/src/arch/arm/isa/insts/str.isa
index 1c697d3..c165eaf 100644
--- a/src/arch/arm/isa/insts/str.isa
+++ b/src/arch/arm/isa/insts/str.isa
@@ -112,6 +112,12 @@
if self.add:
wbDiff = 8
accCode = '''
+
+ auto tc = xc->tcBase();
+ if (badMode32(tc, static_cast<OperatingMode>(regMode))) {
+ return undefinedFault32(tc, opModeToEL(currOpMode(tc)));
+ }
+
CPSR cpsr = Cpsr;
Mem_ud = (uint64_t)cSwap(LR_uw, cpsr.e) |
((uint64_t)cSwap(Spsr_uw, cpsr.e) << 32);
diff --git a/src/arch/arm/types.hh b/src/arch/arm/types.hh
index 07cdfad..9ce0252 100644
--- a/src/arch/arm/types.hh
+++ b/src/arch/arm/types.hh
@@ -710,7 +710,7 @@
}
static inline bool
- badMode(OperatingMode mode)
+ unknownMode(OperatingMode mode)
{
switch (mode) {
case MODE_EL0T:
@@ -735,9 +735,8 @@
}
}
-
static inline bool
- badMode32(OperatingMode mode)
+ unknownMode32(OperatingMode mode)
{
switch (mode) {
case MODE_USER:
diff --git a/src/arch/arm/utility.cc b/src/arch/arm/utility.cc
index 7659e1e..dec85ef 100644
--- a/src/arch/arm/utility.cc
+++ b/src/arch/arm/utility.cc
@@ -313,6 +313,18 @@
}
}
+bool
+badMode32(ThreadContext *tc, OperatingMode mode)
+{
+ return unknownMode32(mode) || !ArmSystem::haveEL(tc, opModeToEL(mode));
+}
+
+bool
+badMode(ThreadContext *tc, OperatingMode mode)
+{
+ return unknownMode(mode) || !ArmSystem::haveEL(tc, opModeToEL(mode));
+}
+
Addr
purifyTaggedAddr(Addr addr, ThreadContext *tc, ExceptionLevel el,
TTBCR tcr)
diff --git a/src/arch/arm/utility.hh b/src/arch/arm/utility.hh
index 796ded7..9d0131b 100644
--- a/src/arch/arm/utility.hh
+++ b/src/arch/arm/utility.hh
@@ -181,6 +181,26 @@
bool isBigEndian64(ThreadContext *tc);
+/**
+ * badMode is checking if the execution mode provided as an argument is
+ * valid and implemented for AArch32
+ *
+ * @param tc ThreadContext
+ * @param mode OperatingMode to check
+ * @return false if mode is valid and implemented, true otherwise
+ */
+bool badMode32(ThreadContext *tc, OperatingMode mode);
+
+/**
+ * badMode is checking if the execution mode provided as an argument is
+ * valid and implemented.
+ *
+ * @param tc ThreadContext
+ * @param mode OperatingMode to check
+ * @return false if mode is valid and implemented, true otherwise
+ */
+bool badMode(ThreadContext *tc, OperatingMode mode);
+
static inline uint8_t
itState(CPSR psr)
{
--
To view, visit https://gem5-review.googlesource.com/11196
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: master
Gerrit-Change-Id: Ibfe385c5465b904acc0d2eb9647710891d72c9df
Gerrit-Change-Number: 11196
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini <giacomo.travagl...@arm.com>
Gerrit-Reviewer: Andreas Sandberg <andreas.sandb...@arm.com>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
