The PollEvent seems to be the GDB connect mechanism.  I don't know if that's 
changed or I didn't sufficiently test the checkpoint restore.  Oh well.
llEvent seems to be the GDB connect mechanism.  I don't know if that's changed 
to insert an earlier event (I have a vague memory of some GDB fix having 
happened recently) or I didn't sufficiently test the checkpoint restore.

In any case ...

It strikes me that the async event handling should actually tolerate these kind 
of problems and so pre-setup events should be allowed.  I've knocked together a 
fix that provides a second SC_METHOD to handle async events that doesn't report 
the error (that is, doesn't expect to have anything in the event queues yet).

For the moment, can you remove the error message/make it a warning and let me 
know if you see any other problems.

The composition of main.cc, BTW, is mostly a copy of the main.cc in cxx_config 
and so follows its slightly tortured structure.  I would have moved the 
checkpoint restore up into end_of_elaboration if it hadn't been for the wait 
statement that's required to catch gem5 up to SystemC time after checkpoint 
restore.  After thinking about it for a bit, I believe that I can separate out 
those two actions by putting the wait into run (saving the restored gem5 time 
between the two).  I'll put that in the patch.

The reason, BTW, why it doesn't happen in non-checkpoint restore is that the 
startup calls after instantiate will insert first tick events from at least the 
CPUs into the event queue causing eventLoop to exit through the first 'return' 
and not print the error (this is actually the correct behaviour).

To give a touch of perspective on why eventLoop is as it is, BTW: I started 
with eventLoop being an SC_THREAD with an explicit wait() for events at future 
times and the outer while() {} being called exactly once per 'simulate' call.  
It now works by being triggered by an event but keeps the same structure but 
with a couple of slightly ugly local 'return's.

Thanks again

- Andrew

________________________________________
From: gem5-dev [[email protected]] On Behalf Of Cagdas Dirik via 
gem5-dev [[email protected]]
Sent: 07 November 2014 23:28
To: Cagdas Dirik; Andreas Hansson; Default
Subject: Re: [gem5-dev] Review Request 2458: sim: SystemC hosting

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2458/#review5446
-----------------------------------------------------------


There is an issue with this patch for cases when a checkpoint is being 
restored. The code keeps tripping at

Module::eventLoop()
{
...
fatal("Ran out of events without seeing exit event");
}

I think keeping this fail is good and defensive. But it does not play well with 
initial PollQueue event. The way I can see the flow is:
1. SimControl::SimControl calls config_manager->instantiate()
2. CxxConfigManager::instantiate calls forEachObject(&SimObject::init)
3. this trigger cpu to schedule a poll event
4. Now this poll event triggers Module::SCEventQueue::wakeup
5. Then this causes externalSchedulingEvent to be schedule through 
Module::notify

SC_METHOD(eventLoop) has don't_initialize, so it is intentional that eventLoop 
is not entered at the very beginning. Because there is a bunch of other setup 
calls in SimControl::run, and that is where exit event is also entered into 
event queue.

Since this poll event is scheduled way early during instantiate, it ends up 
being the only event in the queue when systemc scheduler run. Exit event has 
not made it to the queue yet because systemc scheduler has not run 
SimControl::run yet. And therefore fatal is triggered.

I don't really know the reason for the poll event, and if it must be issued at 
instantiate time.

Also I understand some of the activity done in SimObject::run, but it also 
feels like initialization and simulation events are intermixed here - which is 
a bit dangerous because this is sc_thread and it is safer to have only actual 
"simulation" related activity here.

I don't have a clean way to fix this yet, so I cannot suggest a solution yet. I 
will keep looking, but I wanted to publish my thoughts here for now.

It is also a bit confusing that this is only happening in checkpoint restore 
cases, but sequence of events here suggests it should happen every time 
regardless of checkpoint_restore.

- Cagdas Dirik


On Sept. 29, 2014, 10:45 a.m., Andreas Hansson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/2458/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2014, 10:45 a.m.)
>
>
> Review request for Default.
>
>
> Repository: gem5
>
>
> Description
> -------
>
> Changeset 10455:87461da3cc35
> ---------------------------
> sim: SystemC hosting
>
> This patch hosts gem5 onto SystemC scheduler. There's already an upstream
> review board patch that does something similar but this patch ...:
>
>  1) is less obtrusive to the existing gem5 code organisation. It's divided
>  into the 'generic' preparatory patches (already submitted) and this patch
>  which affects no existing files
>
>  2) does not try to exactly track the gem5 event queue with notifys into
>  SystemC and so doesn't requive the event queue to be modified for
>  anything other than 'out of event queue' scheduling events
>
>  3) supports debug logging with SC_REPORT
>
> The patch consists of the files:
>     util/systemc/
>         sc_gem5_control.{cc,hh} -- top level objects to use to
>                                    instantiate gem5 Systems within
>                                    larger SystemC test harnesses as
>                                    sc_module objects
>         sc_logger.{cc,hh}       -- logging support
>         sc_module.{cc,hh}       -- a separated event loop specific to
>                                    SystemC
>         stats.{cc,hh}           -- example Stats handling for the sample
>                                    top level
>         main.{cc,hh}            -- a sample top level
>
> On the downside this patch is only currently functional with C++
> configuration at the top level.
>
> The above sc_... files are indended to be compiled alongside gem5 (as a
> library, see main.cc for a command line and util/systemc/README for
> more details.)
>
> The top-level system instantiation in sc_gem5_control.{cc,hh} provides
> two classes: Gem5Control and Gem5System
>
> Gem5Control is a simulation control class (from which a singleton
> object should be created) derived from Gem5SystemC::Module which
> carries the top level simulation control interface for gem5.  This
> includes hosting a system-building configuration file and
> instantiating the Root object from that file.
>
> Gem5System is a base class for instantiating renamed gem5 Systems
> from the config file hosted by the Gem5Control object.  In use, a
> SystemC module class should be made which represents the desired,
> instantiable gem5 System.  That class's instances should create
> a Gem5System during their construction, set the parameters of that
> system and then call instantiate to build that system.  If this
> is all carried out in the sc_core::sc_module-derived classes
> constructor, the System's external ports will become children of
> that module and can then be recovered by name using sc_core::
> sc_find_object.
>
> It is intended that this interface is used with dlopen.  To that
> end, the header file sc_gem5_control.hh includes no other header
> files from gem5 (and so can be easily copied into another project).
> The classes Gem5System and Gem5Control have all their member
> functions declared `virtual' so that those functions can be called
> through the vtable acquired by building the top level Gem5Control
> using dlsym(..., "makeGem5Control") and `makeSystem' on the
> Gem5Control.
>
>
> Diffs
> -----
>
>   util/systemc/Makefile PRE-CREATION
>   util/systemc/README PRE-CREATION
>   util/systemc/main.cc PRE-CREATION
>   util/systemc/sc_gem5_control.hh PRE-CREATION
>   util/systemc/sc_gem5_control.cc PRE-CREATION
>   util/systemc/sc_logger.hh PRE-CREATION
>   util/systemc/sc_logger.cc PRE-CREATION
>   util/systemc/sc_module.hh PRE-CREATION
>   util/systemc/sc_module.cc PRE-CREATION
>   util/systemc/stats.hh PRE-CREATION
>   util/systemc/stats.cc PRE-CREATION
>
> Diff: http://reviews.gem5.org/r/2458/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andreas Hansson
>
>

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev


-- IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered 
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to