gem5-dev@gem5.org

The gem5 Developer List

View all threads

[S] Change in gem5/gem5[develop]: arch-arm: Add UNSERIALIZE flag to address cpt compatibility

GT
Giacomo Travaglini (Gerrit)
Fri, May 12, 2023 8:45 AM

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.

Change-Id: Icea6563ee5816b14a097926b5734f2fce10530c7
Signed-off-by: Giacomo Travaglini giacomo.travaglini@arm.com
Reviewed-by: Richard Cooper richard.cooper@arm.com

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 giacomo.travaglini@arm.com
Gerrit-Reviewer: Richard Cooper richard.cooper@arm.com
Gerrit-Attention: Richard Cooper richard.cooper@arm.com

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 <giacomo.travaglini@arm.com> Reviewed-by: Richard Cooper <richard.cooper@arm.com> --- 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 <giacomo.travaglini@arm.com> Gerrit-Reviewer: Richard Cooper <richard.cooper@arm.com> Gerrit-Attention: Richard Cooper <richard.cooper@arm.com>