-----Original Message-----
From: [email protected] [mailto:[email protected]] On
Behalf Of Gabriel Michael Black
Sent: Thursday, December 09, 2010 9:04 PM
To: [email protected]
Subject: Re: [m5-dev] changeset in m5: ARM: Add checkpointing support
In reference to this and the next email, I don't think these should be
conditioned on the ARM ISA because it looks like those values are
valid and need to be serialized for all ISAs. It's just that the other
ISAs happen to have checkpoints that are old enough that they break,
and that's not a good enough reason to introduce a superficial ISA
incompatibility.
The core problem, which is a real issue, is that checkpoints created
with one version of M5 aren't necessarily compatible with other
versions, although they tend to be. That's unfortunate, but I think to
avoid really binding our hands as far as how we can change things,
that's just one of those things we'll need to live with. We could try
to implement some sort of versioning system maybe just to warn that
something is incompatible, or maybe have a default value if something
isn't found. I'm not sure whether that would solve the problem,
though, and it would add a decent amount of complexity.
As far as regression testing, that would be great. There have been a
lot of times where I've wished we had that since I accidentally broke
something :-). I remember hearing that would be hard to do for some
reason, though.
Gabe
Quoting "Beckmann, Brad" <[email protected]>:
> Hi Ali,
>
> I just synced with this changeset 7733, as well as changeset 7730,
> and I now notice that the modifications to physical.cc break all
> previous checkpoints. Can we put the lal_addr and lal_cid
> serialization and unserialization in a conditional that tests for
> the ARM ISA? I welcome other suggestions as well.
>
> In general, I would be interested to hear other people's thoughts on
> adding a checkpoint test to the regression tester. It would be
> great if we can at least identify ahead of time what changesets
> break older checkpoints.
>
> Brad
>
>
>
>> -----Original Message-----
>> From: [email protected] [mailto:[email protected]] On
>> Behalf Of Ali Saidi
>> Sent: Monday, November 08, 2010 11:59 AM
>> To: [email protected]
>> Subject: [m5-dev] changeset in m5: ARM: Add checkpointing support
>>
>> changeset 08d6a773d1b6 in /z/repo/m5
>> details: http://repo.m5sim.org/m5?cmd=changeset;node=08d6a773d1b6
>> description:
>> ARM: Add checkpointing support
>>
>> diffstat:
>>
>> src/arch/arm/isa.hh | 12 +++++-
>> src/arch/arm/linux/system.cc | 5 +-
>> src/arch/arm/linux/system.hh | 4 +-
>> src/arch/arm/pagetable.hh | 87 +++++++++++++++++++++++---------
---
>> --------
>> src/arch/arm/table_walker.cc | 16 ++++++-
>> src/arch/arm/table_walker.hh | 2 +-
>> src/arch/arm/tlb.cc | 14 ++++++-
>> src/arch/arm/tlb.hh | 2 -
>> src/dev/arm/gic.cc | 44 +++++++++++++++++++++-
>> src/dev/arm/pl011.cc | 42 ++++++++++++++++++++-
>> src/dev/arm/rv_ctrl.cc | 2 -
>> src/dev/arm/timer_sp804.cc | 59 ++++++++++++++++++++++++++++-
>> src/dev/arm/timer_sp804.hh | 4 ++
>> src/mem/physical.cc | 30 +++++++++++++++
>> src/mem/physical.hh | 5 ++
>> src/sim/SConscript | 1 +
>> src/sim/system.cc | 2 +-
>> src/sim/system.hh | 2 +-
>> 18 files changed, 268 insertions(+), 65 deletions(-)
>>
>> diffs (truncated from 587 to 300 lines):
>>
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/isa.hh
>> --- a/src/arch/arm/isa.hh Mon Nov 08 13:58:24 2010 -0600
>> +++ b/src/arch/arm/isa.hh Mon Nov 08 13:58:25 2010 -0600
>> @@ -178,10 +178,18 @@
>> }
>>
>> void serialize(EventManager *em, std::ostream &os)
>> - {}
>> + {
>> + DPRINTF(Checkpoint, "Serializing Arm Misc
Registers\n");
>> + SERIALIZE_ARRAY(miscRegs, NumMiscRegs);
>> + }
>> void unserialize(EventManager *em, Checkpoint *cp,
>> const std::string §ion)
>> - {}
>> + {
>> + DPRINTF(Checkpoint, "Unserializing Arm Misc
Registers\n");
>> + UNSERIALIZE_ARRAY(miscRegs, NumMiscRegs);
>> + CPSR tmp_cpsr = miscRegs[MISCREG_CPSR];
>> + updateRegMap(tmp_cpsr);
>> + }
>>
>> ISA()
>> {
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/linux/system.cc
>> --- a/src/arch/arm/linux/system.cc Mon Nov 08 13:58:24 2010 -
0600
>> +++ b/src/arch/arm/linux/system.cc Mon Nov 08 13:58:25 2010 -
0600
>> @@ -99,9 +99,9 @@
>> }
>>
>> void
>> -LinuxArmSystem::startup()
>> +LinuxArmSystem::initState()
>> {
>> - ArmSystem::startup();
>> + ArmSystem::initState();
>> ThreadContext *tc = threadContexts[0];
>>
>> // Set the initial PC to be at start of the kernel code
>> @@ -117,7 +117,6 @@
>> {
>> }
>>
>> -
>> LinuxArmSystem *
>> LinuxArmSystemParams::create()
>> {
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/linux/system.hh
>> --- a/src/arch/arm/linux/system.hh Mon Nov 08 13:58:24 2010 -
0600
>> +++ b/src/arch/arm/linux/system.hh Mon Nov 08 13:58:25 2010 -
0600
>> @@ -67,8 +67,8 @@
>> LinuxArmSystem(Params *p);
>> ~LinuxArmSystem();
>>
>> - /** Initialize the CPU for booting */
>> - void startup();
>> + void initState();
>> +
>> private:
>> #ifndef NDEBUG
>> /** Event to halt the simulator if the kernel calls panic() */
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/pagetable.hh
>> --- a/src/arch/arm/pagetable.hh Mon Nov 08 13:58:24 2010 -0600
>> +++ b/src/arch/arm/pagetable.hh Mon Nov 08 13:58:25 2010 -0600
>> @@ -48,6 +48,8 @@
>> #include "arch/arm/vtophys.hh"
>> #include "config/full_system.hh"
>>
>> +#include "sim/serialize.hh"
>> +
>> namespace ArmISA {
>>
>> struct VAddr
>> @@ -71,39 +73,6 @@
>>
>> };
>>
>> -struct TlbRange
>> -{
>> - Addr va;
>> - Addr size;
>> - int contextId;
>> - bool global;
>> -
>> - inline bool
>> - operator<(const TlbRange &r2) const
>> - {
>> - if (!(global || r2.global)) {
>> - if (contextId < r2.contextId)
>> - return true;
>> - else if (contextId > r2.contextId)
>> - return false;
>> - }
>> -
>> - if (va < r2.va)
>> - return true;
>> - return false;
>> - }
>> -
>> - inline bool
>> - operator==(const TlbRange &r2) const
>> - {
>> - return va == r2.va &&
>> - size == r2.size &&
>> - contextId == r2.contextId &&
>> - global == r2.global;
>> - }
>> -};
>> -
>> -
>> // ITB/DTB table entry
>> struct TlbEntry
>> {
>> @@ -143,10 +112,8 @@
>>
>> // Access permissions
>> bool xn; // Execute Never
>> - uint8_t ap:3; // Access permissions bits
>> - uint8_t domain:4; // Access Domain
>> -
>> - TlbRange range; // For fast TLB searching
>> + uint8_t ap; // Access permissions bits
>> + uint8_t domain; // Access Domain
>>
>> //Construct an entry that maps to physical address addr for SE
>> mode
>> TlbEntry(Addr _asn, Addr _vaddr, Addr _paddr)
>> @@ -196,9 +163,49 @@
>> return (pfn << N) | (va & size);
>> }
>>
>> - void serialize(std::ostream &os) { panic("Need to
Implement\n"); }
>> - void unserialize(Checkpoint *cp, const std::string §ion)
>> - { panic("Need to Implement\n");}
>> + void
>> + serialize(std::ostream &os)
>> + {
>> + SERIALIZE_SCALAR(pfn);
>> + SERIALIZE_SCALAR(size);
>> + SERIALIZE_SCALAR(vpn);
>> + SERIALIZE_SCALAR(asid);
>> + SERIALIZE_SCALAR(N);
>> + SERIALIZE_SCALAR(global);
>> + SERIALIZE_SCALAR(valid);
>> + SERIALIZE_SCALAR(nonCacheable);
>> + SERIALIZE_SCALAR(sNp);
>> + SERIALIZE_ENUM(mtype);
>> + SERIALIZE_SCALAR(innerAttrs);
>> + SERIALIZE_SCALAR(outerAttrs);
>> + SERIALIZE_SCALAR(shareable);
>> + SERIALIZE_SCALAR(attributes);
>> + SERIALIZE_SCALAR(xn);
>> + SERIALIZE_SCALAR(ap);
>> + SERIALIZE_SCALAR(domain);
>> + }
>> + void
>> + unserialize(Checkpoint *cp, const std::string §ion)
>> + {
>> + UNSERIALIZE_SCALAR(pfn);
>> + UNSERIALIZE_SCALAR(size);
>> + UNSERIALIZE_SCALAR(vpn);
>> + UNSERIALIZE_SCALAR(asid);
>> + UNSERIALIZE_SCALAR(N);
>> + UNSERIALIZE_SCALAR(global);
>> + UNSERIALIZE_SCALAR(valid);
>> + UNSERIALIZE_SCALAR(nonCacheable);
>> + UNSERIALIZE_SCALAR(sNp);
>> + UNSERIALIZE_ENUM(mtype);
>> + UNSERIALIZE_SCALAR(innerAttrs);
>> + UNSERIALIZE_SCALAR(outerAttrs);
>> + UNSERIALIZE_SCALAR(shareable);
>> + UNSERIALIZE_SCALAR(attributes);
>> + UNSERIALIZE_SCALAR(xn);
>> + UNSERIALIZE_SCALAR(ap);
>> + UNSERIALIZE_SCALAR(domain);
>> + }
>> +
>> };
>>
>>
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/table_walker.cc
>> --- a/src/arch/arm/table_walker.cc Mon Nov 08 13:58:24 2010 -
0600
>> +++ b/src/arch/arm/table_walker.cc Mon Nov 08 13:58:25 2010 -
0600
>> @@ -59,10 +59,20 @@
>> }
>>
>>
>> -unsigned int
>> -drain(Event *de)
>> +unsigned int TableWalker::drain(Event *de)
>> {
>> - panic("Not implemented\n");
>> + if (stateQueueL1.size() != 0 || stateQueueL2.size() != 0)
>> + {
>> + changeState(Draining);
>> + DPRINTF(Checkpoint, "TableWalker busy, wait to drain\n");
>> + return 1;
>> + }
>> + else
>> + {
>> + changeState(Drained);
>> + DPRINTF(Checkpoint, "TableWalker free, no need to
drain\n");
>> + return 0;
>> + }
>> }
>>
>> Port*
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/table_walker.hh
>> --- a/src/arch/arm/table_walker.hh Mon Nov 08 13:58:24 2010 -
0600
>> +++ b/src/arch/arm/table_walker.hh Mon Nov 08 13:58:25 2010 -
0600
>> @@ -350,7 +350,7 @@
>> return dynamic_cast<const Params *>(_params);
>> }
>>
>> - virtual unsigned int drain(Event *de) { panic("write me\n"); }
>> + virtual unsigned int drain(Event *de);
>> virtual Port *getPort(const std::string &if_name, int idx = -
1);
>>
>> Fault walk(RequestPtr req, ThreadContext *tc, uint8_t cid,
>> TLB::Mode mode,
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/tlb.cc
>> --- a/src/arch/arm/tlb.cc Mon Nov 08 13:58:24 2010 -0600
>> +++ b/src/arch/arm/tlb.cc Mon Nov 08 13:58:25 2010 -0600
>> @@ -245,14 +245,24 @@
>> void
>> TLB::serialize(ostream &os)
>> {
>> - panic("Implement Serialize\n");
>> + DPRINTF(Checkpoint, "Serializing Arm TLB\n");
>> +
>> + SERIALIZE_SCALAR(_attr);
>> + for(int i = 0; i < size; i++){
>> + nameOut(os, csprintf("%s.TlbEntry%d", name(), i));
>> + table[i].serialize(os);
>> + }
>> }
>>
>> void
>> TLB::unserialize(Checkpoint *cp, const string §ion)
>> {
>> + DPRINTF(Checkpoint, "Unserializing Arm TLB\n");
>>
>> - panic("Need to properly unserialize TLB\n");
>> + UNSERIALIZE_SCALAR(_attr);
>> + for(int i = 0; i < size; i++){
>> + table[i].unserialize(cp, csprintf("%s.TlbEntry%d", section,
>> i));
>> + }
>> }
>>
>> void
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/arch/arm/tlb.hh
>> --- a/src/arch/arm/tlb.hh Mon Nov 08 13:58:24 2010 -0600
>> +++ b/src/arch/arm/tlb.hh Mon Nov 08 13:58:25 2010 -0600
>> @@ -83,8 +83,6 @@
>> MustBeOne = 0x80
>> };
>> protected:
>> - typedef std::multimap<Addr, int> PageTable;
>> - PageTable lookupTable; // Quick lookup into page table
>>
>> TlbEntry *table; // the Page Table
>> int size; // TLB Size
>> diff -r a2c660de7787 -r 08d6a773d1b6 src/dev/arm/gic.cc
>> --- a/src/dev/arm/gic.cc Mon Nov 08 13:58:24 2010 -0600
>> +++ b/src/dev/arm/gic.cc Mon Nov 08 13:58:25 2010 -0600
>> @@ -495,13 +495,53 @@
>> void
>> Gic::serialize(std::ostream &os)
>> {
>> - panic("Need to implement serialization\n");
>> + DPRINTF(Checkpoint, "Serializing Arm GIC\n");
>> +
>> + SERIALIZE_SCALAR(distAddr);
>> + SERIALIZE_SCALAR(cpuAddr);
>> + SERIALIZE_SCALAR(distPioDelay);
>> + SERIALIZE_SCALAR(cpuPioDelay);
>> + SERIALIZE_SCALAR(enabled);
>> + SERIALIZE_SCALAR(itLines);
>> + SERIALIZE_SCALAR(itLinesLog2);
>> + SERIALIZE_ARRAY(intEnabled, 32);
>> + SERIALIZE_ARRAY(pendingInt, 32);
>> + SERIALIZE_ARRAY(activeInt, 32);
>> + SERIALIZE_ARRAY(iccrpr, 8);
>> + SERIALIZE_ARRAY(intPriority, 1020);
>> + SERIALIZE_ARRAY(cpuTarget, 1020);
>> + SERIALIZE_ARRAY(intConfig, 64);
>> + SERIALIZE_ARRAY(cpuEnabled, 8);
>> + SERIALIZE_ARRAY(cpuPriority, 8);
>> + SERIALIZE_ARRAY(cpuBpr, 8);
>> + SERIALIZE_ARRAY(cpuHighestInt, 8);
>> + SERIALIZE_SCALAR(irqEnable);
>> }
>>
>> void
>> Gic::unserialize(Checkpoint *cp, const std::string §ion)
>> {
>> - panic("Need to implement serialization\n");
>> + DPRINTF(Checkpoint, "Unserializing Arm GIC\n");
>> +
>> + UNSERIALIZE_SCALAR(distAddr);
>> + UNSERIALIZE_SCALAR(cpuAddr);
>> + UNSERIALIZE_SCALAR(distPioDelay);
>> _______________________________________________
>> m5-dev mailing list
>> [email protected]
>> http://m5sim.org/mailman/listinfo/m5-dev
>
>
> _______________________________________________
> m5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/m5-dev
>
_______________________________________________
m5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/m5-dev
