[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option
From: |
Markus Armbruster |
Subject: |
Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option |
Date: |
Tue, 07 Feb 2023 16:56:33 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Peter Maydell <peter.maydell@linaro.org> writes:
> The command line option '-singlestep' and its HMP equivalent
> the 'singlestep' command are very confusingly named, because
> they have nothing to do with single-stepping the guest (either
> via the gdb stub or by emulation of guest CPU architectural
> debug facilities). What they actually do is put TCG into a
> mode where it puts only one guest instruction into each
> translation block. This is useful for some circumstances
> such as when you want the -d debug logging to be easier to
> interpret, or if you have a finicky guest binary that wants
> to see interrupts delivered at something other than the end
> of a basic block.
>
> The confusing name is made worse by the fact that our
> documentation for these is so minimal as to be useless
> for telling users what they really do.
>
> This series:
> * renames the 'singlestep' global variable to 'one_insn_per_tb'
> * Adds new '-one-insn-per-tb' command line options and a
> HMP 'one-insn-per-tb' command
> * Documents the '-singlestep' options and 'singlestep'
> HMP command as deprecated synonyms for the new ones
>
> It does not do anything about the other place where we surface
> 'singlestep', which is in the QMP StatusInfo object returned by the
> 'query-status' command. This is incorrectly documented as "true if
> VCPUs are in single-step mode" and "singlestep is enabled through
> the GDB stub", because what it's actually returning is the
> one-insn-per-tb state.
>
> Things I didn't bother with because this is only an RFC but
> will do if it turns into a non-RFC patchset:
> * test the bsd-user changes :-)
> * add text to deprecated.rst
>
> So, questions:
>
> (1) is this worth bothering with at all? We could just
> name our global variable etc better, and document what
> -singlestep actually does, and not bother with the new
> names for the options/commands.
The feature is kind of esoteric. Rather weak excuse for not fixing bad
UI, in my opinion. Weaker still since you already did a good part of
the actual work.
> (2) if we do do it, do we retain the old name indefinitely,
> or actively put it on the deprecate-and-drop list?
By "the old name", you mean the CLI option name, right?
I'd prefer deprecate and drop.
> (3) what should we do about the HMP StatusInfo object?
> I'm not sure how we handle compatibility for HMP.
Uh, you mean *QMP*, don't you?
As you wrote above, StatusInfo is returned by query-status, which is a
stable interface. Changes to members therefore require the usual
deprecation grace period. We'd add a new member with a sane name, and
deprecate the old one.
The matching HMP command is "info status". It shows member @singlestep
as " (single step mode)". Changing that is fine; HMP is not a stable
interface.
- [RFC PATCH 2/5] linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep', (continued)
- [RFC PATCH 2/5] linux-user: Add '-one-insn-per-tb' option equivalent to '-singlestep', Peter Maydell, 2023/02/06
- [RFC PATCH 3/5] bsd-user: Add '-one-insn-per-tb' option equivalent to '-singlestep', Peter Maydell, 2023/02/06
- [RFC PATCH 1/5] Rename the singlestep global variable to one_insn_per_tb, Peter Maydell, 2023/02/06
- [RFC PATCH 4/5] softmmu: Add '-one-insn-per-tb' option equivalent to '-singlestep', Peter Maydell, 2023/02/06
- Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option, Richard Henderson, 2023/02/06
- Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option, Thomas Huth, 2023/02/06
- Re: [RFC PATCH 0/5] Deprecate/rename singlestep command line option,
Markus Armbruster <=