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

[Claudio Fontana]

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

?

Thanks,

Claudio

> 
>>
>>>
>>> Paolo
>>>
>>> Il lun 25 gen 2021, 06:38 Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru> ha
>>> scritto:
>>>
>>>> On 23.01.2021 21:15, Paolo Bonzini wrote:
>>>>> On 19/01/21 13:39, Pavel Dovgalyuk wrote:
>>>>>> Sometimes interrupt event comes at the same time with
>>>>>> the virtual timers. In this case replay tries to proceed
>>>>>> the timers, because deadline for them is zero.
>>>>>> This patch allows processing interrupts and exceptions
>>>>>> by entering the vCPU execution loop, when deadline is zero,
>>>>>> but checkpoint associated with virtual timers is not ready
>>>>>> to be replayed.
>>>>>>
>>>>>> Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
>>>>>> ---
>>>>>>   accel/tcg/tcg-cpus-icount.c |    8 +++++++-
>>>>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/accel/tcg/tcg-cpus-icount.c b/accel/tcg/tcg-cpus-icount.c
>>>>>> index 9f45432275..a6d2bb8a88 100644
>>>>>> --- a/accel/tcg/tcg-cpus-icount.c
>>>>>> +++ b/accel/tcg/tcg-cpus-icount.c
>>>>>> @@ -81,7 +81,13 @@ void icount_handle_deadline(void)
>>>>>>       int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
>>>>>>
>>>> QEMU_TIMER_ATTR_ALL);
>>>>>> -    if (deadline == 0) {
>>>>>> +    /*
>>>>>> +     * Instructions, interrupts, and exceptions are processed in
>>>>>> cpu-exec.
>>>>>> +     * Don't interrupt cpu thread, when these events are waiting
>>>>>> +     * (i.e., there is no checkpoint)
>>>>>> +     */
>>>>>> +    if (deadline == 0
>>>>>> +        && (replay_mode == REPLAY_MODE_RECORD ||
>>>>>> replay_has_checkpoint())) {
>>>>>
>>>>> Should this be replay_mode != REPLAY_MODE_PLAY ||
>>>> replay_has_checkpoint()?
>>>>
>>>> It was the first idea, but I thought, that == is more straightforward
>>>> to understand than !=.
>>>>
>>>> Pavel Dovgalyuk
>>>>
>>>>
>>
>>
>