Re: [PATCH] qemu-options.hx: Fix minor issues in icount documentation

[Laurent Vivier]

Le 13/12/2020 à 16:54, Laurent Vivier a écrit :
> Le 21/11/2020 à 22:35, Peter Maydell a écrit :
>> The documentation for the icount documentation has some minor issues:
>>  * in a couple of places it says "sleep=on|off" when in the context of the
>>    sentence it means specifically "sleep=on"
>>  * the synopsis line for the documentation has drifted out of sync
>>    with the synopsis line in the DEF() macro (used for "-help" output)
>>  * the synopsis line in the DEF() macro is missing a "][" between
>>    the sleep= part and the rr= part
>>  * the synopsis line doesn't indicate that rrsnapshot is an optional
>>    part of the rr=mode,rrfile=filename subgrouping
>>  * we don't document that sleep=on can't be used with shift=auto
>>    or align=on
>>  * the rr option description had some minor grammar and formatting
>>    errors and was a bit terse
>>  * in commit f1f4b57e88ff in 2015 the documentation of the sleep=
>>    suboption got added between the two paragraphs defining general
>>    behaviour of the icount option. This meant that the second
>>    paragraph talking about the behaviour of "this option" reads as
>>    if it's talking about sleep=on, when it's really describing -icount
>>    as a whole. The paragraph is better moved back up to above the
>>    sleep= section.
>>  * the summary text displayed in "-help" output didn't mention
>>    the record-and-replay part
>>
>> Fix these errors.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1774412
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  qemu-options.hx | 42 ++++++++++++++++++++++++------------------
>>  1 file changed, 24 insertions(+), 18 deletions(-)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 2c83390504d..41cb42e7a5d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -3956,30 +3956,34 @@ SRST
>>  ERST
>>  
>>  DEF("icount", HAS_ARG, QEMU_OPTION_icount, \
>> -    "-icount 
>> [shift=N|auto][,align=on|off][,sleep=on|off,rr=record|replay,rrfile=<filename>,rrsnapshot=<snapshot>]\n"
>>  \
>> +    "-icount 
>> [shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=<filename>[,rrsnapshot=<snapshot>]]\n"
>>  \
>>      "                enable virtual instruction counter with 2^N clock 
>> ticks per\n" \
>>      "                instruction, enable aligning the host and virtual 
>> clocks\n" \
>> -    "                or disable real time cpu sleeping\n", QEMU_ARCH_ALL)
>> +    "                or disable real time cpu sleeping, and optionally 
>> enable\n" \
>> +    "                record-and-replay mode\n", QEMU_ARCH_ALL)
>>  SRST
>> -``-icount 
>> [shift=N|auto][,rr=record|replay,rrfile=filename,rrsnapshot=snapshot]``
>> +``-icount 
>> [shift=N|auto][,align=on|off][,sleep=on|off][,rr=record|replay,rrfile=filename[,rrsnapshot=snapshot]]``
>>      Enable virtual instruction counter. The virtual cpu will execute one
>>      instruction every 2^N ns of virtual time. If ``auto`` is specified
>>      then the virtual cpu speed will be automatically adjusted to keep
>>      virtual time within a few seconds of real time.
>>  
>> -    When the virtual cpu is sleeping, the virtual time will advance at
>> -    default speed unless ``sleep=on|off`` is specified. With
>> -    ``sleep=on|off``, the virtual time will jump to the next timer
>> -    deadline instantly whenever the virtual cpu goes to sleep mode and
>> -    will not advance if no timer is enabled. This behavior give
>> -    deterministic execution times from the guest point of view.
>> -
>>      Note that while this option can give deterministic behavior, it does
>>      not provide cycle accurate emulation. Modern CPUs contain
>>      superscalar out of order cores with complex cache hierarchies. The
>>      number of instructions executed often has little or no correlation
>>      with actual performance.
>>  
>> +    When the virtual cpu is sleeping, the virtual time will advance at
>> +    default speed unless ``sleep=on`` is specified. With
>> +    ``sleep=on``, the virtual time will jump to the next timer
>> +    deadline instantly whenever the virtual cpu goes to sleep mode and
>> +    will not advance if no timer is enabled. This behavior gives
>> +    deterministic execution times from the guest point of view.
>> +    The default if icount is enabled is ``sleep=off``.
>> +    ``sleep=on`` cannot be used together with either ``shift=auto``
>> +    or ``align=on``.
>> +
>>      ``align=on`` will activate the delay algorithm which will try to
>>      synchronise the host clock and the virtual clock. The goal is to
>>      have a guest running at the real frequency imposed by the shift
>> @@ -3989,15 +3993,17 @@ SRST
>>      ``shift`` is ``auto``. Note: The sync algorithm will work for those
>>      shift values for which the guest clock runs ahead of the host clock.
>>      Typically this happens when the shift value is high (how high
>> -    depends on the host machine).
>> +    depends on the host machine). The default if icount is enabled
>> +    is ``align=off``.
>>  
>> -    When ``rr`` option is specified deterministic record/replay is
>> -    enabled. Replay log is written into filename file in record mode and
>> -    read from this file in replay mode.
>> -
>> -    Option rrsnapshot is used to create new vm snapshot named snapshot
>> -    at the start of execution recording. In replay mode this option is
>> -    used to load the initial VM state.
>> +    When the ``rr`` option is specified deterministic record/replay is
>> +    enabled. The ``rrfile=`` option must also be provided to
>> +    specify the path to the replay log. In record mode data is written
>> +    to this file, and in replay mode it is read back.
>> +    If the ``rrsnapshot`` option is given then it specifies a VM snapshot
>> +    name. In record mode, a new VM snapshot with the given name is created
>> +    at the start of execution recording. In replay mode this option
>> +    specifies the snapshot name used to load the initial VM state.
>>  ERST
>>  
>>  DEF("watchdog", HAS_ARG, QEMU_OPTION_watchdog, \
>>
> 
> Applied to my linux-user-for-6.0 branch.

s/linux-user-for-6.0/trivial-patches/