new config option vtsc_tolerance_khz to avoid TSC emulation

Quoted 1 line

Quoted 1 line · On 07.06.18 at 15:08, <[email protected]> wrote:

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Existing padding fields are reused to store vtsc_khz_tolerance as u16. The padding is sent as zero in write_tsc_info to the receving host. The padding is undefined if the changed code runs as receiver. handle_tsc_info has no code to verify that padding is indeed zero. Due to the lack of a version field it is impossible to know if the sender already has the newly introduced vtsc_tolerance field. In the worst case the receiving domU will get an unemulated TSC.

Signed-off-by Olaf Hering <[email protected]>

Reviewed-by Wei Liu <[email protected]> (v07/v08)

Reviewed-by Jan Beulich <[email protected]> (v08)

Hmm, I find this description concerning. Is the field reliably zero when coming from older Xen, or is it not?

Quoted 1 line

Quoted 1 line · On 07.06.18 at 15:47, <[email protected]> wrote:

Am Thu, 07 Jun 2018 07:31:26 -0600
schrieb "Jan Beulich" <[email protected]>:

Quoted 2 lines

Hmm, I find this description concerning. Is the field reliably zero when coming from older Xen, or is it not?

It depends on how old.
If the other sending side has write_tsc_info from 4.6+, then _res1 is zero. If it is older, the XC_SAVE_ID_TSC_INFO is eventually not handled anymore. At least I could not find the relevant code, even in staging-4.6. But then, it is unlikely that migration from 4.5 to 4.6 did not work.

Also, in general the input comes from "remote". So it is untrusted.

The re-use of the field is acceptable only if all existing senders reliably fill zero in there.

Quoted 1 line

Quoted 1 line · On 07.06.18 at 16:49, <[email protected]> wrote:

Am Thu, 07 Jun 2018 08:44:41 -0600
schrieb "Jan Beulich" <[email protected]>:

Quoted 2 lines

The re-use of the field is acceptable only if all existing senders reliably fill zero in there.

How do we know all senders? I just know about write_tsc_info from xen-4.6+.

Quoted 6 lines

Quoted 1 line

Quoted 1 line · On 07.06.18 at 16:49, <[email protected]> wrote:

Am Thu, 07 Jun 2018 08:44:41 -0600
schrieb "Jan Beulich" <[email protected]>:

Quoted 2 lines

The re-use of the field is acceptable only if all existing senders reliably fill zero in there.

How do we know all senders? I just know about write_tsc_info from xen-4.6+.

I don't think we care about senders other than ones using libxc, so by knowing whether all libxc versions conform to the requirement, we could declare we're fine.

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

Am Thu, 7 Jun 2018 15:08:29 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 4 lines

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

Andrew,
do you have any comments on this patch/approach?

Am Tue, 26 Jun 2018 19:11:13 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 6 lines

Am Thu, 7 Jun 2018 15:08:29 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 4 lines

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

Quoted 2 lines

Andrew,
do you have any comments on this patch/approach?

Andrew,
any comments on this patch?
https://lists.xen.org/archives/html/xen-devel/2018-06/msg00398.html

this patch was not applied yet, even after a few "pings".

Am Thu, 13 Sep 2018 09:39:13 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 1 line

this patch was not applied yet, even after a few "pings".

No reaction since months.
So scrap that patch, just in case it is still part of someones to-consider queue.

Quoted 9 lines · On 07.06.18 at 16:49, <[email protected]> wrote:

Quoted 7 lines

Am Thu, 07 Jun 2018 08:44:41 -0600
schrieb "Jan Beulich" <[email protected]>:

Quoted 2 lines

The re-use of the field is acceptable only if all existing senders reliably fill zero in there.

How do we know all senders? I just know about write_tsc_info from xen-4.6+.

Quoted 3 lines

I don't think we care about senders other than ones using libxc, so by knowing whether all libxc versions conform to the requirement, we could declare we're fine.

Yes, I think we are fine. And if migration from pre-4.6 is supported anyway is another question. Most likely the answer is no.

Quoted 1 line

Quoted 1 line · On 01.10.18 at 12:52, <[email protected]> wrote:

Olaf Hering writes ("Re: [PATCH v9] new config option vtsc_tolerance_khz to avoid TSC emulation"):

Quoted 6 lines

Am Thu, 13 Sep 2018 09:39:13 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 1 line

this patch was not applied yet, even after a few "pings".

No reaction since months.
So scrap that patch, just in case it is still part of someones to-consider

queue.

I think it would be worth exploring whether this patch could be applied without an explicit ack from Andrew.

I confess I ignored all the previous mails because they all started
Andrew,
or
Andrew, Lars,
so I assumed that you didn't want attention from other maintainers/committers.

Now that I look at the thread it is difficult for me to see the wood for the trees but I don't see unanswered concerns.

Problem is - discussion around this was (iirc) happening not only on the list, but also on irc (including perhaps private chats).

It was for that reason that I made my R-b conditional upon Andrew at least giving an informal go-ahead (otherwise, together with Wei's R-b, the patch could have gone in).

[lots of stuff]

[Olaf:]

Quoted 3 lines

Existing padding fields are reused to store vtsc_khz_tolerance as u16. The padding is sent as zero in write_tsc_info to the receving host. The padding is undefined if the changed code runs as receiver.

I'm not sure what you mean by this final sentence.

Quoted 4 lines

handle_tsc_info has no code to verify that padding is indeed zero. Due to the lack of a version field it is impossible to know if the sender already has the newly introduced vtsc_tolerance field. In the worst case the receiving domU will get an unemulated TSC.

The lack of padding verification is deliberate, for forwards compatibility.  Why does the sending code matter?  One way or another, if the field is 0, the option wasn't present or wasn't configured. Neither of these situations affect the decision-making that the receiving side needs to perform.

Quoted 3 lines

Signed-off-by Olaf Hering <[email protected]>

Reviewed-by Wei Liu <[email protected]> (v07/v08)

Reviewed-by Jan Beulich <[email protected]> (v08)

I'm still -0.5 for this patch.  I can appreciate why you want it, but it is a gross hack which only works when you don't skew time more than NTP in the guest can cope with.

My gut feeling is that there will be other
more subtle fallout.

Quoted 5 lines

Quoted 5 lines

handle_tsc_info has no code to verify that padding is indeed zero. Due to the lack of a version field it is impossible to know if the sender already has the newly introduced vtsc_tolerance field. In the worst case the receiving domU will get an unemulated TSC.

Quoted 5 lines

The lack of padding verification is deliberate, for forwards compatibility.  Why does the sending code matter?  One way or another, if the field is 0, the option wasn't present or wasn't configured. Neither of these situations affect the decision-making that the receiving side needs to perform.

We are talking here about an unused field that is (supposedly?) always sent as zero ?

AIUI:

In the new code the semantics of zero is "do not allow any tolerance". The old code handles this correctly. The issue is with migrations from new code to old: the tolerance value will silently ignored.

Quoted 21 lines

Quoted 1 line

Quoted 1 line · On 01.10.18 at 12:52, <[email protected]> wrote:

Olaf Hering writes ("Re: [PATCH v9] new config option vtsc_tolerance_khz to avoid TSC emulation"):

Quoted 6 lines

Am Thu, 13 Sep 2018 09:39:13 +0200
schrieb Olaf Hering <[email protected]>:

Quoted 1 line

this patch was not applied yet, even after a few "pings".

No reaction since months.
So scrap that patch, just in case it is still part of someones to-consider

queue.

I think it would be worth exploring whether this patch could be applied without an explicit ack from Andrew.

I confess I ignored all the previous mails because they all started
Andrew,
or
Andrew, Lars,
so I assumed that you didn't want attention from other maintainers/committers.

Quoted 2 lines

Now that I look at the thread it is difficult for me to see the wood for the trees but I don't see unanswered concerns.

Problem is - discussion around this was (iirc) happening not only on the list, but also on irc (including perhaps private chats). It was for that reason that I made my R-b conditional upon Andrew at least giving an informal go-ahead (otherwise, together with Wei's R-b, the patch could have gone in).

Besides the question of correctness from the perspective of guests (which imo is not a problem as the feature needs to be actively enabled, and I think we have no reason to keep admins from breaking their guests if they really mean to),

I think the main concern
was with the way migration of the new value was implemented. But I really have to defer to Andrew for that, irrespective of him not having responded (on the list) to prior pings.

Quoted 1 line

Quoted 1 line · On 01.10.18 at 15:38, <[email protected]> wrote:

Quoted 4 lines · Jan Beulich

I think the main concern
was with the way migration of the new value was implemented. But I really have to defer to Andrew for that, irrespective of him not having responded (on the list) to prior pings.

Is Andrew really the only person who knows enough about migration to give this the thumbs-up?

Quoted 7 lines

Quoted 1 line

Quoted 1 line · On 01.10.18 at 15:38, <[email protected]> wrote:

Quoted 4 lines · Jan Beulich

I think the main concern
was with the way migration of the new value was implemented. But I really have to defer to Andrew for that, irrespective of him not having responded (on the list) to prior pings.

Quoted 2 lines

Is Andrew really the only person who knows enough about migration to give this the thumbs-up?

That's not the point here, at least afaic: He had voiced _some_ concern on an earlier version. In such a case it is, I think, only appropriate to wait with committing until there was indication that the concerns were sufficiently addressed (verbally or by adjustments to the code).

Quoted 7 lines · Jan Beulich

Quoted 7 lines

Quoted 1 line

Quoted 1 line · On 01.10.18 at 15:38, <[email protected]> wrote:

Quoted 4 lines · Jan Beulich

I think the main concern
was with the way migration of the new value was implemented. But I really have to defer to Andrew for that, irrespective of him not having responded (on the list) to prior pings.

Quoted 8 lines

Quoted 2 lines

Is Andrew really the only person who knows enough about migration to give this the thumbs-up?

That's not the point here, at least afaic: He had voiced _some_ concern on an earlier version. In such a case it is, I think, only appropriate to wait with committing until there was indication that the concerns were sufficiently addressed (verbally or by adjustments to the code).

Right -- but it's not your job to make sure the migration stuff is properly addressed; it's Wei and Ian's job. Wei's R-b was a statement from him that the code was good; when Andy questioned that, I think it was then *Wei's* job to address the question, not yours or Andy's (or even Olaf's). If Wei says, "I've considered Andy's objections and I think the patch is fine as-is", then it can be checked in (given a reasonable amount of time for Andy to respond); and Wei can own whatever consequences there are.

-George

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Existing padding fields are reused to store vtsc_khz_tolerance as u16. The padding is sent as zero in write_tsc_info to the receving host. The padding is undefined if the changed code runs as receiver.

handle_tsc_info has no code to verify that padding is indeed zero. Due to the lack of a version field it is impossible to know if the sender already has the newly introduced vtsc_tolerance field. In the worst case the receiving domU will get an unemulated TSC.

Signed-off-by Olaf Hering <[email protected]>

Reviewed-by Wei Liu <[email protected]> (v07/v08)

Reviewed-by Jan Beulich <[email protected]> (v08)

docs/man/xen-tscmode.pod.7 +3 -0

index 3bbc96f201..122ae36679 100644
@@ -99,6 +99,9 @@ whether or not the VM has been saved/restored/migrated
 
 =back
 
+If the tsc_mode is set to "default" the decision to emulate TSC can be
+tweaked further with the "vtsc_tolerance_khz" option.
+
 To understand this in more detail, the rest of this document must
 be read.
 

docs/man/xen-tscmode.pod.7 +11 -0

 This means
+also several otherwise identical systems can have a slightly different
+TSC frequency. As a result TSC access will be emulated if a domU is
+migrated from one host to another, identical host. To avoid the
+performance impact of TSC emulation a certain tolerance of the measured
+host TSC frequency can be specified with "vtsc_tolerance_khz". If the
+measured "cpu_khz" value is within the tolerance range, TSC access
+remains native. Otherwise it will be emulated. This allows to migrate
+domUs between identical hardware. If the domU will be migrated to a
+different kind of hardware, say from a "2.3GHz" to a "2.5GHz" system,
+TSC will be emualted to maintain the TSC frequency expected by the domU.
+
 For environments where both TSC-safeness AND highest performance
 even across migration is a requirement, application code can be specially

modified to use an algorithm explicitly designed into Xen for this purpose.

docs/man/xl.cfg.pod.5.in +0 -0

index 47d88243b1..995277794f 100644
@@ -1898,6 +1898,16 @@ determined in a similar way to that of B<default> TSC mode.
 
 Please see B<xen-tscmode(7)> for more information on this option.

docs/man/xl.cfg.pod.5.in +1 -0

+
 =item B<localtime=BOOLEAN>
 
 Set the real time clock to local time or to UTC. False (0) by default,

docs/specs/libxc-migration-stream.pandoc +2 -2

index 73421ff393..0d0f17edb1 100644
@@ -3,7 +3,7 @@
   Andrew Cooper <<[email protected]>>
   Wen Congyang <<[email protected]>>
   Yang Hongyang <<[email protected]>>
-% Revision 2
+% Revision 3
 
 Introduction
 ============

@@ -472,7 +472,7 @@ XEN\_DOMCTL\_{get,set}tscinfo hypercall sub-ops.
     +------------------------+------------------------+
     | nsec                                            |
     +------------------------+------------------------+
-    | incarnation            | (reserved)             |
+    | incarnation            | tolerance | (reserved) |
     +------------------------+------------------------+
 
 --------------------------------------------------------------------

docs/specs/libxc-migration-stream.pandoc +0 -0

 --------------------------------------------------------------------
 
 \clearpage

xen/arch/x86/time.c +7 -0

index c342d00732..4a9c43b718 100644
@@ -2148,8 +2153,25 @@ void tsc_set_info(struct domain *d,
          * When a guest is created, gtsc_khz is passed in as zero, making
          * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation.
          */
+        disable_vtsc = d->arch.tsc_khz == cpu_khz;
+
+        if ( tsc_mode == TSC_MODE_DEFAULT && gtsc_khz &&
+             d->arch.vtsc_tolerance_khz )
+        {
+            long khz_diff;
+

Quoted 1 line

Quoted 1 line · On 01.10.18 at 14:39, <[email protected]> wrote:

Quoted 10 lines · Olaf Hering

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

Quoted 7 lines

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Do you have any further details of the systems involved? If they are identical systems, they should all have the same real TSC frequency, and its a known issue that Xen isn't very good at working out the frequency. TBH, fixing that would be far better overall.

Quoted 19 lines

Quoted 1 line

Quoted 1 line · On 01.10.18 at 14:39, <[email protected]> wrote:

Quoted 17 lines · Olaf Hering

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Quoted 4 lines

Do you have any further details of the systems involved? If they are identical systems, they should all have the same real TSC frequency, and its a known issue that Xen isn't very good at working out the frequency. TBH, fixing that would be far better overall.

Are you convinced all parts match their nominal frequency without _any_ deviation?

If that was the case, we could indeed use CPUID leaves 0x15 / 0x16 output, if available.

But I very much doubt this.
As an example, here's what bare metal Linux says on my newest system:

tsc: Detected 2600.000 MHz processor
tsc: Refined TSC clocksource calibration: 2591.990 MHz

Xen figures:

(XEN) Detected 2592.107 MHz processor.

And then after another re-boot bare metal Linux again

tsc: Refined TSC clocksource calibration: 2592.008 MHz

Quoted 1 line

Quoted 1 line · On 01.10.18 at 17:17, <[email protected]> wrote:

Quoted 19 lines · Jan Beulich

Quoted 19 lines

Quoted 1 line

Quoted 1 line · On 01.10.18 at 14:39, <[email protected]> wrote:

Quoted 17 lines · Olaf Hering

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Quoted 6 lines

Quoted 4 lines

Do you have any further details of the systems involved? If they are identical systems, they should all have the same real TSC frequency, and its a known issue that Xen isn't very good at working out the frequency. TBH, fixing that would be far better overall.

Are you convinced all parts match their nominal frequency without _any_ deviation?

That is the intent of publishing the numbers, yes.

Quoted 2 lines

If that was the case, we could indeed use CPUID leaves 0x15 / 0x16 output, if available.

We very much should be doing this. There are also model-specific ways of getting the same data on older processors.

Quoted 13 lines

But I very much doubt this.
As an example, here's what bare metal Linux says on my newest system:

tsc: Detected 2600.000 MHz processor
tsc: Refined TSC clocksource calibration: 2591.990 MHz

Xen figures:

(XEN) Detected 2592.107 MHz processor.

And then after another re-boot bare metal Linux again

Quoted 1 line

tsc: Refined TSC clocksource calibration: 2592.008 MHz

What is surprising here? The calibration loop is not 100% accurate and cannot be made to be perfect.

The fact that Linux and Xen agree is because they basically share the same calibration algorithm - not that the processor is really running at 2592MHz.

For one, all calibration options will read slow by the amount
of time it takes an interrupt to propagate through the system fabric, and there is basically nothing software can do to account for this.

Quoted 7 lines

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Do you have any further details of the systems involved?  If they are identical systems, they should all have the same real TSC frequency, and its a known issue that Xen isn't very good at working out the frequency.

Quoted 10 lines

Add an option to control when vTSC emulation will be activated for a domU with tsc_mode=default. Without such option each TSC access from domU will be emulated, which causes a significant perfomance drop for workloads that make use of rdtsc.

One option to avoid the TSC option is to run domUs with tsc_mode=native. This has the drawback that migrating a domU from a "2.3GHz" class host to a "2.4GHz" class host may change the rate at wich the TSC counter increases, the domU may not be prepared for that.

Quoted 7 lines

With the new option the host admin can decide how a domU should behave when it is migrated across systems of the same class. Since there is always some jitter when Xen calibrates the cpu_khz value, all hosts of the same class will most likely have slightly different values. As a result vTSC emulation is unavoidable. Data collected during the incident which triggered this change showed a jitter of up to 200 KHz across systems of the same class.

Do you have any further details of the systems involved?  If they are identical systems, they should all have the same real TSC frequency, and its a known issue that Xen isn't very good at working out the frequency.  TBH, fixing that would be far better overall.

Quoted 3 lines

Existing padding fields are reused to store vtsc_khz_tolerance as u16. The padding is sent as zero in write_tsc_info to the receving host. The padding is undefined if the changed code runs as receiver.

I'm not sure what you mean by this final sentence.

Quoted 4 lines

handle_tsc_info has no code to verify that padding is indeed zero. Due to the lack of a version field it is impossible to know if the sender already has the newly introduced vtsc_tolerance field. In the worst case the receiving domU will get an unemulated TSC.

The lack of padding verification is deliberate, for forwards compatibility.  Why does the sending code matter?  One way or another, if the field is 0, the option wasn't present or wasn't configured. Neither of these situations affect the decision-making that the receiving side needs to perform.

Quoted 3 lines

Signed-off-by Olaf Hering <[email protected]>

Reviewed-by Wei Liu <[email protected]> (v07/v08)

Reviewed-by Jan Beulich <[email protected]> (v08)

I'm still -0.5 for this patch.  I can appreciate why you want it, but it is a gross hack which only works when you don't skew time more than NTP in the guest can cope with.  My gut feeling is that there will be other more subtle fallout.