[PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Elliott Mitchell]

The better to encourage moving to setting via string mode names.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
I'm not actually sure what tsc_mode==0 does.  I didn't find other
references, so I'm unsure how that should be modified.
---
 docs/man/xen-tscmode.7.pod | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/docs/man/xen-tscmode.7.pod b/docs/man/xen-tscmode.7.pod
index 1d81a3fe18..80aea77f76 100644
--- a/docs/man/xen-tscmode.7.pod
+++ b/docs/man/xen-tscmode.7.pod
@@ -63,19 +63,19 @@ The non-default choices for tsc_mode are:
 
 =over 4
 
-=item * B<tsc_mode=1> (always emulate).
+=item * B<tsc_mode='always_emulate'> (always emulate).
 
 All rdtsc instructions are emulated; this is the best choice when
 TSC-sensitive apps are running and it is necessary to understand
 worst-case performance degradation for a specific hardware environment.
 
-=item * B<tsc_mode=2> (never emulate).
+=item * B<tsc_mode='native'> (never emulate).
 
 This is the same as prior to Xen 4.0 and is the best choice if it
 is certain that all apps running in this VM are TSC-resilient and
 highest performance is required.
 
-=item * B<tsc_mode=3> (PVRDTSCP).
+=item * B<tsc_mode='native_paravirt'> (PVRDTSCP).
 
 This mode has been removed.
 
@@ -200,10 +200,10 @@ per second per processor), this performance degradation is not noticeable
 OS-provided alternatives (e.g. Linux's gettimeofday).  For environments
 where it is certain that all apps are TSC-resilient (e.g.
 "TSC-safeness" is not necessary) and highest performance is a
-requirement, TSC emulation may be entirely disabled (tsc_mode==2).
+requirement, TSC emulation may be entirely disabled (tsc_mode='native').
 
-The default mode (tsc_mode==0) checks TSC-safeness of the underlying
-hardware on which the virtual machine is launched.  If it is
+The default mode (tsc_mode='always_emulate') checks TSC-safeness of the
+underlying hardware on which the virtual machine is launched.  If it is
 TSC-safe, rdtsc will execute at hardware speed; if it is not, rdtsc
 will be emulated.  Once a virtual machine is save/restored or migrated,
 however, there are two possibilities: TSC remains native IF the source
@@ -213,12 +213,13 @@ is emulated.  Note that, though emulated, the "apparent" TSC frequency
 will be the TSC frequency of the initial physical machine, even after
 migration.
 
-Finally, tsc_mode==1 always enables TSC emulation, regardless of
+Finally, tsc_mode='always_emulate' always enables TSC emulation, regardless of
 the underlying physical hardware. The "apparent" TSC frequency will
 be the TSC frequency of the initial physical machine, even after migration.
 This mode is useful to measure any performance degradation that
 might be encountered by a tsc_mode==0 domain after migration occurs,
-or a tsc_mode==3 domain when it is running on TSC-unsafe hardware.
+or a tsc_mode='native_paravirt' domain when it is running on
+TSC-unsafe hardware.
 
 Note that while Xen ensures that an emulated TSC is "safe" across migration,
 it does not ensure that it continues to tick at the same rate during
-- 
2.30.2

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
> The better to encourage moving to setting via string mode names.

The numeric values needs to remain documented, otherwise interpreting
pre-existing configs (that may use them) will be tricky.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Elliott Mitchell]

On Fri, Jul 14, 2023 at 03:24:59AM +0200, Marek Marczykowski-Górecki wrote:
> On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
> > The better to encourage moving to setting via string mode names.
> 
> The numeric values needs to remain documented, otherwise interpreting
> pre-existing configs (that may use them) will be tricky.

Problem is the way it is documented tends to encourage continued use of
the numeric values (notice how reports irt Zen 4 mention "tsc_mode=1").

`parse_config_data()` suggests the appropriate string value, so nominally
that should take care of older configurations.  If "xen-tscmode" really
needs to continue mentioning the numeric value, it should be in
parentheses and with "old value" to suggest moving away from that.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Jan Beulich]

On 14.07.2023 05:44, Elliott Mitchell wrote:
> On Fri, Jul 14, 2023 at 03:24:59AM +0200, Marek Marczykowski-Górecki wrote:
>> On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
>>> The better to encourage moving to setting via string mode names.
>>
>> The numeric values needs to remain documented, otherwise interpreting
>> pre-existing configs (that may use them) will be tricky.
> 
> Problem is the way it is documented tends to encourage continued use of
> the numeric values (notice how reports irt Zen 4 mention "tsc_mode=1").
> 
> `parse_config_data()` suggests the appropriate string value, so nominally
> that should take care of older configurations.  If "xen-tscmode" really
> needs to continue mentioning the numeric value, it should be in
> parentheses and with "old value" to suggest moving away from that.

I'm not sure about "old" (we can't change the values without breaking
backwards compatibility), but the numeric values will want mentioning
alongside their spelled out names.

As to mode 0 - that's the default mode, engaging emulation as
necessary. See xen/include/public/arch-x86/cpuid.h

Jan

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Elliott Mitchell]

On Fri, Jul 14, 2023 at 09:21:59AM +0200, Jan Beulich wrote:
> On 14.07.2023 05:44, Elliott Mitchell wrote:
> > On Fri, Jul 14, 2023 at 03:24:59AM +0200, Marek Marczykowski-Górecki wrote:
> >> On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
> >>> The better to encourage moving to setting via string mode names.
> >>
> >> The numeric values needs to remain documented, otherwise interpreting
> >> pre-existing configs (that may use them) will be tricky.
> > 
> > Problem is the way it is documented tends to encourage continued use of
> > the numeric values (notice how reports irt Zen 4 mention "tsc_mode=1").
> > 
> > `parse_config_data()` suggests the appropriate string value, so nominally
> > that should take care of older configurations.  If "xen-tscmode" really
> > needs to continue mentioning the numeric value, it should be in
> > parentheses and with "old value" to suggest moving away from that.
> 
> I'm not sure about "old" (we can't change the values without breaking
> backwards compatibility), but the numeric values will want mentioning
> alongside their spelled out names.

Then why is there a warning message about numeric tsc_mode in
`parse_config_data()`?

"WARNING: specifying \"tsc_mode\" as an integer is deprecated. "
"Please use the named parameter variant. %s%s%s\n"

Declaring them deprecated suggests it could be removed at some future
point.  This message was added at af3b530c03, over a decade ago.

Though I suspect `tsc_mode` hasn't been heavily used since no one ever
bothered to remove the debugging message.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Jan Beulich]

On 18.07.2023 22:58, Elliott Mitchell wrote:
> On Fri, Jul 14, 2023 at 09:21:59AM +0200, Jan Beulich wrote:
>> On 14.07.2023 05:44, Elliott Mitchell wrote:
>>> On Fri, Jul 14, 2023 at 03:24:59AM +0200, Marek Marczykowski-Górecki wrote:
>>>> On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
>>>>> The better to encourage moving to setting via string mode names.
>>>>
>>>> The numeric values needs to remain documented, otherwise interpreting
>>>> pre-existing configs (that may use them) will be tricky.
>>>
>>> Problem is the way it is documented tends to encourage continued use of
>>> the numeric values (notice how reports irt Zen 4 mention "tsc_mode=1").
>>>
>>> `parse_config_data()` suggests the appropriate string value, so nominally
>>> that should take care of older configurations.  If "xen-tscmode" really
>>> needs to continue mentioning the numeric value, it should be in
>>> parentheses and with "old value" to suggest moving away from that.
>>
>> I'm not sure about "old" (we can't change the values without breaking
>> backwards compatibility), but the numeric values will want mentioning
>> alongside their spelled out names.
> 
> Then why is there a warning message about numeric tsc_mode in
> `parse_config_data()`?

I'm afraid this question can only be answered by whoever was involved
in adding the warning.

Jan

> "WARNING: specifying \"tsc_mode\" as an integer is deprecated. "
> "Please use the named parameter variant. %s%s%s\n"
> 
> Declaring them deprecated suggests it could be removed at some future
> point.  This message was added at af3b530c03, over a decade ago.
> 
> Though I suspect `tsc_mode` hasn't been heavily used since no one ever
> bothered to remove the debugging message.
> 
> 

Re: [PATCH] docs/xen-tscmode: remove mention of numeric tsc_mode= values

[Elliott Mitchell]

On Wed, Jul 19, 2023 at 08:24:15AM +0200, Jan Beulich wrote:
> On 18.07.2023 22:58, Elliott Mitchell wrote:
> > On Fri, Jul 14, 2023 at 09:21:59AM +0200, Jan Beulich wrote:
> >> On 14.07.2023 05:44, Elliott Mitchell wrote:
> >>> On Fri, Jul 14, 2023 at 03:24:59AM +0200, Marek Marczykowski-Górecki wrote:
> >>>> On Thu, Jul 13, 2023 at 05:16:40PM -0700, Elliott Mitchell wrote:
> >>>>> The better to encourage moving to setting via string mode names.
> >>>>
> >>>> The numeric values needs to remain documented, otherwise interpreting
> >>>> pre-existing configs (that may use them) will be tricky.
> >>>
> >>> Problem is the way it is documented tends to encourage continued use of
> >>> the numeric values (notice how reports irt Zen 4 mention "tsc_mode=1").
> >>>
> >>> `parse_config_data()` suggests the appropriate string value, so nominally
> >>> that should take care of older configurations.  If "xen-tscmode" really
> >>> needs to continue mentioning the numeric value, it should be in
> >>> parentheses and with "old value" to suggest moving away from that.
> >>
> >> I'm not sure about "old" (we can't change the values without breaking
> >> backwards compatibility), but the numeric values will want mentioning
> >> alongside their spelled out names.
> > 
> > Then why is there a warning message about numeric tsc_mode in
> > `parse_config_data()`?
> 
> I'm afraid this question can only be answered by whoever was involved
> in adding the warning.

I already tracked that down, commit af3b530c03 by Ian Campbell.  Appears
Ian Campbell has moved onto other things and may not be readily
accessible.

The messages themselves seem to suggest >10 years ago Ian Campbell wanted
to get rid of the numeric values for tsc_mode.  Problem is one debug
string was left in and the documentation doesn't discourage the numeric
values.  As such they may still be heavily used.

What needs to happen is a decision of which direction to push needs to be
made.  Then that direction needs to be consistently pushed.

Notice the immediately prior message trying to get rid of the
printf-debugging.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445