x86/vpmu: Improve documentation and parsing for vpmu=

The behaviour of vpmu=<bool> being exclusive of vpmu=bts|ipc|arch is odd and
contrary to Xen's normal command line parsing behaviour.  Rewrite the parsing
to use the normal form, but retain the previous behaviour where the use of
bts/ipc/arch implies vpmu=true.

Parts of the documenation are stale, most notibly the HVM-only statement.
Update it for consistency and correctness.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index c8d1ced0186db81c26b0dea0a1183eba4d19d050..a03c0b4bdfa48721aac8ddb135d15a9e3bfcbada 100644 (file)
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2108,36 +2108,38 @@ Use Virtual Processor ID support if available.  This prevents the need for TLB
 flushes on VM entry and exit, increasing performance.
 
 ### vpmu (x86)
-> `= ( <boolean> | { bts | ipc | arch [, ...] } )`
+    = List of [ <bool>, bts, ipc, arch ]
 
-> Default: `off`
+    Applicability: x86.  Default: false
 
-Switch on the virtualized performance monitoring unit for HVM guests.
+Controls for Performance Monitoring Unit virtualisation.
 
-If the current cpu isn't supported a message like
-'VPMU: Initialization failed. ...'
-is printed on the hypervisor serial log.
+Performance monitoring facilities tend to be very hardware specific, and
+provide access to a wealth of low level processor information.
 
-For some Intel Nehalem processors a quirk handling exist for an unknown
-wrong behaviour (see `handle_pmc_quirk()`).
+*   An overall boolean can be used to enable or disable vPMU support.  vPMU is
+    disabled by default.
 
-If 'vpmu=bts' is specified the virtualisation of the Branch Trace Store (BTS)
-feature is switched on on Intel processors supporting this feature.
+    When enabled, guests have full access to all performance counter settings,
+    including model specific functionality.  This is a superset of the
+    functionality offered by `ipc` and/or `arch`, but a subset of the
+    functionality offered by `bts`.
 
-vpmu=ipc enables performance monitoring, but restricts the counters to the
-most minimum set possible: instructions, cycles, and reference cycles. These
-can be used to calculate instructions per cycle (IPC).
+    Xen's watchdog functionality is implemented using performance counters.
+    As a result, use of the **watchdog** option will override and disable
+    vPMU.
 
-vpmu=arch enables performance monitoring, but restricts the counters to the
-pre-defined architectural events only. These are exposed by cpuid, and listed
-in the Pre-Defined Architectural Performance Events table from the Intel 64
-and IA-32 Architectures Software Developer's Manual, Volume 3B, System
-Programming Guide, Part 2.
+*   The `bts` option enables performance monitoring, and permits additional
+    access to the Branch Trace Store controls.  BTS is an Intel feature where
+    the processor can write data into a buffer whenever a branch occurs.
+    However, as this feature isn't virtualised, a misconfiguration by the
+    guest can lock the entire system up.
 
-If a boolean is not used, combinations of flags are allowed, comma separated.
-For example, vpmu=arch,bts.
+*   The `ipc` option allows access to the most minimal set of counters
+    possible: instructions, cycles, and reference cycles.  These can be used
+    to calculate instructions per cycle (IPC).
 
-Note that if **watchdog** option is also specified vpmu will be turned off.
+*   The `arch` option allows access to the pre-defined architectural events.
 
 *Warning:*
 As the virtualisation is not 100% safe, don't use the vpmu flag on

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 13da7d0a6856b019aae4b078b107985c09b3454d..8324d62f11debe03e6d520281bfd93fa62505f75 100644 (file)
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -42,19 +42,9 @@ CHECK_pmu_cntr_pair;
 CHECK_pmu_data;
 CHECK_pmu_params;
 
-/*
- * "vpmu" :     vpmu generally enabled (all counters)
- * "vpmu=off"  : vpmu generally disabled
- * "vpmu=bts"  : vpmu enabled and Intel BTS feature switched on.
- * "vpmu=ipc"  : vpmu enabled for IPC counters only (most restrictive)
- * "vpmu=arch" : vpmu enabled for predef arch counters only (restrictive)
- * flag combinations are allowed, eg, "vpmu=ipc,bts".
- */
 static unsigned int __read_mostly opt_vpmu_enabled;
 unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF;
 unsigned int __read_mostly vpmu_features = 0;
-static int parse_vpmu_params(const char *s);
-custom_param("vpmu", parse_vpmu_params);
 
 static DEFINE_SPINLOCK(vpmu_lock);
 static unsigned vpmu_count;
@@ -64,37 +54,41 @@ static DEFINE_PER_CPU(struct vcpu *, last_vcpu);
 static int __init parse_vpmu_params(const char *s)
 {
     const char *ss;
+    int rc = 0, val;
 
-    switch ( parse_bool(s, NULL) )
-    {
-    case 0:
-        break;
-    default:
-        do {
-            ss = strchr(s, ',');
-            if ( !ss )
-                ss = strchr(s, '\0');
-
-            if ( !cmdline_strcmp(s, "bts") )
-                vpmu_features |= XENPMU_FEATURE_INTEL_BTS;
-            else if ( !cmdline_strcmp(s, "ipc") )
-                vpmu_features |= XENPMU_FEATURE_IPC_ONLY;
-            else if ( !cmdline_strcmp(s, "arch") )
-                vpmu_features |= XENPMU_FEATURE_ARCH_ONLY;
-            else
-                return -EINVAL;
+    do {
+        ss = strchr(s, ',');
+        if ( !ss )
+            ss = strchr(s, '\0');
+
+        if ( (val = parse_bool(s, ss)) >= 0 )
+        {
+            opt_vpmu_enabled = val;
+            if ( !val )
+                vpmu_features = 0;
+        }
+        else if ( !cmdline_strcmp(s, "bts") )
+            vpmu_features |= XENPMU_FEATURE_INTEL_BTS;
+        else if ( !cmdline_strcmp(s, "ipc") )
+            vpmu_features |= XENPMU_FEATURE_IPC_ONLY;
+        else if ( !cmdline_strcmp(s, "arch") )
+            vpmu_features |= XENPMU_FEATURE_ARCH_ONLY;
+        else
+            rc = -EINVAL;
+
+        s = ss + 1;
+    } while ( *ss );
 
-            s = ss + 1;
-        } while ( *ss );
-        /* fall through */
-    case 1:
-        /* Default VPMU mode */
+    /* Selecting bts/ipc/arch implies vpmu=1. */
+    if ( vpmu_features )
+        opt_vpmu_enabled = true;
+
+    if ( opt_vpmu_enabled )
         vpmu_mode = XENPMU_MODE_SELF;
-        opt_vpmu_enabled = 1;
-        break;
-    }
-    return 0;
+
+    return rc;
 }
+custom_param("vpmu", parse_vpmu_params);
 
 void vpmu_lvtpc_update(uint32_t val)
 {