Re: [PATCH] replay: fix replay of the interrupts

[Alex Bennée]

Claudio Fontana <cfontana@suse.de> writes:

> On 1/25/21 1:43 PM, Claudio Fontana wrote:
>> On 1/25/21 1:12 PM, Alex Bennée wrote:
>>>
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> In general I agree, but != means that rr disabled returns true. In general
>>>> it seems to me that rr disabled should work more or less the same as record
>>>> mode, because there is no replay log to provide the checkpoints.
>>>
>>> Is this not an argument to combine the mode and check into replay.h
>>> inline helpers with some clear semantic documentation and the call sites
>>> become self documenting?
>>>
>>> if (deadline == 0 && replay_recording_or_checkpoint())
>>>
>>> which also makes things easier to compile away if replay isn't there?
>> 
>> 
>> Seems that the TCG build faces a similar issue to the issue I was facing 
>> with the non-TCG build,
>> before the non-TCG build got functional again (for x86).
>> 
>> We solved the non-TCG build problem, by not compiling replay at all for 
>> non-TCG, plus closing our nose and stubbing away what couldn't be completely 
>> removed (yet).
>> 
>> But the CONFIG_TCG build has the same legitimate requirement towards a 
>> non-CONFIG_REPLAY build.
>> 
>> ie, like we have tcg_enabled(), should we have replay_enabled()? Maybe it 
>> could be reworked starting from replay_events_enabled()?
>> 
>> And then when things are refactored properly for replay_enabled(), a 
>> non-REPLAY TCG build can basically ignore all the inner workings of replay.
>> 
>
> I guess to summarize the above, should there be a CONFIG_REPLAY, dependent on 
> CONFIG_TCG, by default on,
> but which could be disabled with
>
> --disable-replay
>
> ?

I'm not sure - fundamentally having replay is one of those cool things
you can do when you have TCG and I suspect there is less pressure to
have a finely tuned TCG enabled build compared to our HW accelerated
brethren using hypervisors. TCG plugins are a configure option purely
because there is a small but non-marginal cost in having it enabled. I
doubt replay figures that much if you are not using it.

That said if it makes it easier to make sure our abstractions are clean
and the layering is good then maybe it is worth having a build that
allows us to check that. But if it's going to be super fiddly and
involve large amounts of code motion I doubt the "win" is big enough for
the effort.

Also I suspect the config option would be CONFIG_ICOUNT because replay
is just one of the features on the spectrum of:

 deterministic icount -> record/replay -> reverse debugging

which all require the base support for icount.

-- 
Alex Bennée