> On July 4, 2015, 12:15 p.m., Steve Reinhardt wrote: > > src/sim/serialize.hh, line 337 > > <http://reviews.gem5.org/r/2861/diff/1/?file=45893#file45893line337> > > > > Is there a plan in place to change the objects that use this? If not > > (i.e., if there's a legitimate need for some objects to modify their state > > when chackpointing) then we shouldn't be so adamant about calling this > > serializeOld() and saying "don't ever use this". Why not call it something > > like serializeNonConst() and just say "it's desirable to avoid this when > > possible"? > > Andreas Sandberg wrote: > I can't see how there could be a legitimate need to modify state when > serializing. The whole reason for drain and the memWriteback/memInvalidate > calls is to sort out that kind of business before draining and switching > CPUs. At the moment, the following components still use the old non-const > call: > > * The Sinic NIC device > * The i8042 keyboard controller > * Ruby > > I think Ruby will be mostly fixed by Tim Jones's patches. I'm very > reluctant to poke around in Ruby myself. The i8042 shouldn't be too hard to > fix, I could probably do that in a future patch. The Sinic device had some > minor quirks that made it non-trivial to transition to the new stricter > regime, but it's definitely doable and something I could do in a separate > patch. > > I really, really, want all state mangling to happen in drain et al., this > makes testing a lot easier. Particularly, I want to be able to test that the > system ends up in the same state when doing this sequence: > > * drain() > * checkpoint() + restore() (optional switch CPU before restoring) > * resume() > > And this sequence: > * drain() > * memWriteback() + memInvalidate() > * Optionally switch CPU > * resume()
OK, this all makes sense to me, particularly saying that drain is the place for state changes. Can you add a sentence to the comment here saying that there's no need for non-const serialization because all state changes should take place in drain or memWriteback/memInvalidate? - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/2861/#review6707 ----------------------------------------------------------- On May 28, 2015, 10:35 a.m., Andreas Sandberg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2861/ > ----------------------------------------------------------- > > (Updated May 28, 2015, 10:35 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 10854:dfde63d4e52c > --------------------------- > sim: Refactor the serialization base class > > Objects that are can be serialized are supposed to inherit from the > Serializable class. This class is meant to provide a unified API for > such objects. However, so far it has mainly been used by SimObjects > due to some fundamental design limitations. This changeset redesigns > to the serialization interface to make it more generic and hide the > underlying checkpoint storage. Specifically: > > * Add a set of APIs to serialize into a subsection of the current > object. Previously, objects that needed this functionality would > use ad-hoc solutions using nameOut() and section name > generation. In the new world, an object that implements the > interface has the methods serializeSection() and > unserializeSection() that serialize into a named /subsection/ of > the current object. Calling serialize() serializes an object into > the current section. > > * Move the name() method from Serializable to SimObject as it is no > longer needed for serialization. The fully qualified section name > is generated by the main serialization code on the fly as objects > serialize sub-objects. > > * Add a scoped ScopedCheckpointSection helper class. Some objects > need to serialize data structures, that are not deriving from > Serializable, into subsections. Previously, this was done using > nameOut() and manual section name generation. To simplify this, > this changeset introduces a ScopedCheckpointSection() helper > class. When this class is instantiated, it adds a new /subsection/ > and subsequent serialization calls during the lifetime of this > helper class happen inside this section (or a subsection in case > of nested sections). > > * The serialize() call is now const which prevents accidental state > manipulation during serialization. Objects that rely on modifying > state can use the serializeOld() call instead. The default > implementation simply calls serialize(). Note: The old-style calls > need to be explicitly called using the > serializeOld()/serializeSectionOld() style APIs. These are used by > default when serializing SimObjects. > > * Both the input and output checkpoints now use their own named > types. This hides underlying checkpoint implementation from > objects that need checkpointing and makes it easier to change the > underlying checkpoint storage code. > > > Diffs > ----- > > src/arch/generic/types.hh e61f847e74fd > src/arch/mips/interrupts.hh e61f847e74fd > src/arch/mips/pagetable.hh e61f847e74fd > src/arch/arm/tlb.cc e61f847e74fd > src/arch/arm/types.hh e61f847e74fd > src/arch/arm/pmu.cc e61f847e74fd > src/arch/arm/tlb.hh e61f847e74fd > src/arch/arm/interrupts.hh e61f847e74fd > src/arch/arm/isa.hh e61f847e74fd > src/arch/arm/kvm/gic.hh PRE-CREATION > src/arch/arm/kvm/gic.cc PRE-CREATION > src/arch/arm/pagetable.hh e61f847e74fd > src/arch/arm/pmu.hh e61f847e74fd > src/arch/alpha/interrupts.hh e61f847e74fd > src/arch/alpha/isa.hh e61f847e74fd > src/arch/alpha/isa.cc e61f847e74fd > src/arch/alpha/kernel_stats.hh e61f847e74fd > src/arch/alpha/kernel_stats.cc e61f847e74fd > src/arch/alpha/pagetable.hh e61f847e74fd > src/arch/alpha/pagetable.cc e61f847e74fd > src/arch/alpha/process.hh e61f847e74fd > src/arch/alpha/process.cc e61f847e74fd > src/arch/alpha/system.hh e61f847e74fd > src/arch/alpha/system.cc e61f847e74fd > src/arch/alpha/tlb.hh e61f847e74fd > src/arch/alpha/tlb.cc e61f847e74fd > src/sim/voltage_domain.cc e61f847e74fd > src/sim/ticked_object.hh e61f847e74fd > src/sim/ticked_object.cc e61f847e74fd > src/sim/voltage_domain.hh e61f847e74fd > src/sim/system.hh e61f847e74fd > src/sim/system.cc e61f847e74fd > src/sim/sim_object.hh e61f847e74fd > src/sim/sim_object.cc e61f847e74fd > src/sim/root.hh e61f847e74fd > src/sim/root.cc e61f847e74fd > src/sim/serialize.hh e61f847e74fd > src/sim/serialize.cc e61f847e74fd > src/sim/sim_events.hh e61f847e74fd > src/sim/sim_events.cc e61f847e74fd > src/sim/cxx_manager.cc e61f847e74fd > src/sim/dvfs_handler.hh e61f847e74fd > src/sim/dvfs_handler.cc e61f847e74fd > src/sim/eventq.hh e61f847e74fd > src/sim/eventq.cc e61f847e74fd > src/sim/process.hh e61f847e74fd > src/sim/process.cc e61f847e74fd > src/python/swig/pyobject.cc e61f847e74fd > src/sim/clock_domain.hh e61f847e74fd > src/sim/clock_domain.cc e61f847e74fd > src/sim/cxx_manager.hh e61f847e74fd > src/mem/physical.cc e61f847e74fd > src/mem/ruby/system/System.hh e61f847e74fd > src/mem/ruby/system/System.cc e61f847e74fd > src/python/m5/SimObject.py e61f847e74fd > src/python/swig/core.i e61f847e74fd > src/python/swig/pyobject.hh e61f847e74fd > src/dev/arm/timer_cpulocal.hh e61f847e74fd > src/dev/arm/timer_cpulocal.cc e61f847e74fd > src/dev/arm/timer_sp804.hh e61f847e74fd > src/dev/arm/timer_sp804.cc e61f847e74fd > src/dev/arm/ufs_device.hh e61f847e74fd > src/dev/arm/ufs_device.cc e61f847e74fd > src/dev/arm/vgic.hh e61f847e74fd > src/dev/arm/vgic.cc e61f847e74fd > src/dev/copy_engine.hh e61f847e74fd > src/dev/copy_engine.cc e61f847e74fd > src/dev/copy_engine_defs.hh e61f847e74fd > src/dev/disk_image.hh e61f847e74fd > src/dev/disk_image.cc e61f847e74fd > src/dev/etherlink.hh e61f847e74fd > src/dev/etherlink.cc e61f847e74fd > src/dev/etherpkt.hh e61f847e74fd > src/dev/etherpkt.cc e61f847e74fd > src/dev/ethertap.hh e61f847e74fd > src/dev/ethertap.cc e61f847e74fd > src/dev/i2cbus.hh e61f847e74fd > src/dev/i2cbus.cc e61f847e74fd > src/dev/i8254xGBe.hh e61f847e74fd > src/dev/i8254xGBe.cc e61f847e74fd > src/dev/i8254xGBe_defs.hh e61f847e74fd > src/dev/ide_ctrl.hh e61f847e74fd > src/dev/ide_ctrl.cc e61f847e74fd > src/dev/ide_disk.hh e61f847e74fd > src/dev/ide_disk.cc e61f847e74fd > src/dev/intel_8254_timer.hh e61f847e74fd > src/dev/intel_8254_timer.cc e61f847e74fd > src/dev/mc146818.hh e61f847e74fd > src/dev/mc146818.cc e61f847e74fd > src/dev/mips/malta.hh e61f847e74fd > src/dev/mips/malta.cc e61f847e74fd > src/dev/mips/malta_cchip.hh e61f847e74fd > src/dev/mips/malta_cchip.cc e61f847e74fd > src/dev/mips/malta_io.hh e61f847e74fd > src/dev/mips/malta_io.cc e61f847e74fd > src/dev/mips/malta_pchip.hh e61f847e74fd > src/dev/mips/malta_pchip.cc e61f847e74fd > src/dev/ns_gige.hh e61f847e74fd > src/dev/ns_gige.cc e61f847e74fd > src/dev/pcidev.hh e61f847e74fd > src/dev/pcidev.cc e61f847e74fd > src/dev/pktfifo.hh e61f847e74fd > src/dev/pktfifo.cc e61f847e74fd > src/dev/sinic.hh e61f847e74fd > src/dev/sinic.cc e61f847e74fd > src/dev/sparc/dtod.hh e61f847e74fd > src/dev/sparc/dtod.cc e61f847e74fd > src/dev/sparc/iob.hh e61f847e74fd > src/dev/sparc/iob.cc e61f847e74fd > src/dev/sparc/mm_disk.hh e61f847e74fd > src/dev/sparc/mm_disk.cc e61f847e74fd > src/dev/uart8250.hh e61f847e74fd > src/dev/uart8250.cc e61f847e74fd > src/dev/virtio/base.hh e61f847e74fd > src/dev/virtio/base.cc e61f847e74fd > src/dev/virtio/fs9p.hh e61f847e74fd > src/dev/virtio/fs9p.cc e61f847e74fd > src/dev/x86/cmos.hh e61f847e74fd > src/dev/x86/cmos.cc e61f847e74fd > src/dev/x86/i8042.hh e61f847e74fd > src/dev/x86/i8042.cc e61f847e74fd > src/dev/x86/i82094aa.hh e61f847e74fd > src/dev/x86/i82094aa.cc e61f847e74fd > src/dev/x86/i8237.hh e61f847e74fd > src/dev/x86/i8237.cc e61f847e74fd > src/dev/x86/i8254.hh e61f847e74fd > src/dev/x86/i8254.cc e61f847e74fd > src/dev/x86/i8259.hh e61f847e74fd > src/dev/x86/i8259.cc e61f847e74fd > src/dev/x86/speaker.hh e61f847e74fd > src/dev/x86/speaker.cc e61f847e74fd > src/kern/kernel_stats.hh e61f847e74fd > src/kern/kernel_stats.cc e61f847e74fd > src/mem/cache/cache.hh e61f847e74fd > src/mem/cache/cache_impl.hh e61f847e74fd > src/mem/multi_level_page_table.hh e61f847e74fd > src/mem/multi_level_page_table_impl.hh e61f847e74fd > src/mem/page_table.hh e61f847e74fd > src/mem/page_table.cc e61f847e74fd > src/mem/physical.hh e61f847e74fd > src/dev/arm/rtc_pl031.hh e61f847e74fd > src/dev/arm/rtc_pl031.cc e61f847e74fd > src/dev/arm/rv_ctrl.hh e61f847e74fd > src/dev/arm/rv_ctrl.cc e61f847e74fd > src/cpu/testers/traffic_gen/traffic_gen.cc e61f847e74fd > src/cpu/thread_context.hh e61f847e74fd > src/cpu/thread_context.cc e61f847e74fd > src/cpu/thread_state.hh e61f847e74fd > src/cpu/thread_state.cc e61f847e74fd > src/dev/alpha/backdoor.hh e61f847e74fd > src/dev/alpha/backdoor.cc e61f847e74fd > src/dev/alpha/tsunami.hh e61f847e74fd > src/dev/alpha/tsunami.cc e61f847e74fd > src/dev/alpha/tsunami_cchip.hh e61f847e74fd > src/dev/alpha/tsunami_cchip.cc e61f847e74fd > src/dev/alpha/tsunami_io.hh e61f847e74fd > src/dev/alpha/tsunami_io.cc e61f847e74fd > src/dev/alpha/tsunami_pchip.hh e61f847e74fd > src/dev/alpha/tsunami_pchip.cc e61f847e74fd > src/dev/arm/energy_ctrl.hh e61f847e74fd > src/dev/arm/energy_ctrl.cc e61f847e74fd > src/dev/arm/flash_device.hh e61f847e74fd > src/dev/arm/flash_device.cc e61f847e74fd > src/dev/arm/generic_timer.hh e61f847e74fd > src/dev/arm/generic_timer.cc e61f847e74fd > src/dev/arm/gic_pl390.hh e61f847e74fd > src/dev/arm/gic_pl390.cc e61f847e74fd > src/dev/arm/hdlcd.hh e61f847e74fd > src/dev/arm/hdlcd.cc e61f847e74fd > src/dev/arm/kmi.hh e61f847e74fd > src/dev/arm/kmi.cc e61f847e74fd > src/dev/arm/pl011.hh e61f847e74fd > src/dev/arm/pl011.cc e61f847e74fd > src/dev/arm/pl111.hh e61f847e74fd > src/dev/arm/pl111.cc e61f847e74fd > src/cpu/minor/cpu.hh e61f847e74fd > src/cpu/minor/cpu.cc e61f847e74fd > src/cpu/o3/cpu.hh e61f847e74fd > src/cpu/o3/cpu.cc e61f847e74fd > src/cpu/o3/thread_state.hh e61f847e74fd > src/cpu/simple/base.hh e61f847e74fd > src/cpu/simple/base.cc e61f847e74fd > src/cpu/simple_thread.hh e61f847e74fd > src/cpu/simple_thread.cc e61f847e74fd > src/cpu/testers/traffic_gen/traffic_gen.hh e61f847e74fd > src/arch/x86/types.hh e61f847e74fd > src/arch/x86/types.cc e61f847e74fd > src/base/cp_annotate.hh e61f847e74fd > src/base/cp_annotate.cc e61f847e74fd > src/base/loader/symtab.hh e61f847e74fd > src/base/loader/symtab.cc e61f847e74fd > src/base/pollevent.hh e61f847e74fd > src/base/pollevent.cc e61f847e74fd > src/base/random.hh e61f847e74fd > src/base/random.cc e61f847e74fd > src/base/time.hh e61f847e74fd > src/base/time.cc e61f847e74fd > src/cpu/base.hh e61f847e74fd > src/cpu/base.cc e61f847e74fd > src/cpu/checker/cpu.hh e61f847e74fd > src/cpu/checker/cpu.cc e61f847e74fd > src/cpu/checker/thread_context.hh e61f847e74fd > src/cpu/kvm/BaseKvmCPU.py e61f847e74fd > src/cpu/kvm/base.hh e61f847e74fd > src/cpu/kvm/base.cc e61f847e74fd > src/cpu/kvm/x86_cpu.hh e61f847e74fd > src/cpu/kvm/x86_cpu.cc e61f847e74fd > src/arch/sparc/interrupts.hh e61f847e74fd > src/arch/sparc/isa.hh e61f847e74fd > src/arch/sparc/isa.cc e61f847e74fd > src/arch/sparc/pagetable.hh e61f847e74fd > src/arch/sparc/pagetable.cc e61f847e74fd > src/arch/sparc/system.hh e61f847e74fd > src/arch/sparc/system.cc e61f847e74fd > src/arch/sparc/tlb.hh e61f847e74fd > src/arch/sparc/tlb.cc e61f847e74fd > src/arch/x86/interrupts.hh e61f847e74fd > src/arch/x86/interrupts.cc e61f847e74fd > src/arch/x86/isa.hh e61f847e74fd > src/arch/x86/isa.cc e61f847e74fd > src/arch/x86/pagetable.hh e61f847e74fd > src/arch/x86/pagetable.cc e61f847e74fd > src/arch/x86/tlb.hh e61f847e74fd > src/arch/x86/tlb.cc e61f847e74fd > src/arch/mips/pagetable.cc e61f847e74fd > src/arch/mips/tlb.hh e61f847e74fd > src/arch/mips/tlb.cc e61f847e74fd > src/arch/power/pagetable.hh e61f847e74fd > src/arch/power/pagetable.cc e61f847e74fd > src/arch/power/tlb.hh e61f847e74fd > src/arch/power/tlb.cc e61f847e74fd > > Diff: http://reviews.gem5.org/r/2861/diff/ > > > Testing > ------- > > Quick regressions pass for all platforms, including checkpoint regressions on > ARM. Generated full-system checkpoint for X86, Alpha, and ARM. Compared > checkpoints before and after patch (checkpoints are identical). > > Note to reviewers: I'm really sorry about the size of this patch. :( > > > Thanks, > > Andreas Sandberg > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev