diff mbox series

[v10,06/11] x86/time: add a domain context record for tsc_info...

Message ID 20201008185735.29875-7-paul@xen.org (mailing list archive)
State New, archived
Headers show
Series domain context infrastructure | expand

Commit Message

Paul Durrant Oct. 8, 2020, 6:57 p.m. UTC 
From: Paul Durrant <pdurrant@amazon.com>

... and update xen-domctx to dump some information describing the record.

NOTE: Whilst the record is x86 specific, it is visible directly in the common
      header as context record numbers should be unique across all
      architectures.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Ian Jackson <iwj@xenproject.org>
Cc: Wei Liu <wl@xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v10:
 - Re-base
 - Amend the specification now there is one
 - Kept Jan's R-b as the changes are cosmetic

v8:
 - Removed stray blank line

v7:
 - New in v7
---
 docs/specs/domain-context.md | 32 ++++++++++++++++++++++++++++++++
 tools/misc/xen-domctx.c      | 12 ++++++++++++
 xen/arch/x86/time.c          | 30 ++++++++++++++++++++++++++++++
 xen/include/public/save.h    |  8 ++++++++
 4 files changed, 82 insertions(+)

Comments

Andrew Cooper Jan. 25, 2021, 7:24 p.m. UTC | #1 
On 08/10/2020 19:57, Paul Durrant wrote:
> +The record body contains the following fields:
> +
> +| Field         | Description                                   |
> +|---------------|-----------------------------------------------|
> +| `mode`        | 0x00000000: Default (emulate if necessary)    |
> +|               | 0x00000001: Always emulate                    |
> +|               | 0x00000002: Never emulate                     |
> +|               |                                               |
> +| `khz`         | The TSC frequency in kHz                      |
> +|               |                                               |
> +| `nsec`        | Elapsed time in nanoseconds                   |
> +|               |                                               |
> +| `incarnation` | Incarnation (counter indicating how many      |
> +|               | times the TSC value has been set)             |

It is how many set_tsc_info() (hyper)calls have been made, not how many
times the guest has written to the TSC MSR.

That said, it is totally useless now that PVRDTSCP mode has gone, other
than the fact that it appears in guest CPUID as an approximation of "how
many times has this domain been migrated".

I.e. its a number you'll want to actively squash in your usecase.

I'm not sure whether to suggest dropping the field entirely, or not.  I
highly doubt any user exists - IIRC, it was specifically for PVRDTSCP
userspace to notice that the frequency may have changed, and to
re-adjust its calculations.

> diff --git a/xen/include/public/save.h b/xen/include/public/save.h
> index bccbaadd0b..86881864cf 100644
> --- a/xen/include/public/save.h
> +++ b/xen/include/public/save.h
> @@ -50,6 +50,7 @@ enum {
>      DOMAIN_CONTEXT_END,
>      DOMAIN_CONTEXT_START,
>      DOMAIN_CONTEXT_SHARED_INFO,
> +    DOMAIN_CONTEXT_TSC_INFO,

At a minimum, this wants an /* x86 only */ comment.  Possibly an X86 infix.

~Andrew
diff mbox series

Patch

diff --git a/docs/specs/domain-context.md b/docs/specs/domain-context.md
index 95e9f9d1ab..8aa3466d96 100644
--- a/docs/specs/domain-context.md
+++ b/docs/specs/domain-context.md
@@ -155,6 +155,38 @@  The record body contains the following fields:
 | `buffer`    | The shared info (`length` being architecture    |
 |             | dependent[4])                                   |
 
+### TSC_INFO
+
+```
+    0       1       2       3       4       5       6       7    octet
++-------+-------+-------+-------+-------+-------+-------+-------+
+| type == 3                     | instance == 0                 |
++-------------------------------+-------------------------------+
+| length == 20                                                  |
++-------------------------------+-------------------------------+
+| mode                          | khz                           |
++-------------------------------+-------------------------------+
+| nsec                                                          |
++-------------------------------+-------------------------------+
+| incarnation                   |
++-------------------------------+
+```
+
+The record body contains the following fields:
+
+| Field         | Description                                   |
+|---------------|-----------------------------------------------|
+| `mode`        | 0x00000000: Default (emulate if necessary)    |
+|               | 0x00000001: Always emulate                    |
+|               | 0x00000002: Never emulate                     |
+|               |                                               |
+| `khz`         | The TSC frequency in kHz                      |
+|               |                                               |
+| `nsec`        | Elapsed time in nanoseconds                   |
+|               |                                               |
+| `incarnation` | Incarnation (counter indicating how many      |
+|               | times the TSC value has been set)             |
+
 * * *
 
 [1] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/designs/non-cooperative-migration.md
diff --git a/tools/misc/xen-domctx.c b/tools/misc/xen-domctx.c
index 5ea6de50d1..1a5dfb9d5a 100644
--- a/tools/misc/xen-domctx.c
+++ b/tools/misc/xen-domctx.c
@@ -136,6 +136,17 @@  static void dump_shared_info(void)
 #undef GET_FIELD_PTR
 }
 
+static void dump_tsc_info(void)
+{
+    struct domain_context_tsc_info *t;
+
+    GET_PTR(t);
+
+    printf("    TSC_INFO: mode: %u incarnation: %u\n"
+           "              khz %u nsec: %"PRIu64"\n",
+           t->mode, t->incarnation, t->khz, t->nsec);
+}
+
 static void dump_end(void)
 {
     struct domain_context_end *e;
@@ -225,6 +236,7 @@  int main(int argc, char **argv)
             {
             case DOMAIN_CONTEXT_START: dump_start(); break;
             case DOMAIN_CONTEXT_SHARED_INFO: dump_shared_info(); break;
+            case DOMAIN_CONTEXT_TSC_INFO: dump_tsc_info(); break;
             case DOMAIN_CONTEXT_END: dump_end(); break;
             default:
                 printf("Unknown type %u: skipping\n", rec->type);
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 8938c0f435..aec4bfb0f3 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -26,6 +26,7 @@ 
 #include <xen/symbols.h>
 #include <xen/keyhandler.h>
 #include <xen/guest_access.h>
+#include <xen/save.h>
 #include <asm/io.h>
 #include <asm/iocap.h>
 #include <asm/msr.h>
@@ -2451,6 +2452,35 @@  int tsc_set_info(struct domain *d,
     return 0;
 }
 
+static int save_tsc_info(struct domain *d, struct domain_ctxt_state *c,
+                         bool dry_run)
+{
+    struct domain_context_tsc_info t = {};
+
+    if ( !dry_run )
+        tsc_get_info(d, &t.mode, &t.nsec, &t.khz, &t.incarnation);
+
+    return domain_save_ctxt_rec(c, DOMAIN_CONTEXT_TSC_INFO, 0, &t, sizeof(t));
+}
+
+static int load_tsc_info(struct domain *d, struct domain_ctxt_state *c)
+{
+    struct domain_context_tsc_info t;
+    unsigned int i;
+    int rc;
+
+    rc = domain_load_ctxt_rec(c, DOMAIN_CONTEXT_TSC_INFO, &i, &t, sizeof(t));
+    if ( rc )
+        return rc;
+
+    if ( i ) /* expect only a single instance */
+        return -ENXIO;
+
+    return tsc_set_info(d, t.mode, t.nsec, t.khz, t.incarnation);
+}
+
+DOMAIN_REGISTER_CTXT_TYPE(TSC_INFO, save_tsc_info, load_tsc_info);
+
 /* vtsc may incur measurable performance degradation, diagnose with this */
 static void dump_softtsc(unsigned char key)
 {
diff --git a/xen/include/public/save.h b/xen/include/public/save.h
index bccbaadd0b..86881864cf 100644
--- a/xen/include/public/save.h
+++ b/xen/include/public/save.h
@@ -50,6 +50,7 @@  enum {
     DOMAIN_CONTEXT_END,
     DOMAIN_CONTEXT_START,
     DOMAIN_CONTEXT_SHARED_INFO,
+    DOMAIN_CONTEXT_TSC_INFO,
     /* New types go here */
     DOMAIN_CONTEXT_NR_TYPES
 };
@@ -69,6 +70,13 @@  struct domain_context_shared_info {
     uint8_t buffer[XEN_FLEX_ARRAY_DIM];
 };
 
+struct domain_context_tsc_info {
+    uint32_t mode;
+    uint32_t khz;
+    uint64_t nsec;
+    uint32_t incarnation;
+};
+
 /* Terminating entry */
 struct domain_context_end {};