[gem5-dev] [S] Change in gem5/gem5[develop]: arch-arm: Add UNSERIALIZE flag to address cpt compatibility
Attention is currently required from: Richard Cooper.
Hello Richard Cooper,
I'd like you to do a code review.
Please visit
https://gem5-review.googlesource.com/c/public/gem5/+/70557?usp=email
to review the following change.
Change subject: arch-arm: Add UNSERIALIZE flag to address cpt compatibility
..
arch-arm: Add UNSERIALIZE flag to address cpt compatibility
This patch is adding the MISCREG_UNSERIALIZE flag to expose
the user to the following checkpoint compatibility problem:
What happens when a checkpoint is restored with a different
architectural configuration?
The current behaviour is to silently restore the checkpoint
and to populate the ISA registers accordingly. However some of
these restored values will be used and some of them will
be actually discarded.
For example the value of the MISCREG_ID_AA64ISAR0_EL1 register
(initially configured at construction time [1]) will be overwritten by
the checkpointed value in ISA::unserialize (checkpointed params win over
current params). On the other hand we "discard" the checkpointed value
for registers handled in the ISA::readMiscReg method (not accessing the
storage) like MISCREG_ID_AA64PFR0_EL1 [2] (current params win over
checkpointed params).
In other words some registers will be unserialized while some others
will discard the checkpointed value in favour of the current
configuration setup. This categorization is currently implicit and it
ultimately depends on whether or not a register read access its storage
(see MISCREG_ID_AA64PFR0_EL1 above).
With this patch we formalize this distinction. We allow the developer to
be explict on which register should not be unserialized and should
instead use the new simulation parameters.
If there is a mismatch between the reset value of such register and
the checkpointed one, we warn the user and we undo the unserialization
for such register.
[1]: https://github.com/gem5/gem5/blob/v22.1.0.0/src/arch/arm/isa.cc#L437
[2]: https://github.com/gem5/gem5/blob/v22.1.0.0/src/arch/arm/isa.cc#L1019
Change-Id: Icea6563ee5816b14a097926b5734f2fce10530c7
Signed-off-by: Giacomo Travaglini
Reviewed-by: Richard Cooper
---
M src/arch/arm/isa.cc
M src/arch/arm/regs/misc.hh
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index ffd9cfc..f55235d 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -1879,6 +1879,18 @@
{
DPRINTF(Checkpoint, "Unserializing Arm Misc Registers\n");
UNSERIALIZE_MAPPING(miscRegs, miscRegName, NUM_PHYS_MISCREGS);
+
+for (auto idx = 0; idx < NUM_MISCREGS; idx++) {
+if (!lookUpMiscReg[idx].info[MISCREG_UNSERIALIZE] &&
+miscRegs[idx] != lookUpMiscReg[idx].reset()) {
+warn("Checkpoint value for register %s does not match "
+ "current configuration (checkpointed: %#x, current: %#x)",
+ miscRegName[idx], miscRegs[idx],
+ lookUpMiscReg[idx].reset());
+miscRegs[idx] = lookUpMiscReg[idx].reset();
+}
+}
+
CPSR tmp_cpsr = miscRegs[MISCREG_CPSR];
updateRegMap(tmp_cpsr);
}
diff --git a/src/arch/arm/regs/misc.hh b/src/arch/arm/regs/misc.hh
index 265a697..3a32623 100644
--- a/src/arch/arm/regs/misc.hh
+++ b/src/arch/arm/regs/misc.hh
@@ -1125,6 +1125,7 @@
MISCREG_IMPLEMENTED,
MISCREG_UNVERIFIABLE, // Does the value change on every read
(e.g. a
// arch generic counter)
+MISCREG_UNSERIALIZE,// Should the checkpointed value be
restored?
MISCREG_WARN_NOT_FAIL, // If MISCREG_IMPLEMENTED is deasserted, it
// tells whether the instruction should
raise a
// warning or fail
@@ -1277,6 +1278,12 @@
return *this;
}
chain
+unserialize(bool v = true) const
+{
+entry.info[MISCREG_UNSERIALIZE] = v;
+return *this;
+}
+chain
warnNotFail(bool v = true) const
{
entry.info[MISCREG_WARN_NOT_FAIL] = v;
@@ -1595,7 +1602,7 @@
: entry(e)
{
// force unimplemented registers to be thusly declared
-implemented(1);
+implemented(1).unserialize(1);
}
};
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/70557?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Icea6563ee5816b14a097926b5734f2fce10530c7
Gerrit-Change-Number: 70557
Gerrit-PatchSet: 1
Gerrit-Owner: Giacomo Travaglini
Gerrit-Reviewer: Richard Cooper
Gerrit-Attention: Richard Cooper
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe s
[gem5-dev] [S] Change in gem5/gem5[develop]: arch-arm: Add UNSERIALIZE flag to address cpt compatibility
Giacomo Travaglini has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/70557?usp=email )
(
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the
submitted one.
)Change subject: arch-arm: Add UNSERIALIZE flag to address cpt
compatibility
..
arch-arm: Add UNSERIALIZE flag to address cpt compatibility
This patch is adding the MISCREG_UNSERIALIZE flag to expose
the user to the following checkpoint compatibility problem:
What happens when a checkpoint is restored with a different
architectural configuration?
The current behaviour is to silently restore the checkpoint
and to populate the ISA registers accordingly. However some of
these restored values will be used and some of them will
be actually discarded.
For example the value of the MISCREG_ID_AA64ISAR0_EL1 register
(initially configured at construction time [1]) will be overwritten by
the checkpointed value in ISA::unserialize (checkpointed params win over
current params). On the other hand we "discard" the checkpointed value
for registers handled in the ISA::readMiscReg method (not accessing the
storage) like MISCREG_ID_AA64PFR0_EL1 [2] (current params win over
checkpointed params).
In other words some registers will be unserialized while some others
will discard the checkpointed value in favour of the current
configuration setup. This categorization is currently implicit and it
ultimately depends on whether or not a register read access its storage
(see MISCREG_ID_AA64PFR0_EL1 above).
With this patch we formalize this distinction. We allow the developer to
be explict on which register should not be unserialized and should
instead use the new simulation parameters.
If there is a mismatch between the reset value of such register and
the checkpointed one, we warn the user and we undo the unserialization
for such register.
[1]: https://github.com/gem5/gem5/blob/v22.1.0.0/src/arch/arm/isa.cc#L437
[2]: https://github.com/gem5/gem5/blob/v22.1.0.0/src/arch/arm/isa.cc#L1019
Change-Id: Icea6563ee5816b14a097926b5734f2fce10530c7
Signed-off-by: Giacomo Travaglini
Reviewed-by: Richard Cooper
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/70557
Maintainer: Jason Lowe-Power
Tested-by: kokoro
---
M src/arch/arm/isa.cc
M src/arch/arm/regs/misc.hh
2 files changed, 20 insertions(+), 1 deletion(-)
Approvals:
Jason Lowe-Power: Looks good to me, approved
Richard Cooper: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/arch/arm/isa.cc b/src/arch/arm/isa.cc
index ffd9cfc..f55235d 100644
--- a/src/arch/arm/isa.cc
+++ b/src/arch/arm/isa.cc
@@ -1879,6 +1879,18 @@
{
DPRINTF(Checkpoint, "Unserializing Arm Misc Registers\n");
UNSERIALIZE_MAPPING(miscRegs, miscRegName, NUM_PHYS_MISCREGS);
+
+for (auto idx = 0; idx < NUM_MISCREGS; idx++) {
+if (!lookUpMiscReg[idx].info[MISCREG_UNSERIALIZE] &&
+miscRegs[idx] != lookUpMiscReg[idx].reset()) {
+warn("Checkpoint value for register %s does not match "
+ "current configuration (checkpointed: %#x, current: %#x)",
+ miscRegName[idx], miscRegs[idx],
+ lookUpMiscReg[idx].reset());
+miscRegs[idx] = lookUpMiscReg[idx].reset();
+}
+}
+
CPSR tmp_cpsr = miscRegs[MISCREG_CPSR];
updateRegMap(tmp_cpsr);
}
diff --git a/src/arch/arm/regs/misc.hh b/src/arch/arm/regs/misc.hh
index 265a697..3a32623 100644
--- a/src/arch/arm/regs/misc.hh
+++ b/src/arch/arm/regs/misc.hh
@@ -1125,6 +1125,7 @@
MISCREG_IMPLEMENTED,
MISCREG_UNVERIFIABLE, // Does the value change on every read
(e.g. a
// arch generic counter)
+MISCREG_UNSERIALIZE,// Should the checkpointed value be
restored?
MISCREG_WARN_NOT_FAIL, // If MISCREG_IMPLEMENTED is deasserted, it
// tells whether the instruction should
raise a
// warning or fail
@@ -1277,6 +1278,12 @@
return *this;
}
chain
+unserialize(bool v = true) const
+{
+entry.info[MISCREG_UNSERIALIZE] = v;
+return *this;
+}
+chain
warnNotFail(bool v = true) const
{
entry.info[MISCREG_WARN_NOT_FAIL] = v;
@@ -1595,7 +1602,7 @@
: entry(e)
{
// force unimplemented registers to be thusly declared
-implemented(1);
+implemented(1).unserialize(1);
}
};
--
To view, visit
https://gem5-review.googlesource.com/c/public/gem5/+/70557?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Icea6563ee5816b14a097926b5734
