[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