[RFC PATCH 00/22] vPMU bugfixes and support for PMUv5

[RFC PATCH 00/22] vPMU bugfixes and support for PMUv5

[Edwin Török]

These are a set of patches to improve performance monitoring in Xen.
It starts by fixing MSR_PLATFORM_INFO and making MSR_SMI_COUNT available.
Also allows a pinned Dom0 to read any MSR, there is no reason why this shouldn't be allowed, and it prevents having to recompile Xen in order to investigate a problem that is difficult to reproduce (e.g. one such MSR is the SMI counter).

Then it fixes bugs in each architectural PMU version starting from version 1, and introduces support
for new features up to version 5. This was done by reading the Intel SDM for each version and checking whether Xen implements the needed features correctly.

AMD is next with fixes for performance counters that were readable but not exposed in CPUID, and new support for another performance counter.

There is more to be done here (APERF/MPERF, IBS, temperature monitoring, and I haven't even began approaching uncore counters), but those are difficult to do correctly and I've split them out of this series.

The patches were written in 2021, then git segfaulted when rebasing them and they got lost in a branch until now.
I've rebased and compile-tested them on latest Xen master, but I haven't done any further testing on them yet, hence the RFC prefix to mark these as experimental (e.g. I'm not entirely sure whether some changes got lost in the cpuid.c to cpu-policy.c rebase or not).

I'm sending this out as an RFC so it is not lost again, and I'll resend a proper version once the code has been tested more.
The backport tags attempt to indicate which version of Xen is the oldest one that has a bug fixed by the patch, should a backport be desired.
E.g. Xen with vpmu on would claim it supports PMUv3, but doesn't actually implement it correctly, so those patches+AMD CPUID fix could be backported.

Edwin Török (22):
  x86/msr: MSR_PLATFORM_INFO shouldn't claim that turbo is programmable
  x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel
  x86/msr: always allow a pinned Dom0 to read any unknown MSR
  x86/msr-index: add references to vendor manuals
  x86/PMUv1: report correct information in 0xa CPUID
  x86/PMUv1: limit arch PMCs to 4 for non-Dom0
  x86/PMUv1: allow topdown slots arch perf event
  x86/PMUv1: define macro for max number of events
  x86/PMUv1: consistently use 8 perf counters in Dom0
  x86/PMUv2: limit number of fixed perf counters to 3
  x86/PMUv2: freeze support in IA32_DEBUGCTL
  x86/msr-index: define more architectural MSRs
  x86/vpmu: expose PDCM and IA32_PERF_CAPABILITIES when vpmu is enabled
  x86/msr: RO MSR_TURBO_RATIO_LIMIT{,1,2}, MSR_TEMPERATURE_TARGET
  x86/VPMU: use macros for max supported VPMU version
  x86/PMUv4: support LBR_Frz and CTR_Frz
  x86/PMUv4: IA32_PERF_GLOBAL_{STATUS_SET, INUSE} support
  x86/PMUv4: disable intercept for PERF_GLOBAL_STATUS
  x86/PMUv4: bump max PMU version to 4
  x86/PMUv5: limit available fixed PMCs and enable support
  x86/AMD: fix CPUID for PerfCtr{4,5}
  x86/AMD: add IRPerf support

 xen/arch/x86/cpu-policy.c                   | 32 +++++--
 xen/arch/x86/cpu/vpmu_intel.c               | 99 ++++++++++++++-------
 xen/arch/x86/cpuid.c                        | 40 ++++++++-
 xen/arch/x86/hvm/svm/svm.c                  |  4 +
 xen/arch/x86/hvm/vmx/vmx.c                  | 33 +++++--
 xen/arch/x86/include/asm/msr-index.h        | 79 ++++++++++++++--
 xen/arch/x86/include/asm/vpmu.h             | 32 +++++++
 xen/arch/x86/msr.c                          | 22 +++++
 xen/arch/x86/pv/emul-priv-op.c              | 28 +++---
 xen/include/public/arch-x86/cpufeatureset.h |  6 +-
 xen/include/xen/lib/x86/cpu-policy.h        |  5 +-
 11 files changed, 317 insertions(+), 63 deletions(-)

-- 
2.41.0

[RFC PATCH 01/22] x86/msr: MSR_PLATFORM_INFO shouldn't claim that turbo is programmable

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Xen forbids writes to the various turbo control MSRs, however MSR_PLATFORM_INFO claims that these MSRs are writable.
Override MSR_PLATFORM_INFO bits to indicate lack of support.

See Intel SDM Volume 4, 2.17.6 "MSRs Introduced in the Intel Xeon Scaslable Processor Family",
which describes that MSR_PLATFORM_INFO.[28] = 1 implies that MSR_TURBO_RATIO_LIMIT is R/W,
and similarly bit 29 for TDP control, and bit 30 for MSR_TEMPERATURE_TARGET.

These bits were not all present on earlier processors, however where missing the bits were reserved,
and when present they are always present in the same bits.

(Curiously bit 31 that Xen uses is not documented anywhere in this manual but a separate one).

Backport: 4.0+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu-policy.c            | 8 ++++++++
 xen/include/xen/lib/x86/cpu-policy.h | 5 ++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 81e574390f..64c8857a61 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -407,6 +407,14 @@ static void __init calculate_host_policy(void)
     /* 0x000000ce  MSR_INTEL_PLATFORM_INFO */
     /* probe_cpuid_faulting() sanity checks presence of MISC_FEATURES_ENABLES */
     p->platform_info.cpuid_faulting = cpu_has_cpuid_faulting;
+
+    /* Xen denies write access to turbo control MSRs, however natively the CPU may support them
+       and advertise those MSRs as writable by having bits 28 to 30 set to 1 in MSR_PLATFORM_INFO.
+       Set these bits to 0 to avoid confusing guests on the availability of turbo controls.
+    */
+    p->platform_info.programmable_ratio_turbo = 0;
+    p->platform_info.programmable_tdp_turbo = 0;
+    p->platform_info.programmable_tj_offset = 0;
 }
 
 static void __init guest_common_max_feature_adjustments(uint32_t *fs)
diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
index bab3eecda6..70479689f2 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -339,7 +339,10 @@ struct cpu_policy
     union {
         uint32_t raw;
         struct {
-            uint32_t :31;
+            uint32_t :28;
+            bool programmable_ratio_turbo:1;
+            bool programmable_tdp_turbo:1;
+            bool programmable_tj_offset:1;
             bool cpuid_faulting:1;
         };
     } platform_info;
-- 
2.41.0

[RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Dom0 should always be able to read this MSR: it is useful when
investigating performance issues in production.
Although the count is Thread scoped, in practice all cores were observed
to return the same count (perhaps due to implementation details of SMM),
so do not require the cpu to be pinned in order to read it.

This MSR exists on Intel since Nehalem.

Backport: 4.15+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/include/asm/msr-index.h | 3 +++
 xen/arch/x86/msr.c                   | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 82a81bd0a2..2853a276ca 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -641,6 +641,9 @@
 #define MSR_NHL_LBR_SELECT		0x000001c8
 #define MSR_NHL_LASTBRANCH_TOS		0x000001c9
 
+/* Nehalem and newer other MSRs */
+#define MSR_SMI_COUNT                       0x00000034
+
 /* Skylake (and newer) last-branch recording */
 #define MSR_SKL_LASTBRANCH_0_FROM_IP	0x00000680
 #define MSR_SKL_LASTBRANCH_0_TO_IP	0x000006c0
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index c33dc78cd8..0bf6d263e7 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -139,6 +139,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
         *val = msrs->misc_features_enables.raw;
         break;
 
+    case MSR_SMI_COUNT:
+        if ( cp->x86_vendor != X86_VENDOR_INTEL )
+            goto gp_fault;
+        if ( is_hardware_domain(d) && !rdmsr_safe(msr, *val) )
+            break;
+        return X86EMUL_UNHANDLEABLE;
+
     case MSR_P5_MC_ADDR:
     case MSR_P5_MC_TYPE:
     case MSR_IA32_MCG_CAP     ... MSR_IA32_MCG_CTL:      /* 0x179 -> 0x17b */
-- 
2.41.0

[RFC PATCH 03/22] x86/msr: always allow a pinned Dom0 to read any unknown MSR

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

This can be useful if you realize you have to inspect the value of an
MSR in production, without having to change into a new Xen first that
handles the MSR.

E.g. SMI count didn't use to be explicitly allowed in the past
(it now is, see a previous commit), but there could be other MSRs that
are useful when tracking down issues.

Backport: 4.15+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/hvm/svm/svm.c     | 3 +++
 xen/arch/x86/hvm/vmx/vmx.c     | 3 +++
 xen/arch/x86/pv/emul-priv-op.c | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 24c417ca71..45f8e1ffd1 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1933,6 +1933,9 @@ static int cf_check svm_msr_read_intercept(
         break;
 
     default:
+        if ( is_hwdom_pinned_vcpu(v) && !rdmsr_safe(msr, *msr_content) )
+            break;
+
         if ( d->arch.msr_relaxed && !rdmsr_safe(msr, tmp) )
         {
             *msr_content = 0;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 1edc7f1e91..f6e5123f66 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3377,6 +3377,9 @@ static int cf_check vmx_msr_read_intercept(
         if ( vmx_read_guest_msr(curr, msr, msr_content) == 0 )
             break;
 
+        if ( is_hwdom_pinned_vcpu(curr) && !rdmsr_safe(msr, *msr_content) )
+            return X86EMUL_OKAY;
+
         if ( is_last_branch_msr(msr) )
         {
             *msr_content = 0;
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 0d9f84f458..978ae679a2 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -985,6 +985,9 @@ static int cf_check read_msr(
         }
         /* fall through */
     default:
+        if ( is_hwdom_pinned_vcpu(curr) && !rdmsr_safe(reg, *val) )
+            return X86EMUL_OKAY;
+
         if ( currd->arch.msr_relaxed && !rdmsr_safe(reg, tmp) )
         {
             *val = 0;
-- 
2.41.0

[RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

To more easily lookup the semantics of these MSRs add references to
vendor manuals.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/include/asm/msr-index.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 2853a276ca..8601f8f426 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -13,6 +13,16 @@
  * Blocks of related constants should be sorted by MSR index.  The constant
  * names should be as concise as possible, and the bit names may have an
  * abbreviated name.  Exceptions will be considered on a case-by-case basis.
+ *
+ * References:
+ * - https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
+ *    Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific registers
+ *    Chapter 2, "Model-Specific Registers (MSRs)"
+ *
+ * - https://developer.amd.com/resources/developer-guides-manuals/
+ *    AMD64 Architecture Programmer’s Manual Volume 2: System Programming
+ *    Appendix A "MSR Cross-Reference"
+ *
  */
 
 #define MSR_P5_MC_ADDR                      0
-- 
2.41.0

[RFC PATCH 05/22] x86/PMUv1: report correct information in 0xa CPUID

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

The 0xa CPUID leaf has to report supported number of:
- fixed performance counters
- general purpose performance counters
- architectural predefined events

And the PMU version (which was already limited to 3).

Type punning is used, which should be safe due to -fno-strict-aliasing.

This limits the number of arch events supported when vpmu=arch on Icelake.

Backport: 4.0+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c   |  2 +-
 xen/arch/x86/cpuid.c            | 39 ++++++++++++++++++++++++++++++---
 xen/arch/x86/include/asm/vpmu.h |  4 ++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index fa5b40c65c..9602728f1b 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -66,7 +66,7 @@ static bool_t __read_mostly full_width_write;
 #define ARCH_CNTR_PIN_CONTROL (1ULL << 19)
 
 /* Number of general-purpose and fixed performance counters */
-static unsigned int __read_mostly arch_pmc_cnt, fixed_pmc_cnt;
+unsigned int __read_mostly arch_pmc_cnt, fixed_pmc_cnt;
 
 /* Masks used for testing whether and MSR is valid */
 #define ARCH_CTRL_MASK  (~((1ull << 32) - 1) | (1ull << 21) | ARCH_CNTR_PIN_CONTROL)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 455a09b2dd..dfbcd1b3a4 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -304,9 +304,42 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
             *res = EMPTY_LEAF;
         else
         {
-            /* Report at most v3 since that's all we currently emulate. */
-            if ( (res->a & 0xff) > 3 )
-                res->a = (res->a & ~0xff) | 3;
+            union {
+                uint32_t eax;
+                struct {
+                    uint8_t version;
+                    uint8_t general_nr;
+                    uint8_t general_width;
+                    uint8_t arch_nr;
+                };
+            } u;
+            u.eax = res->a;
+
+            /* Report at most VPMU_VERSION_MAX since that's all we currently emulate. */
+            if ( u.version >  VPMU_VERSION_MAX ) {
+                gdprintk(XENLOG_WARNING, "Limiting PMU version to %d (actual %d)", VPMU_VERSION_MAX, u.version);
+                u.version = VPMU_VERSION_MAX;
+            }
+
+            if ( u.general_nr > arch_pmc_cnt ) {
+                gdprintk(XENLOG_WARNING, "Limiting general purpose PMU count to %d (actual %d)", arch_pmc_cnt, u.general_nr);
+                u.general_nr = arch_pmc_cnt;
+            }
+
+            if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY |
+                                  XENPMU_FEATURE_ARCH_ONLY) ) {
+                unsigned limit = ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) ? 7 : 3;
+                if (limit < u.arch_nr) {
+                    gdprintk(XENLOG_WARNING, "Limiting architectural PMU events to %d (actual %d)", limit, u.arch_nr);
+                    u.arch_nr = limit;
+                }
+            }
+
+            res->a = u.eax;
+
+            /* We only implement 3 fixed function counters */
+            if ( (res->d & 0x1f) > fixed_pmc_cnt )
+                res->d = (res->d & ~0x1f) | fixed_pmc_cnt;
         }
         break;
 
diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index b165acc6c2..1ef6089ccb 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -74,6 +74,8 @@ struct vpmu_struct {
 #define VPMU_CPU_HAS_DS                     0x1000 /* Has Debug Store */
 #define VPMU_CPU_HAS_BTS                    0x2000 /* Has Branch Trace Store */
 
+#define VPMU_VERSION_MAX                    0x3
+
 static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask)
 {
     vpmu->flags |= mask;
@@ -118,6 +120,8 @@ static inline int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
 
 extern unsigned int vpmu_mode;
 extern unsigned int vpmu_features;
+extern unsigned int arch_pmc_cnt;
+extern unsigned int fixed_pmc_cnt;
 
 /* Context switch */
 static inline void vpmu_switch_from(struct vcpu *prev)
-- 
2.41.0

[RFC PATCH 06/22] x86/PMUv1: limit arch PMCs to 4 for non-Dom0

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Only PERFEVTSEL{0-3} are architectural MSRs and Thread scoped.

PERFEVTSEL{4-7} are Core scoped, and we cannot allow using them if more
than 1 guest can attempt to modify them: if they program them with
different events (quite likely when multiplexing) then one of the VMs
would sample the wrong PMCs.

For now only allow this when Dom0 is the only one using the PMU, i.e. in
vpmu mode `all`.

We could also allow this when sched_gran >= SCHED_GRAN_core, but we
don't have access to the cpupool here.

There is some indication that this was causing bugs, e.g.
`pcm` mentions about a bug with perf counters beyond 3 on AWS:
https://github.com/opcm/pcm/commit/02f3b00f304401c723131372e09b71798df613ff

Backport: 4.0+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 9602728f1b..ec9ab01fde 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -926,6 +926,16 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
     }
 
     arch_pmc_cnt = core2_get_arch_pmc_count();
+    if ( arch_pmc_cnt > 4 &&
+         vpmu_mode != XENPMU_MODE_ALL )
+    {
+        /* Architectural PMCs 0-3 are Thread scoped, but 4+ are Core scoped.
+         * We can only allow using them if we know that we have at most one guest using a PMU
+         * on all siblings threads on a core. */
+        printk(XENLOG_INFO "VPMU: limiting architectural PMCs to 4\n");
+        arch_pmc_cnt = 4;
+    }
+
     fixed_pmc_cnt = core2_get_fixed_pmc_count();
 
     if ( cpu_has_pdcm )
-- 
2.41.0

[RFC PATCH 07/22] x86/PMUv1: allow topdown slots arch perf event

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

This is part of 'Architectural Performance Monitoring Version 1'
and implemented on Icelake.

Backport: 4.13+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c | 1 +
 xen/arch/x86/cpuid.c          | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index ec9ab01fde..44a1ed5b10 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -645,6 +645,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
                 case 0x412e:	/* Last Level Cache Misses */
                 case 0x00c4:	/* Branch Instructions Retired */
                 case 0x00c5:	/* All Branch Mispredict Retired */
+                case 0x01a4:	/* Topdown Slots */
                     blocked = 0;
                     break;
                }
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index dfbcd1b3a4..51ee89afc4 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -328,7 +328,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
 
             if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY |
                                   XENPMU_FEATURE_ARCH_ONLY) ) {
-                unsigned limit = ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) ? 7 : 3;
+                unsigned limit = ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) ? 8 : 3;
                 if (limit < u.arch_nr) {
                     gdprintk(XENLOG_WARNING, "Limiting architectural PMU events to %d (actual %d)", limit, u.arch_nr);
                     u.arch_nr = limit;
-- 
2.41.0

[RFC PATCH 08/22] x86/PMUv1: define macro for max number of events

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

This is needed so we can expose the maximum supported in CPUID,
without cpuid.c and vpmu_intel.c going out of sync.

The macros defined here take a parameter that controls how the enum
values are used: either to generate case statements or to count how many
elements we have.

They are a variation on https://en.wikipedia.org/wiki/X_Macro

No functional change.

Could be backported to 4.13.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c   | 16 ++--------------
 xen/arch/x86/cpuid.c            |  2 +-
 xen/arch/x86/include/asm/vpmu.h | 27 +++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 44a1ed5b10..ef8d69a0d6 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -622,15 +622,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
                 blocked = 1;
                 switch ( umaskevent )
                 {
-                /*
-                 * See 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.
-                 */
-                case 0x003c:	/* UnHalted Core Cycles */
-                case 0x013c:	/* UnHalted Reference Cycles */
-                case 0x00c0:	/* Instructions Retired */
+                VPMU_IPC_EVENTS(DEFCASE)
                     blocked = 0;
                     break;
                 }
@@ -641,11 +633,7 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
                 /* Additional counters beyond IPC only; blocked already set. */
                 switch ( umaskevent )
                 {
-                case 0x4f2e:	/* Last Level Cache References */
-                case 0x412e:	/* Last Level Cache Misses */
-                case 0x00c4:	/* Branch Instructions Retired */
-                case 0x00c5:	/* All Branch Mispredict Retired */
-                case 0x01a4:	/* Topdown Slots */
+                VPMU_ARCH_EVENTS(DEFCASE)
                     blocked = 0;
                     break;
                }
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 51ee89afc4..12e768ae87 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -328,7 +328,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
 
             if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY |
                                   XENPMU_FEATURE_ARCH_ONLY) ) {
-                unsigned limit = ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) ? 8 : 3;
+                unsigned limit = VPMU_IPC_EVENTS_MAX + ( vpmu_features & XENPMU_FEATURE_ARCH_ONLY ) ? VPMU_ARCH_EVENTS_MAX : 0;
                 if (limit < u.arch_nr) {
                     gdprintk(XENLOG_WARNING, "Limiting architectural PMU events to %d (actual %d)", limit, u.arch_nr);
                     u.arch_nr = limit;
diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index 1ef6089ccb..49c3e8c19a 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -146,5 +146,32 @@ static inline int vpmu_allocate_context(struct vcpu *v)
 }
 #endif
 
+/*
+ * See "20.2.1.2 Pre-Defined Architectural Performance Events"
+ * from the Intel 64 and IA-32 Architectures Software
+ * Developer's Manual, Volume 3B, System Programming Guide,
+ * Part 2.
+ */
+#define VPMU_IPC_EVENTS(DEF) \
+    DEF(0x003c)	/* UnHalted Core Cycles */\
+    DEF(0x00c0)	/* Instructions Retired */\
+    DEF(0x013c)	/* UnHalted Reference Cycles */\
+
+
+#define VPMU_ARCH_EVENTS(DEF) \
+    VPMU_IPC_EVENTS(DEF)\
+    DEF(0x4f2e)	/* Last Level Cache References */\
+    DEF(0x412e)	/* Last Level Cache Misses */\
+    DEF(0x00c4)	/* Branch Instructions Retired */\
+    DEF(0x00c5)	/* All Branch Mispredict Retired */\
+    DEF(0x01a4)	/* Topdown Slots */\
+
+#define DEFCASE(x) case (x):
+#define DEFSUM(x) +1
+#define DEFCOUNT(X) (0+X(DEFSUM))
+
+#define VPMU_IPC_EVENTS_MAX DEFCOUNT(VPMU_IPC_EVENTS)
+#define VPMU_ARCH_EVENTS_MAX DEFCOUNT(VPMU_ARCH_EVENTS)
+
 #endif /* __ASM_X86_HVM_VPMU_H_*/
 
-- 
2.41.0

[RFC PATCH 09/22] x86/PMUv1: consistently use 8 perf counters in Dom0

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

The code is currently inconsistent: supports 4 on read and 8 on write.
Sandy Bridge+ supports 8 of these, and the MSR range is architecturally
reserved, so always support 8.

Make it a macro to ensure we use the same value everywhere.

Although DomUs are now restricted to only 4 PMCs, we may still want to
use all 8 in Dom0 when available, and since the default rdwmsr
restrictions Dom0 would be prevented to read these MSRs as well.

Depends on:
"x86/PMUv1: limit arch PMCs to 4 for non-Dom0"

Backport: 4.15+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c           | 8 ++++----
 xen/arch/x86/include/asm/msr-index.h | 3 +++
 xen/arch/x86/pv/emul-priv-op.c       | 8 ++++----
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index f6e5123f66..7d51addf7a 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3360,8 +3360,8 @@ static int cf_check vmx_msr_read_intercept(
                        MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL;
         /* Perhaps vpmu will change some bits. */
         /* FALLTHROUGH */
-    case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
-    case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(3):
+    case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR_LAST:
+    case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
     case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
     case MSR_IA32_PEBS_ENABLE:
@@ -3678,8 +3678,8 @@ static int cf_check vmx_msr_write_intercept(
             goto gp_fault;
         break;
 
-    case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR(7):
-    case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL(7):
+    case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR_LAST:
+    case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
     case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
     case MSR_IA32_PEBS_ENABLE:
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 8601f8f426..011a926e0e 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -521,8 +521,11 @@
 #define MSR_IA32_PSR_MBA_MASK(n)	(0x00000d50 + (n))
 
 /* Intel Model 6 */
+#define MSR_P6_PERFCTR_MAX		8
 #define MSR_P6_PERFCTR(n)		(0x000000c1 + (n))
 #define MSR_P6_EVNTSEL(n)		(0x00000186 + (n))
+#define MSR_P6_PERFCTR_LAST		MSR_P6_PERFCTR(MSR_P6_PERFCTR_MAX-1)
+#define MSR_P6_EVNTSEL_LAST		MSR_P6_EVNTSEL(MSR_P6_PERFCTR_MAX-1)
 
 /* P4/Xeon+ specific */
 #define MSR_IA32_MCG_EAX		0x00000180
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 978ae679a2..301a70f5ea 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -965,8 +965,8 @@ static int cf_check read_msr(
         *val = 0;
         return X86EMUL_OKAY;
 
-    case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR(7):
-    case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL(3):
+    case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
+    case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
@@ -1145,8 +1145,8 @@ static int cf_check write_msr(
             return X86EMUL_OKAY;
         break;
 
-    case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR(7):
-    case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL(3):
+    case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
+    case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
-- 
2.41.0

[RFC PATCH 10/22] x86/PMUv2: limit number of fixed perf counters to 3

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

There are only 3 architectural fixed function counters defined,
however Icelake introduces a 4th.
So we'll need to report the number of fixed counter implemented in CPUID
correctly for Icelake, define a macro to ensure we are consistent about
which counter is last.

Note: simply adding MSR_CORE_PERF_FIXED_CTR3 is not enough, Icelake also
defines MSR_PERF_METRICS and there are some ordering constraints on
restoring the MSR, and atomicity constraints on IA32_PERF_GLOBAL_CTRL,
so this is not implemented yet.

Backport: 4.13+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c        | 6 ++++++
 xen/arch/x86/hvm/vmx/vmx.c           | 4 ++--
 xen/arch/x86/include/asm/msr-index.h | 4 ++--
 xen/arch/x86/pv/emul-priv-op.c       | 4 ++--
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index ef8d69a0d6..4c0776cee7 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -926,6 +926,12 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
     }
 
     fixed_pmc_cnt = core2_get_fixed_pmc_count();
+#define PERF_FIXED_CTR_MAX (MSR_CORE_PERF_FIXED_CTRn - MSR_CORE_PERF_FIXED_CTR0 + 1)
+    if ( fixed_pmc_cnt > PERF_FIXED_CTR_MAX )
+    {
+        printk(XENLOG_INFO "VPMU: limiting fixed perf counters to %d\n", PERF_FIXED_CTR_MAX);
+        fixed_pmc_cnt = PERF_FIXED_CTR_MAX;
+    }
 
     if ( cpu_has_pdcm )
     {
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 7d51addf7a..1510e980dd 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3362,7 +3362,7 @@ static int cf_check vmx_msr_read_intercept(
         /* FALLTHROUGH */
     case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL_LAST:
-    case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
+    case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
     case MSR_IA32_PEBS_ENABLE:
     case MSR_IA32_DS_AREA:
@@ -3680,7 +3680,7 @@ static int cf_check vmx_msr_write_intercept(
 
     case MSR_P6_PERFCTR(0)...MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0)...MSR_P6_EVNTSEL_LAST:
-    case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTR2:
+    case MSR_CORE_PERF_FIXED_CTR0...MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
     case MSR_IA32_PEBS_ENABLE:
     case MSR_IA32_DS_AREA:
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 011a926e0e..8a881a8a6f 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -674,8 +674,8 @@
 
 /* Intel Core-based CPU performance counters */
 #define MSR_CORE_PERF_FIXED_CTR0	0x00000309
-#define MSR_CORE_PERF_FIXED_CTR1	0x0000030a
-#define MSR_CORE_PERF_FIXED_CTR2	0x0000030b
+#define MSR_CORE_PERF_FIXED_CTRn	0x0000030b
+
 #define MSR_CORE_PERF_FIXED_CTR_CTRL	0x0000038d
 #define MSR_CORE_PERF_GLOBAL_STATUS	0x0000038e
 #define MSR_CORE_PERF_GLOBAL_CTRL	0x0000038f
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 301a70f5ea..a8472fc779 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -967,7 +967,7 @@ static int cf_check read_msr(
 
     case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
-    case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
+    case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         {
@@ -1147,7 +1147,7 @@ static int cf_check write_msr(
 
     case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
-    case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTR2:
+    case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         {
-- 
2.41.0

[RFC PATCH 11/22] x86/PMUv2: freeze support in IA32_DEBUGCTL

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

This is not yet exposed by HVM policies, but PMU version 2 requires that
if PDCM is supported in CPUID then these 2 bits would work.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c           | 4 ++++
 xen/arch/x86/include/asm/msr-index.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 1510e980dd..f1f8a9afa2 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3601,6 +3601,10 @@ static int cf_check vmx_msr_write_intercept(
                           IA32_DEBUGCTLMSR_BTS_OFF_USR);
         }
 
+        if (cp->basic.pmu_version >= 2 && cpu_has(&current_cpu_data, X86_FEATURE_PDCM)) {
+            rsvd &= ~(IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI);
+        }
+
         if ( cp->feat.rtm )
             rsvd &= ~IA32_DEBUGCTLMSR_RTM;
 
diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 8a881a8a6f..0dfb5b499f 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -305,7 +305,9 @@
 #define IA32_DEBUGCTLMSR_BTINT		(1<<8) /* Branch Trace Interrupt */
 #define IA32_DEBUGCTLMSR_BTS_OFF_OS	(1<<9)  /* BTS off if CPL 0 */
 #define IA32_DEBUGCTLMSR_BTS_OFF_USR	(1<<10) /* BTS off if CPL > 0 */
-#define IA32_DEBUGCTLMSR_RTM		(1<<15) /* RTM debugging enable */
+#define IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI	(1<<11) /* LBR stack frozen on PMI */
+#define IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI	(1<<12) /*  Global counter control ENABLE bit frozen on PMI */
+#define IA32_DEBUGCTLMSR_RTM			(1<<15) /* RTM debugging enable */
 
 #define MSR_IA32_LASTBRANCHFROMIP	0x000001db
 #define MSR_IA32_LASTBRANCHTOIP		0x000001dc
-- 
2.41.0

[RFC PATCH 12/22] x86/msr-index: define more architectural MSRs

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Add most architectural MSRs, except those behind CPUID features that are
not yet implemented, such as TME, SGX.

Based on "2.1 Architectural MSRs" of Intel SDM volume 4

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/include/asm/msr-index.h | 54 +++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 0dfb5b499f..061b07c7ae 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -58,6 +58,14 @@
 #define  PRED_CMD_IBPB                      (_AC(1, ULL) <<  0)
 #define  PRED_CMD_SBPB                      (_AC(1, ULL) <<  7)
 
+#define MSR_IA32_SMM_MONITOR_CTL            0x0000009b
+#define MSR_IA32_SMBASE                     0x0000009e
+#define MSR_IA32_SMRR_PHYSBASE              0x000001f2
+#define MSR_IA32_SMRR_PHYSMASK              0x000001f3
+#define MSR_IA32_PLATFORM_DCA_CAP           0x000001f8
+#define MSR_IA32_CPU_DCA_CAP                0x000001f9
+#define MSR_IA32_DCA_0_CAP                  0x000001fa
+
 #define MSR_PPIN_CTL                        0x0000004e
 #define  PPIN_LOCKOUT                       (_AC(1, ULL) <<  0)
 #define  PPIN_ENABLE                        (_AC(1, ULL) <<  1)
@@ -267,13 +275,21 @@
 #define MSR_IA32_MCG_CAP		0x00000179
 #define MSR_IA32_MCG_STATUS		0x0000017a
 #define MSR_IA32_MCG_CTL		0x0000017b
-#define MSR_IA32_MCG_EXT_CTL	0x000004d0
+#define MSR_IA32_MCG_EXT_CTL		0x000004d0
 
 #define MSR_IA32_PEBS_ENABLE		0x000003f1
 #define MSR_IA32_DS_AREA		0x00000600
 #define MSR_IA32_PERF_CAPABILITIES	0x00000345
 /* Lower 6 bits define the format of the address in the LBR stack */
-#define MSR_IA32_PERF_CAP_LBR_FORMAT	0x3f
+#define MSR_IA32_PERF_CAP_LBR_FORMAT    	0x3f
+#define MSR_IA32_PERF_CAP_PEBS_TRAP		(_AC(1,ULL) << 6)
+#define MSR_IA32_PERF_CAP_PEBS_SAVE_ARCH_REGS	(_AC(1,ULL) << 7)
+#define MSR_IA32_PERF_CAP_PEBS_RECORD_FORMAT	0xf00
+#define MSR_IA32_PERF_CAP_FREEZE_WHILE_SMM	(_AC(1,ULL) << 12)
+#define MSR_IA32_PERF_CAP_FULLWIDTH_PMC 	(_AC(1,ULL) << 13)
+#define MSR_IA32_PERF_CAP_PEBS_BASELINE		(_AC(1,ULL) << 14)
+#define MSR_IA32_PERF_CAP_PERF_METRICS		(_AC(1,ULL) << 15)
+#define MSR_IA32_PERF_CAP_PEBS_TO_PT		(_AC(1,ULL) << 16)
 
 #define MSR_IA32_BNDCFGS		0x00000d90
 #define IA32_BNDCFGS_ENABLE		0x00000001
@@ -307,6 +323,8 @@
 #define IA32_DEBUGCTLMSR_BTS_OFF_USR	(1<<10) /* BTS off if CPL > 0 */
 #define IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI	(1<<11) /* LBR stack frozen on PMI */
 #define IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI	(1<<12) /*  Global counter control ENABLE bit frozen on PMI */
+#define IA32_DEBUGCTLMSR_ENABLE_UNCORE_PMI	(1<<13) /* Enable uncore PMI */
+#define IA32_DEBUGCTLMSR_FREEZE_WHILE_SMM	(1<<14) /* Freeze perfmon/trace while in SMM */
 #define IA32_DEBUGCTLMSR_RTM			(1<<15) /* RTM debugging enable */
 
 #define MSR_IA32_LASTBRANCHFROMIP	0x000001db
@@ -469,6 +487,7 @@
 #define MSR_VIA_RNG			0x0000110b
 
 /* Intel defined MSRs. */
+#define MSR_IA32_MONITOR_FILTER_SIZE	0x00000006
 #define MSR_IA32_TSC			0x00000010
 #define MSR_IA32_PLATFORM_ID		0x00000017
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
@@ -491,6 +510,7 @@
 #define MSR_IA32_PERF_STATUS		0x00000198
 #define MSR_IA32_PERF_CTL		0x00000199
 
+#define MSR_IA32_UMWAIT_CONTROL		0x000000e1
 #define MSR_IA32_MPERF			0x000000e7
 #define MSR_IA32_APERF			0x000000e8
 
@@ -498,6 +518,7 @@
 #define MSR_IA32_THERM_INTERRUPT	0x0000019b
 #define MSR_IA32_THERM_STATUS		0x0000019c
 #define MSR_IA32_MISC_ENABLE		0x000001a0
+#define MSR_IA32_MISC_ENABLE_FAST_STRINGS (1<<0)
 #define MSR_IA32_MISC_ENABLE_PERF_AVAIL   (1<<7)
 #define MSR_IA32_MISC_ENABLE_BTS_UNAVAIL  (1<<11)
 #define MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL (1<<12)
@@ -508,15 +529,38 @@
 #define MSR_IA32_MISC_ENABLE_TURBO_DISENGAGE (_AC(1, ULL) << 38)
 
 #define MSR_IA32_TSC_DEADLINE		0x000006E0
+
+#define MSR_IA32_PM_ENABLE		0x00000770
+#define MSR_IA32_HWP_CAPABILITIES	0x00000771
+#define MSR_IA32_HWP_REQUEST_PKG	0x00000772
+#define MSR_IA32_HWP_INTERRUPT		0x00000773
+#define MSR_IA32_HWP_REQUEST		0x00000774
+#define MSR_IA32_PECI_HWP_REQUEST_INFO	0x00000775
+#define MSR_IA32_HWP_STATUS		0x00000777
+
+#define MSR_IA32_PKG_HDC_CTL		0x00000db0
+#define MSR_IA32_PM_CTL1		0x00000db1
+#define MSR_IA32_THREAD_STALL		0x00000db2
+#define MSR_IA32_HW_FEEDBACK_PTR	0x000017d0
+#define MSR_IA32_HW_FEEDBACK_CONFIG	0x000017d1
+
+#define MSR_TEMPERATURE_TARGET		0x000001a2
+#define MSR_TURBO_RATIO_LIMIT		0x000001ad
+#define MSR_TURBO_RATIO_LIMIT1		0x000001ae
+#define MSR_TURBO_RATIO_LIMIT2		0x000001af
+
 #define MSR_IA32_ENERGY_PERF_BIAS	0x000001b0
+#define MSR_IA32_PACKAGE_THERM_STATUS	0x000001b1
+#define MSR_IA32_PACKAGE_THERM_INTERRUPT 0x000001b2
 
 /* Platform Shared Resource MSRs */
+#define MSR_IA32_PSR_L3_QOS_CFG		0x00000c81
+#define MSR_IA32_PSR_L2_QOS_CFG		0x00000c82
 #define MSR_IA32_CMT_EVTSEL		0x00000c8d
 #define MSR_IA32_CMT_EVTSEL_UE_MASK	0x0000ffff
 #define MSR_IA32_CMT_CTR		0x00000c8e
 #define MSR_IA32_PSR_ASSOC		0x00000c8f
-#define MSR_IA32_PSR_L3_QOS_CFG	0x00000c81
-#define MSR_IA32_PSR_L3_MASK(n)	(0x00000c90 + (n))
+#define MSR_IA32_PSR_L3_MASK(n)		(0x00000c90 + (n))
 #define MSR_IA32_PSR_L3_MASK_CODE(n)	(0x00000c90 + (n) * 2 + 1)
 #define MSR_IA32_PSR_L3_MASK_DATA(n)	(0x00000c90 + (n) * 2)
 #define MSR_IA32_PSR_L2_MASK(n)		(0x00000d10 + (n))
@@ -682,6 +726,8 @@
 #define MSR_CORE_PERF_GLOBAL_STATUS	0x0000038e
 #define MSR_CORE_PERF_GLOBAL_CTRL	0x0000038f
 #define MSR_CORE_PERF_GLOBAL_OVF_CTRL	0x00000390
+#define MSR_CORE_PERF_GLOBAL_STATUS_SET	0x00000391
+#define MSR_CORE_PERF_GLOBAL_INUSE	0x00000392
 
 /* Intel cpuid spoofing MSRs */
 #define MSR_INTEL_MASK_V1_CPUID1        0x00000478
-- 
2.41.0

[RFC PATCH 13/22] x86/vpmu: expose PDCM and IA32_PERF_CAPABILITIES when vpmu is enabled

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Marked as exposed by default, but then hidden if vpmu is not available.
TODO: the interaction between vpmu and policy might need some changes.

Only expose LBR and the full-width MSR capabilities, and not PEBS.

Backport: 4.15+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu-policy.c                   | 10 ++++++++--
 xen/arch/x86/hvm/vmx/vmx.c                  |  2 +-
 xen/arch/x86/msr.c                          |  8 ++++++++
 xen/arch/x86/pv/emul-priv-op.c              |  5 -----
 xen/include/public/arch-x86/cpufeatureset.h |  2 +-
 5 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 64c8857a61..e38b648f7d 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -388,8 +388,10 @@ static void __init calculate_host_policy(void)
     recalculate_misc(p);
 
     /* When vPMU is disabled, drop it from the host policy. */
-    if ( vpmu_mode == XENPMU_MODE_OFF )
+    if ( vpmu_mode == XENPMU_MODE_OFF ) {
         p->basic.raw[0xa] = EMPTY_LEAF;
+        p->basic.pdcm = 0;
+    }
 
     if ( p->extd.svm )
     {
@@ -899,8 +901,12 @@ void recalculate_cpuid_policy(struct domain *d)
     }
 
     if ( vpmu_mode == XENPMU_MODE_OFF ||
-         ((vpmu_mode & XENPMU_MODE_ALL) && !is_hardware_domain(d)) )
+         ((vpmu_mode & XENPMU_MODE_ALL) && !is_hardware_domain(d)) ) {
         p->basic.raw[0xa] = EMPTY_LEAF;
+        p->basic.pdcm = 0;
+    }
+    if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY | XENPMU_FEATURE_ARCH_ONLY) )
+        p->basic.pdcm = 0;
 
     if ( !p->extd.svm )
         p->extd.raw[0xa] = EMPTY_LEAF;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index f1f8a9afa2..fefd01be40 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3602,7 +3602,7 @@ static int cf_check vmx_msr_write_intercept(
         }
 
         if (cp->basic.pmu_version >= 2 && cpu_has(&current_cpu_data, X86_FEATURE_PDCM)) {
-            rsvd &= ~(IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI);
+            rsvd &= ~(IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI | IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI | IA32_DEBUGCTLMSR_FREEZE_WHILE_SMM);
         }
 
         if ( cp->feat.rtm )
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 0bf6d263e7..483b5e4f70 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -186,6 +186,14 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
             goto gp_fault;
         goto get_reg;
 
+    case MSR_IA32_PERF_CAPABILITIES:
+        if ( cp->x86_vendor != X86_VENDOR_INTEL )
+            goto gp_fault;
+        if ( !cp->basic.pdcm || rdmsr_safe(msr, *val) )
+            goto gp_fault;
+        *val &= (MSR_IA32_PERF_CAP_LBR_FORMAT | MSR_IA32_PERF_CAP_FREEZE_WHILE_SMM | MSR_IA32_PERF_CAP_FULLWIDTH_PMC);
+        break;
+
     case MSR_X2APIC_FIRST ... MSR_X2APIC_LAST:
         if ( !is_hvm_domain(d) || v != curr )
             goto gp_fault;
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index a8472fc779..e623e57b55 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -960,11 +960,6 @@ static int cf_check read_msr(
         *val = guest_misc_enable(*val);
         return X86EMUL_OKAY;
 
-    case MSR_IA32_PERF_CAPABILITIES:
-        /* No extra capabilities are supported. */
-        *val = 0;
-        return X86EMUL_OKAY;
-
     case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 6b6ce2745c..0aa3251397 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -118,7 +118,7 @@ XEN_CPUFEATURE(SSSE3,         1*32+ 9) /*A  Supplemental Streaming SIMD Extensio
 XEN_CPUFEATURE(FMA,           1*32+12) /*A  Fused Multiply Add */
 XEN_CPUFEATURE(CX16,          1*32+13) /*A  CMPXCHG16B */
 XEN_CPUFEATURE(XTPR,          1*32+14) /*   Send Task Priority Messages */
-XEN_CPUFEATURE(PDCM,          1*32+15) /*   Perf/Debug Capability MSR */
+XEN_CPUFEATURE(PDCM,          1*32+15) /*A  Perf/Debug Capability MSR */
 XEN_CPUFEATURE(PCID,          1*32+17) /*H  Process Context ID */
 XEN_CPUFEATURE(DCA,           1*32+18) /*   Direct Cache Access */
 XEN_CPUFEATURE(SSE4_1,        1*32+19) /*A  Streaming SIMD Extensions 4.1 */
-- 
2.41.0

[RFC PATCH 14/22] x86/msr: RO MSR_TURBO_RATIO_LIMIT{,1,2}, MSR_TEMPERATURE_TARGET

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Expose MSR_TURBO_RATIO_LIMIT{,1,2} and MSR_TEMPERATURE_TARGET to guest as RO.
Although these are not architectural MSRs they are in the same place
currently on all supported CPUs.
They also have the same meaning, except for 06_55H and 06_5C where
they have a different meaning (turbo core count).

It is safe to expose this to the guest by default: they are only
statically defined limits and don't expose runtime measurements.

It has been observed that some drivers BSOD on an unguarded read on
MSR 1ADH (e.g. socwatch).

Also we read as zero the actual temperature, so reporting the temp
target as 0 might lead to 0/0.

Backport: 4.15+

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/hvm/vmx/vmx.c     | 9 +++++++++
 xen/arch/x86/pv/emul-priv-op.c | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index fefd01be40..cd772585fe 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3353,6 +3353,15 @@ static int cf_check vmx_msr_read_intercept(
         if ( !nvmx_msr_read_intercept(msr, msr_content) )
             goto gp_fault;
         break;
+
+    case MSR_TEMPERATURE_TARGET:
+    case MSR_TURBO_RATIO_LIMIT...MSR_TURBO_RATIO_LIMIT2:
+        if ( !rdmsr_safe(msr, *msr_content) )
+            break;
+        /* RO for guests, MSR_PLATFORM_INFO bits set accordingly in msr.c to indicate lack of write
+         * support. */
+        goto gp_fault;
+
     case MSR_IA32_MISC_ENABLE:
         rdmsrl(MSR_IA32_MISC_ENABLE, *msr_content);
         /* Debug Trace Store is not supported. */
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index e623e57b55..09bfde1060 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -960,6 +960,10 @@ static int cf_check read_msr(
         *val = guest_misc_enable(*val);
         return X86EMUL_OKAY;
 
+    case MSR_TEMPERATURE_TARGET:
+    case MSR_TURBO_RATIO_LIMIT...MSR_TURBO_RATIO_LIMIT2:
+        goto normal;
+
     case MSR_P6_PERFCTR(0) ... MSR_P6_PERFCTR_LAST:
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
-- 
2.41.0

[RFC PATCH 15/22] x86/VPMU: use macros for max supported VPMU version

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

This ensures consistency between the 2 pieces of code that check for
VPMU version.

No functional change.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c   | 20 ++++++--------------
 xen/arch/x86/include/asm/vpmu.h |  1 +
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 4c0776cee7..82cd2656ea 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -810,7 +810,7 @@ static int cf_check core2_vpmu_initialise(struct vcpu *v)
     static bool_t ds_warned;
 
     if ( v->domain->arch.cpuid->basic.pmu_version <= 1 ||
-         v->domain->arch.cpuid->basic.pmu_version >= 6 )
+         v->domain->arch.cpuid->basic.pmu_version >= VPMU_VERSION_MAX_SUPPORTED )
         return -EINVAL;
 
     if ( (arch_pmc_cnt + fixed_pmc_cnt) == 0 )
@@ -890,22 +890,14 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
     if ( current_cpu_data.cpuid_level >= 0xa )
         version = MASK_EXTR(cpuid_eax(0xa), PMU_VERSION_MASK);
 
-    switch ( version )
-    {
-    case 4:
-    case 5:
-        printk(XENLOG_INFO "VPMU: PMU version %u is not fully supported. "
-               "Emulating version 3\n", version);
-        /* FALLTHROUGH */
-
-    case 2:
-    case 3:
-        break;
-
-    default:
+    if ( version <= 1 ||
+         version > VPMU_VERSION_MAX_SUPPORTED ) {
         printk(XENLOG_WARNING "VPMU: PMU version %u is not supported\n",
                version);
         return ERR_PTR(-EINVAL);
+    } else if ( version > VPMU_VERSION_MAX ) {
+        printk(XENLOG_INFO "VPMU: PMU version %u is not fully supported. "
+               "Emulating version %d\n", version, VPMU_VERSION_MAX);
     }
 
     if ( current_cpu_data.x86 != 6 )
diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index 49c3e8c19a..79f7f4a09e 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -75,6 +75,7 @@ struct vpmu_struct {
 #define VPMU_CPU_HAS_BTS                    0x2000 /* Has Branch Trace Store */
 
 #define VPMU_VERSION_MAX                    0x3
+#define VPMU_VERSION_MAX_SUPPORTED          0x5
 
 static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask)
 {
-- 
2.41.0

[RFC PATCH 16/22] x86/PMUv4: support LBR_Frz and CTR_Frz

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

The behaviour is changed from Legacy to Streamlined for the LBR and
PERFMON freeze bits.
See "17.4.7 Freezing LBR and Performance Counters on PMI".

Instead of clearing the freeze bits through DEBUGCTL they are now
cleared through MSR 0x390 like everything else.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 82cd2656ea..923fe42a0b 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -952,6 +952,13 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
          */
         global_ovf_ctrl_mask &= ~(1ULL << 61);
 
+    if ( version >= 4)
+        /* On PMU version 4 bits 58 and 59 are defined in
+         * IA32_PERF_GLOBAL_STATUS_RESET (same MSR as IA32_PERF_GLOBAL_STATUS).
+         * Also allow clearing overflow for processor trace, even if we don't support it yet.
+         * */
+        global_ovf_ctrl_mask &= ~((3ULL << 58) | (1ULL << 55));
+
     regs_sz = (sizeof(struct xen_pmu_intel_ctxt) - regs_off) +
               sizeof(uint64_t) * fixed_pmc_cnt +
               sizeof(struct xen_pmu_cntr_pair) * arch_pmc_cnt;
-- 
2.41.0

[RFC PATCH 17/22] x86/PMUv4: IA32_PERF_GLOBAL_{STATUS_SET, INUSE} support

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Expose thse MSRs to the guest when PMU version is >= 4.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c  | 20 +++++++++++++++++++-
 xen/arch/x86/hvm/vmx/vmx.c     |  5 +++++
 xen/arch/x86/pv/emul-priv-op.c |  5 +++++
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 923fe42a0b..5e660af395 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -171,6 +171,8 @@ static int is_core2_vpmu_msr(u32 msr_index, int *type, int *index)
     case MSR_CORE_PERF_GLOBAL_CTRL:
     case MSR_CORE_PERF_GLOBAL_STATUS:
     case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+    case MSR_CORE_PERF_GLOBAL_INUSE:
         *type = MSR_TYPE_GLOBAL;
         return 1;
 
@@ -545,10 +547,21 @@ static int cf_check core2_vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
         core2_vpmu_cxt->global_status &= ~msr_content;
         wrmsrl(MSR_CORE_PERF_GLOBAL_OVF_CTRL, msr_content);
         return 0;
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+        if ( (v->domain->arch.cpuid->basic.pmu_version < 4) ||
+             (msr_content & global_ovf_ctrl_mask) )
+            return -EINVAL;
+        core2_vpmu_cxt->global_status |= msr_content;
+        wrmsrl(MSR_CORE_PERF_GLOBAL_STATUS_SET, msr_content);
+        return 0;
     case MSR_CORE_PERF_GLOBAL_STATUS:
         gdprintk(XENLOG_INFO, "Can not write readonly MSR: "
                  "MSR_PERF_GLOBAL_STATUS(0x38E)!\n");
         return -EINVAL;
+    case MSR_CORE_PERF_GLOBAL_INUSE:
+        gdprintk(XENLOG_INFO, "Can not write readonly MSR: "
+                 "MSR_PERF_GLOBAL_INUSE(0x392)!\n");
+        return -EINVAL;
     case MSR_IA32_PEBS_ENABLE:
         if ( vpmu_features & (XENPMU_FEATURE_IPC_ONLY |
                               XENPMU_FEATURE_ARCH_ONLY) )
@@ -688,7 +701,8 @@ static int cf_check core2_vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
         core2_vpmu_cxt = vpmu->context;
         switch ( msr )
         {
-        case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
+        case MSR_CORE_PERF_GLOBAL_OVF_CTRL: /* FALLTHROUGH */
+        case MSR_CORE_PERF_GLOBAL_STATUS_SET:
             *msr_content = 0;
             break;
         case MSR_CORE_PERF_GLOBAL_STATUS:
@@ -700,6 +714,10 @@ static int cf_check core2_vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
             else
                 rdmsrl(MSR_CORE_PERF_GLOBAL_CTRL, *msr_content);
             break;
+        case MSR_CORE_PERF_GLOBAL_INUSE:
+            if ( v->domain->arch.cpuid->basic.pmu_version < 4 )
+                return -EINVAL;
+            /* FALLTHROUGH */
         default:
             rdmsrl(msr, *msr_content);
         }
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index cd772585fe..af70ed8f30 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3375,6 +3375,8 @@ static int cf_check vmx_msr_read_intercept(
     case MSR_CORE_PERF_FIXED_CTR_CTRL...MSR_CORE_PERF_GLOBAL_OVF_CTRL:
     case MSR_IA32_PEBS_ENABLE:
     case MSR_IA32_DS_AREA:
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+    case MSR_CORE_PERF_GLOBAL_INUSE:
         if ( vpmu_do_rdmsr(msr, msr_content) )
             goto gp_fault;
         break;
@@ -3698,6 +3700,9 @@ static int cf_check vmx_msr_write_intercept(
     case MSR_IA32_PEBS_ENABLE:
     case MSR_IA32_DS_AREA:
          if ( vpmu_do_wrmsr(msr, msr_content) )
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+    case MSR_CORE_PERF_GLOBAL_INUSE:
+         if ( vpmu_do_wrmsr(msr, msr_content) )
             goto gp_fault;
         break;
 
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 09bfde1060..105485bb1e 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -968,6 +968,9 @@ static int cf_check read_msr(
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
+    case MSR_IA32_PEBS_ENABLE:
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+    case MSR_CORE_PERF_GLOBAL_INUSE:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         {
             vpmu_msr = true;
@@ -1148,6 +1151,8 @@ static int cf_check write_msr(
     case MSR_P6_EVNTSEL(0) ... MSR_P6_EVNTSEL_LAST:
     case MSR_CORE_PERF_FIXED_CTR0 ... MSR_CORE_PERF_FIXED_CTRn:
     case MSR_CORE_PERF_FIXED_CTR_CTRL ... MSR_CORE_PERF_GLOBAL_OVF_CTRL:
+    case MSR_CORE_PERF_GLOBAL_STATUS_SET:
+    case MSR_CORE_PERF_GLOBAL_INUSE:
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         {
             vpmu_msr = true;
-- 
2.41.0

[RFC PATCH 18/22] x86/PMUv4: disable intercept for PERF_GLOBAL_STATUS

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Now that we have a way to set PERF_GLOBAL_STATUS by writing to
PERF_GLOBAL_STATUS_RESET (== PERF_GLOBAL_OVF_CTRL) and
PERF_GLOBAL_STATUS_SET we do not need to intercept this MSR anymore.

We can save/restore its state when saving/loading vPMU state, and
otherwise let the guest read it directly.

This is an optimization, perhaps it'd need a flag to disable it for
debugging purposes.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu/vpmu_intel.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index 5e660af395..59d0b2ca36 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -67,6 +67,7 @@ static bool_t __read_mostly full_width_write;
 
 /* Number of general-purpose and fixed performance counters */
 unsigned int __read_mostly arch_pmc_cnt, fixed_pmc_cnt;
+static unsigned int __read_mostly vpmu_version;
 
 /* Masks used for testing whether and MSR is valid */
 #define ARCH_CTRL_MASK  (~((1ull << 32) - 1) | (1ull << 21) | ARCH_CNTR_PIN_CONTROL)
@@ -228,6 +229,9 @@ static void core2_vpmu_set_msr_bitmap(struct vcpu *v)
 
     vmx_clear_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, VMX_MSR_R);
     vmx_clear_msr_intercept(v, MSR_IA32_DS_AREA, VMX_MSR_R);
+
+    if ( vpmu_version >= 4 )
+        vmx_clear_msr_intercept(v, MSR_CORE_PERF_GLOBAL_STATUS, VMX_MSR_R);
 }
 
 static void core2_vpmu_unset_msr_bitmap(struct vcpu *v)
@@ -250,6 +254,9 @@ static void core2_vpmu_unset_msr_bitmap(struct vcpu *v)
 
     vmx_set_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, VMX_MSR_R);
     vmx_set_msr_intercept(v, MSR_IA32_DS_AREA, VMX_MSR_R);
+
+    if ( vpmu_version >= 4 )
+        vmx_set_msr_intercept(v, MSR_CORE_PERF_GLOBAL_STATUS, VMX_MSR_R);
 }
 
 static inline void __core2_vpmu_save(struct vcpu *v)
@@ -268,7 +275,7 @@ static inline void __core2_vpmu_save(struct vcpu *v)
         rdmsrl(MSR_P6_EVNTSEL(i), xen_pmu_cntr_pair[i].control);
     }
 
-    if ( !is_hvm_vcpu(v) )
+    if ( !is_hvm_vcpu(v) || vpmu_version >= 4 )
         rdmsrl(MSR_CORE_PERF_GLOBAL_STATUS, core2_vpmu_cxt->global_status);
     /* Save MSR to private context to make it fork-friendly */
     else if ( mem_sharing_enabled(v->domain) )
@@ -338,6 +345,15 @@ static inline void __core2_vpmu_load(struct vcpu *v)
     else if ( mem_sharing_is_fork(v->domain) )
         vmx_write_guest_msr(v, MSR_CORE_PERF_GLOBAL_CTRL,
                             core2_vpmu_cxt->global_ctrl);
+
+    if ( vpmu_version >= 4 ) {
+        const uint64_t global_status = core2_vpmu_cxt->global_status;
+        const uint64_t reset = (~global_status) & global_ovf_ctrl_mask ;
+        if ( reset )
+            wrmsrl(MSR_CORE_PERF_GLOBAL_OVF_CTRL, reset);
+        if ( global_status )
+            wrmsrl(MSR_CORE_PERF_GLOBAL_STATUS_SET, global_status);
+    }
 }
 
 static int core2_vpmu_verify(struct vcpu *v)
@@ -917,6 +933,7 @@ const struct arch_vpmu_ops *__init core2_vpmu_init(void)
         printk(XENLOG_INFO "VPMU: PMU version %u is not fully supported. "
                "Emulating version %d\n", version, VPMU_VERSION_MAX);
     }
+    vpmu_version = version;
 
     if ( current_cpu_data.x86 != 6 )
     {
-- 
2.41.0

[RFC PATCH 19/22] x86/PMUv4: bump max PMU version to 4

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Depends on the other x86/PMUv4 patches:
"x86/PMUv4: disable intercept for PERF_GLOBAL_STATUS"
"x86/PMUv4: IA32_PERF_GLOBAL_{STATUS_SET, INUSE} support"
"x86/PMUv4: support LBR_Frz and CTR_Frz"

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/include/asm/vpmu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index 79f7f4a09e..eaededadb5 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -74,7 +74,7 @@ struct vpmu_struct {
 #define VPMU_CPU_HAS_DS                     0x1000 /* Has Debug Store */
 #define VPMU_CPU_HAS_BTS                    0x2000 /* Has Branch Trace Store */
 
-#define VPMU_VERSION_MAX                    0x3
+#define VPMU_VERSION_MAX                    0x4
 #define VPMU_VERSION_MAX_SUPPORTED          0x5
 
 static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask)
-- 
2.41.0

[RFC PATCH 20/22] x86/PMUv5: limit available fixed PMCs and enable support

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

AnyThread deprecation means a bit in 0xa edx, which we pass through.
(we could also avoid doing the anythread masking, but we need that
 for version <= 4 support).

Fixed Counter enumeration means we need to limit fixed counters if we
hide any.

Domain separation needs no action from the hypervisor AFAICT.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpuid.c            | 3 ++-
 xen/arch/x86/include/asm/vpmu.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 12e768ae87..8900943bcd 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -338,7 +338,8 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
             res->a = u.eax;
 
             /* We only implement 3 fixed function counters */
-            if ( (res->d & 0x1f) > fixed_pmc_cnt )
+            res->c &= ~((1 << fixed_pmc_cnt) - 1);
+            if ( (res->d & 0x1f) > fixed_pmc_cnt)
                 res->d = (res->d & ~0x1f) | fixed_pmc_cnt;
         }
         break;
diff --git a/xen/arch/x86/include/asm/vpmu.h b/xen/arch/x86/include/asm/vpmu.h
index eaededadb5..f066b17e45 100644
--- a/xen/arch/x86/include/asm/vpmu.h
+++ b/xen/arch/x86/include/asm/vpmu.h
@@ -74,7 +74,7 @@ struct vpmu_struct {
 #define VPMU_CPU_HAS_DS                     0x1000 /* Has Debug Store */
 #define VPMU_CPU_HAS_BTS                    0x2000 /* Has Branch Trace Store */
 
-#define VPMU_VERSION_MAX                    0x4
+#define VPMU_VERSION_MAX                    0x5
 #define VPMU_VERSION_MAX_SUPPORTED          0x5
 
 static inline void vpmu_set(struct vpmu_struct *vpmu, const u32 mask)
-- 
2.41.0

[RFC PATCH 21/22] x86/AMD: fix CPUID for PerfCtr{4,5}

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

These are available, but were hidden by CPUID previously.

There are IR (all guests), NB and L2I (dom0 only) performance counters too
that need to be implemented, add placeholder entries for them.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/cpu-policy.c                   | 14 +++++++++++---
 xen/arch/x86/hvm/svm/svm.c                  |  1 +
 xen/arch/x86/pv/emul-priv-op.c              |  1 +
 xen/include/public/arch-x86/cpufeatureset.h |  4 ++++
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index e38b648f7d..4242a21e1d 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -340,9 +340,16 @@ static void recalculate_misc(struct cpu_policy *p)
         p->extd.raw[0x1e] = EMPTY_LEAF; /* TopoExt APIC ID/Core/Node */
         p->extd.raw[0x1f] = EMPTY_LEAF; /* SEV */
         p->extd.raw[0x20] = EMPTY_LEAF; /* Platform QoS */
-        break;
-    }
-}
+
+        /* These are not implemented yet, hide from CPUID.
+         * When they become implemented, make them available when full vpmu is on */
+        p->extd.irperf = 0;
+        p->extd.perfctrextnb = 0;
+        p->extd.perfctrextl2i = 0;
+
+         break;
+     }
+ }
 
 void calculate_raw_cpu_policy(void)
 {
@@ -391,6 +398,7 @@ static void __init calculate_host_policy(void)
     if ( vpmu_mode == XENPMU_MODE_OFF ) {
         p->basic.raw[0xa] = EMPTY_LEAF;
         p->basic.pdcm = 0;
+        p->extd.perfctrextcore = 0;
     }
 
     if ( p->extd.svm )
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 45f8e1ffd1..ecb6184f51 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1905,6 +1905,7 @@ static int cf_check svm_msr_read_intercept(
     case MSR_AMD_FAM15H_EVNTSEL3:
     case MSR_AMD_FAM15H_EVNTSEL4:
     case MSR_AMD_FAM15H_EVNTSEL5:
+    /* TODO: IRPerfCnt, L2I_* and NB_* support */
         if ( vpmu_do_rdmsr(msr, msr_content) )
             goto gpf;
         break;
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 105485bb1e..8d802b5df0 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -1156,6 +1156,7 @@ static int cf_check write_msr(
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         {
             vpmu_msr = true;
+            /* fall-through */
     case MSR_AMD_FAM15H_EVNTSEL0 ... MSR_AMD_FAM15H_PERFCTR5:
     case MSR_K7_EVNTSEL0 ... MSR_K7_PERFCTR3:
             if ( vpmu_msr || (boot_cpu_data.x86_vendor &
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 0aa3251397..5faca0bf7a 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -166,7 +166,10 @@ XEN_CPUFEATURE(FMA4,          3*32+16) /*A  4 operands MAC instructions */
 XEN_CPUFEATURE(NODEID_MSR,    3*32+19) /*   NodeId MSR */
 XEN_CPUFEATURE(TBM,           3*32+21) /*A  trailing bit manipulations */
 XEN_CPUFEATURE(TOPOEXT,       3*32+22) /*   topology extensions CPUID leafs */
+XEN_CPUFEATURE(PERFCTREXTCORE, 3*32+23) /*A! Extended core performance event-select registers */
+XEN_CPUFEATURE(PERFCTREXTNB,  3*32+24) /*   Extended Northbridge performance counters */
 XEN_CPUFEATURE(DBEXT,         3*32+26) /*A  data breakpoint extension */
+XEN_CPUFEATURE(PERFCTREXTL2I, 3*32+28) /*   Extended L2 cache performance counters */
 XEN_CPUFEATURE(MONITORX,      3*32+29) /*   MONITOR extension (MONITORX/MWAITX) */
 
 /* Intel-defined CPU features, CPUID level 0x0000000D:1.eax, word 4 */
@@ -238,6 +241,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF Read Only interface */
 
 /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
 XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
+XEN_CPUFEATURE(IRPERF,        8*32+ 1) /* Instruction Retired Performance Counter */
 XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
 XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
 XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */
-- 
2.41.0

[RFC PATCH 22/22] x86/AMD: add IRPerf support

[Edwin Török]

From: Edwin Török <edvin.torok@citrix.com>

Instruction retired perf counter, enabled by writing to a bit in HWCR.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
---
 xen/arch/x86/include/asm/msr-index.h        | 1 +
 xen/arch/x86/msr.c                          | 7 +++++++
 xen/include/public/arch-x86/cpufeatureset.h | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h b/xen/arch/x86/include/asm/msr-index.h
index 061b07c7ae..1d94fe3a5b 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -393,6 +393,7 @@
 
 #define MSR_K8_HWCR			0xc0010015
 #define K8_HWCR_TSC_FREQ_SEL		(1ULL << 24)
+#define K8_HWCR_IRPERF_EN		(1ULL << 30)
 #define K8_HWCR_CPUID_USER_DIS		(1ULL << 35)
 
 #define MSR_K7_FID_VID_CTL		0xc0010041
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 483b5e4f70..b3cd851d9d 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -584,6 +584,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
         }
         break;
 
+    case MSR_K8_HWCR:
+        if ( !(cp->x86_vendor & X86_VENDOR_AMD) ||
+             (val & ~K8_HWCR_IRPERF_EN) ||
+             wrmsr_safe(msr, val) != 0 )
+            goto gp_fault;
+        break;
+
     case MSR_AMD64_DE_CFG:
         /*
          * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 5faca0bf7a..40f74cd5e8 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -241,7 +241,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF Read Only interface */
 
 /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
 XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
-XEN_CPUFEATURE(IRPERF,        8*32+ 1) /* Instruction Retired Performance Counter */
+XEN_CPUFEATURE(IRPERF,        8*32+ 1) /*A! Instruction Retired Performance Counter */
 XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
 XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
 XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */
-- 
2.41.0

Re: [RFC PATCH 01/22] x86/msr: MSR_PLATFORM_INFO shouldn't claim that turbo is programmable

[Andrew Cooper]

On 25/10/2023 8:29 pm, Edwin Török wrote:
> From: Edwin Török <edvin.torok@citrix.com>
>
> Xen forbids writes to the various turbo control MSRs, however MSR_PLATFORM_INFO claims that these MSRs are writable.
> Override MSR_PLATFORM_INFO bits to indicate lack of support.
>
> See Intel SDM Volume 4, 2.17.6 "MSRs Introduced in the Intel Xeon Scaslable Processor Family",
> which describes that MSR_PLATFORM_INFO.[28] = 1 implies that MSR_TURBO_RATIO_LIMIT is R/W,
> and similarly bit 29 for TDP control, and bit 30 for MSR_TEMPERATURE_TARGET.
>
> These bits were not all present on earlier processors, however where missing the bits were reserved,
> and when present they are always present in the same bits.
>
> (Curiously bit 31 that Xen uses is not documented anywhere in this manual but a separate one).
>
> Backport: 4.0+
>
> Signed-off-by: Edwin Török <edvin.torok@citrix.com>

p->platform_info never has any bit other than cpuid_faulting set in it. 
We still don't even report the proper raw value, because we don't (yet)
have clean MSR derivation logic.

I'm confused as to how you managed to find these set.  Even back in Xen
4.13, PLATFORM_INFO was covered by the msr_policy (later merged into
cpu_policy).  Furthermore, even patch 3 oughtn't to have such an effect.

Sadly, the whole of this MSR is model specific.  Vol4 2.17 is only for
one SKX/CLX/ICX/SPR.  Technically its wrong to treat the cpuid_faulting
in the way we do, but it is enumerated separately, and we intentionally
don't have an Intel check because we need to emulate CPUID faulting on
AMD hardware to make PVShim work.

~Andrew

Re: [RFC PATCH 22/22] x86/AMD: add IRPerf support

[Andrew Cooper]

On 25/10/2023 8:29 pm, Edwin Török wrote:
> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
> index 483b5e4f70..b3cd851d9d 100644
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -584,6 +584,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
>          }
>          break;
>  
> +    case MSR_K8_HWCR:
> +        if ( !(cp->x86_vendor & X86_VENDOR_AMD) ||
> +             (val & ~K8_HWCR_IRPERF_EN) ||
> +             wrmsr_safe(msr, val) != 0 )
> +            goto gp_fault;
> +        break;
> +
>      case MSR_AMD64_DE_CFG:
>          /*
>           * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
> index 5faca0bf7a..40f74cd5e8 100644
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -241,7 +241,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF Read Only interface */
>  
>  /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
>  XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
> -XEN_CPUFEATURE(IRPERF,        8*32+ 1) /* Instruction Retired Performance Counter */
> +XEN_CPUFEATURE(IRPERF,        8*32+ 1) /*A! Instruction Retired Performance Counter */
>  XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
>  XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
>  XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */

Last things first.  You can advertise this bit to guests, but I'm
looking at the AMD manuals and woefully little information on what this
is an how it works.

It does have an architectural enumeration, but there isn't even a a
description of what HWCR.IPERF_EN does.

The HWCR documentation says "enables Core::X86::Msr::IRPerfCount", but
the IRPERF MSR says no such think, noting only that EFRO_LOCK makes the
MSR non-writeable (which in turn means we can't always offer it to
guests anyway.  See below).

At a guess, the HWCR bit activates the counter, but nothing states
this.  At a guess, it's a free-running, reset-able counter, but again
nothing states this.  The manual just says "is a dedicated counter" and
doesn't even state whether it is (or is not) tied into the other core
fixed counters and whether it is integrated with PMI or not.

Which brings us on to the middle hunk.  Putting it there short circuits
the PV-specific handling, but you can't do that in general without
arranging for HWCR to be context switched properly for vCPUs, nor in
this case IPERF_COUNT itself.

Unless you context switch HWCR and IPERF_COUNT, a guest will see it not
counting, or the count going backwards, 50% of the time as the vCPU is
scheduled around.

So while in principle we could let guests use this facility (it would
have to be off by default, because there doesn't appear to be any
filtering capability in it at all, so we can't restrict it to
instructions retired in guest context), it will need a far larger patch
than this to do it safely.

~Andrew

Re: [RFC PATCH 22/22] x86/AMD: add IRPerf support

[Edwin Torok]

> On 25 Oct 2023, at 21:59, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> On 25/10/2023 8:29 pm, Edwin Török wrote:
>> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
>> index 483b5e4f70..b3cd851d9d 100644
>> --- a/xen/arch/x86/msr.c
>> +++ b/xen/arch/x86/msr.c
>> @@ -584,6 +584,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
>>         }
>>         break;
>> 
>> +    case MSR_K8_HWCR:
>> +        if ( !(cp->x86_vendor & X86_VENDOR_AMD) ||
>> +             (val & ~K8_HWCR_IRPERF_EN) ||
>> +             wrmsr_safe(msr, val) != 0 )
>> +            goto gp_fault;
>> +        break;
>> +
>>     case MSR_AMD64_DE_CFG:
>>         /*
>>          * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
>> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
>> index 5faca0bf7a..40f74cd5e8 100644
>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>> @@ -241,7 +241,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF Read Only interface */
>> 
>> /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
>> XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
>> -XEN_CPUFEATURE(IRPERF,        8*32+ 1) /* Instruction Retired Performance Counter */
>> +XEN_CPUFEATURE(IRPERF,        8*32+ 1) /*A! Instruction Retired Performance Counter */
>> XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
>> XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
>> XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */
> 
> Last things first.  You can advertise this bit to guests, but I'm
> looking at the AMD manuals and woefully little information on what this
> is an how it works.
> 
> It does have an architectural enumeration, but there isn't even a a
> description of what HWCR.IPERF_EN does.
> 
> The HWCR documentation says "enables Core::X86::Msr::IRPerfCount", but
> the IRPERF MSR says no such think,

> noting only that EFRO_LOCK makes the
> MSR non-writeable (which in turn means we can't always offer it to
> guests anyway.  See below).

> At a guess, the HWCR bit activates the counter, but nothing states
> this.


My version (https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf) says:
"This is a dedicated counter that is always counting instructions retired. It exists at MSR address C000_00E9. It is enabled by setting a 1 to HWCR[30] and its support is indicated by CPUID Fn8000_0008_EBX[1]."

Although digging a bit more there are some erratas around deep C states on some chips and the HWCR register itself is not global but per CCD (which is neatly buried in the _ccd[7:0]_.... smallprint at the top:
https://github.com/cyring/CoreFreq/issues/206#issuecomment-722713147


>   At a guess, it's a free-running, reset-able counter, but again
> nothing states this.  The manual just says "is a dedicated counter" and
> doesn't even state whether it is (or is not) tied into the other core
> fixed counters and whether it is integrated with PMI or not.

There is also a LWP 'Instructions Retired' described in 13.4.2.2, which is user-mode only and per-core, but I assume that is completely different from this MSR
and part of the features that got dropped in newer cores.

> 
> Which brings us on to the middle hunk.  Putting it there short circuits
> the PV-specific handling, but you can't do that in general without
> arranging for HWCR to be context switched properly for vCPUs, nor in
> this case IPERF_COUNT itself.
> 
> Unless you context switch HWCR and IPERF_COUNT, a guest will see it not
> counting, or the count going backwards, 50% of the time as the vCPU is
> scheduled around.
> 
> So while in principle we could let guests use this facility (it would
> have to be off by default, because there doesn't appear to be any
> filtering capability in it at all, so we can't restrict it to
> instructions retired in guest context), it will need a far larger patch
> than this to do it safely.

Thanks, I'll move this patch into the 'needs more work' category.


Thanks,
--Edwin

Re: [RFC PATCH 01/22] x86/msr: MSR_PLATFORM_INFO shouldn't claim that turbo is programmable

[Edwin Torok]

> On 25 Oct 2023, at 21:33, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> On 25/10/2023 8:29 pm, Edwin Török wrote:
>> From: Edwin Török <edvin.torok@citrix.com>
>> 
>> Xen forbids writes to the various turbo control MSRs, however MSR_PLATFORM_INFO claims that these MSRs are writable.
>> Override MSR_PLATFORM_INFO bits to indicate lack of support.
>> 
>> See Intel SDM Volume 4, 2.17.6 "MSRs Introduced in the Intel Xeon Scaslable Processor Family",
>> which describes that MSR_PLATFORM_INFO.[28] = 1 implies that MSR_TURBO_RATIO_LIMIT is R/W,
>> and similarly bit 29 for TDP control, and bit 30 for MSR_TEMPERATURE_TARGET.
>> 
>> These bits were not all present on earlier processors, however where missing the bits were reserved,
>> and when present they are always present in the same bits.
>> 
>> (Curiously bit 31 that Xen uses is not documented anywhere in this manual but a separate one).
>> 
>> Backport: 4.0+
>> 
>> Signed-off-by: Edwin Török <edvin.torok@citrix.com>
> 
> p->platform_info never has any bit other than cpuid_faulting set in it. 
> We still don't even report the proper raw value, because we don't (yet)
> have clean MSR derivation logic.
> 
> I'm confused as to how you managed to find these set.  Even back in Xen
> 4.13, PLATFORM_INFO was covered by the msr_policy (later merged into
> cpu_policy).  Furthermore, even patch 3 oughtn't to have such an effect.
> 
> Sadly, the whole of this MSR is model specific.  Vol4 2.17 is only for
> one SKX/CLX/ICX/SPR.  Technically its wrong to treat the cpuid_faulting
> in the way we do, but it is enumerated separately, and we intentionally
> don't have an Intel check because we need to emulate CPUID faulting on
> AMD hardware to make PVShim work.
> 

xen/lib/x86/msr.c:    COPY_MSR(MSR_INTEL_PLATFORM_INFO, p->platform_info.raw);

This code made me believe that the underlying MSR value was copied, and only specific values from it were overwritten, I didn't spot any zeroing.
Although running rdmsr now (on 4.13.5) does show that all the other bits are zero, so the zeroing must happen somewhere else in the code, making this patch obsolete.
I'll drop it.

Thanks,
--Edwin

> ~Andrew

Re: [RFC PATCH 22/22] x86/AMD: add IRPerf support

[Andrew Cooper]

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

On 26/10/2023 5:39 pm, Edwin Torok wrote:
>> On 25 Oct 2023, at 21:59, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
>>
>> On 25/10/2023 8:29 pm, Edwin Török wrote:
>>> diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
>>> index 483b5e4f70..b3cd851d9d 100644
>>> --- a/xen/arch/x86/msr.c
>>> +++ b/xen/arch/x86/msr.c
>>> @@ -584,6 +584,13 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
>>>         }
>>>         break;
>>>
>>> +    case MSR_K8_HWCR:
>>> +        if ( !(cp->x86_vendor & X86_VENDOR_AMD) ||
>>> +             (val & ~K8_HWCR_IRPERF_EN) ||
>>> +             wrmsr_safe(msr, val) != 0 )
>>> +            goto gp_fault;
>>> +        break;
>>> +
>>>     case MSR_AMD64_DE_CFG:
>>>         /*
>>>          * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
>>> diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
>>> index 5faca0bf7a..40f74cd5e8 100644
>>> --- a/xen/include/public/arch-x86/cpufeatureset.h
>>> +++ b/xen/include/public/arch-x86/cpufeatureset.h
>>> @@ -241,7 +241,7 @@ XEN_CPUFEATURE(EFRO,          7*32+10) /*   APERF/MPERF Read Only interface */
>>>
>>> /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */
>>> XEN_CPUFEATURE(CLZERO,        8*32+ 0) /*A  CLZERO instruction */
>>> -XEN_CPUFEATURE(IRPERF,        8*32+ 1) /* Instruction Retired Performance Counter */
>>> +XEN_CPUFEATURE(IRPERF,        8*32+ 1) /*A! Instruction Retired Performance Counter */
>>> XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
>>> XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
>>> XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */
>> Last things first.  You can advertise this bit to guests, but I'm
>> looking at the AMD manuals and woefully little information on what this
>> is an how it works.
>>
>> It does have an architectural enumeration, but there isn't even a a
>> description of what HWCR.IPERF_EN does.
>>
>> The HWCR documentation says "enables Core::X86::Msr::IRPerfCount", but
>> the IRPERF MSR says no such think,
>> noting only that EFRO_LOCK makes the
>> MSR non-writeable (which in turn means we can't always offer it to
>> guests anyway.  See below).
>> At a guess, the HWCR bit activates the counter, but nothing states
>> this.
>
> My version (https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf) says:
> "This is a dedicated counter that is always counting instructions retired. It exists at MSR address C000_00E9. It is enabled by setting a 1 to HWCR[30] and its support is indicated by CPUID Fn8000_0008_EBX[1]."
>
> Although digging a bit more there are some erratas around deep C states on some chips and the HWCR register itself is not global but per CCD (which is neatly buried in the _ccd[7:0]_.... smallprint at the top:
> https://github.com/cyring/CoreFreq/issues/206#issuecomment-722713147

HWCR is per-thread, and that's what both screenshots there say.  Ryzen's
don't typically have CCDs, which is why CCD is missing from the scope
description.

Consider the implications of making bit 30 work if the MSR was CCD-wide,
seeing as the counting really does need to be done in at the core level.

Or for that matter bit 35 (the CPUID-faulting enable bit).
>>   At a guess, it's a free-running, reset-able counter, but again
>> nothing states this.  The manual just says "is a dedicated counter" and
>> doesn't even state whether it is (or is not) tied into the other core
>> fixed counters and whether it is integrated with PMI or not.
> There is also a LWP 'Instructions Retired' described in 13.4.2.2, which is user-mode only and per-core, but I assume that is completely different from this MSR
> and part of the features that got dropped in newer cores.

LWP doesn't exist any more, even in older processors.

It was the feature sacrificed to make enough microcode space to retrofit
IBPB in microcode for speculation protections.

~Andrew

[-- Attachment #2: Type: text/html, Size: 5587 bytes --]

Re: [RFC PATCH 12/22] x86/msr-index: define more architectural MSRs

[Jan Beulich]

On 25.10.2023 21:29, Edwin Török wrote:
> From: Edwin Török <edvin.torok@citrix.com>
> 
> Add most architectural MSRs, except those behind CPUID features that are
> not yet implemented, such as TME, SGX.

I'm not convinced we should blindly add MSR definitions for ones we
don't use. But if you do, ...

> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -58,6 +58,14 @@
>  #define  PRED_CMD_IBPB                      (_AC(1, ULL) <<  0)
>  #define  PRED_CMD_SBPB                      (_AC(1, ULL) <<  7)
>  
> +#define MSR_IA32_SMM_MONITOR_CTL            0x0000009b
> +#define MSR_IA32_SMBASE                     0x0000009e
> +#define MSR_IA32_SMRR_PHYSBASE              0x000001f2
> +#define MSR_IA32_SMRR_PHYSMASK              0x000001f3
> +#define MSR_IA32_PLATFORM_DCA_CAP           0x000001f8
> +#define MSR_IA32_CPU_DCA_CAP                0x000001f9
> +#define MSR_IA32_DCA_0_CAP                  0x000001fa
> +
>  #define MSR_PPIN_CTL                        0x0000004e
>  #define  PPIN_LOCKOUT                       (_AC(1, ULL) <<  0)
>  #define  PPIN_ENABLE                        (_AC(1, ULL) <<  1)
> @@ -267,13 +275,21 @@
>  #define MSR_IA32_MCG_CAP		0x00000179
>  #define MSR_IA32_MCG_STATUS		0x0000017a
>  #define MSR_IA32_MCG_CTL		0x0000017b
> -#define MSR_IA32_MCG_EXT_CTL	0x000004d0
> +#define MSR_IA32_MCG_EXT_CTL		0x000004d0

... please obey to the comment a few lines up from here: Altering
indentation is kind of okay, but most of what you add below here
should be added (well-formed) above that comment.

Jan

>  #define MSR_IA32_PEBS_ENABLE		0x000003f1
>  #define MSR_IA32_DS_AREA		0x00000600
>  #define MSR_IA32_PERF_CAPABILITIES	0x00000345
>  /* Lower 6 bits define the format of the address in the LBR stack */
> -#define MSR_IA32_PERF_CAP_LBR_FORMAT	0x3f
> +#define MSR_IA32_PERF_CAP_LBR_FORMAT    	0x3f
> +#define MSR_IA32_PERF_CAP_PEBS_TRAP		(_AC(1,ULL) << 6)
> +#define MSR_IA32_PERF_CAP_PEBS_SAVE_ARCH_REGS	(_AC(1,ULL) << 7)
> +#define MSR_IA32_PERF_CAP_PEBS_RECORD_FORMAT	0xf00
> +#define MSR_IA32_PERF_CAP_FREEZE_WHILE_SMM	(_AC(1,ULL) << 12)
> +#define MSR_IA32_PERF_CAP_FULLWIDTH_PMC 	(_AC(1,ULL) << 13)
> +#define MSR_IA32_PERF_CAP_PEBS_BASELINE		(_AC(1,ULL) << 14)
> +#define MSR_IA32_PERF_CAP_PERF_METRICS		(_AC(1,ULL) << 15)
> +#define MSR_IA32_PERF_CAP_PEBS_TO_PT		(_AC(1,ULL) << 16)
>  
>  #define MSR_IA32_BNDCFGS		0x00000d90
>  #define IA32_BNDCFGS_ENABLE		0x00000001
> @@ -307,6 +323,8 @@
>  #define IA32_DEBUGCTLMSR_BTS_OFF_USR	(1<<10) /* BTS off if CPL > 0 */
>  #define IA32_DEBUGCTLMSR_FREEZE_LBRS_ON_PMI	(1<<11) /* LBR stack frozen on PMI */
>  #define IA32_DEBUGCTLMSR_FREEZE_PERFMON_ON_PMI	(1<<12) /*  Global counter control ENABLE bit frozen on PMI */
> +#define IA32_DEBUGCTLMSR_ENABLE_UNCORE_PMI	(1<<13) /* Enable uncore PMI */
> +#define IA32_DEBUGCTLMSR_FREEZE_WHILE_SMM	(1<<14) /* Freeze perfmon/trace while in SMM */
>  #define IA32_DEBUGCTLMSR_RTM			(1<<15) /* RTM debugging enable */
>  
>  #define MSR_IA32_LASTBRANCHFROMIP	0x000001db
> @@ -469,6 +487,7 @@
>  #define MSR_VIA_RNG			0x0000110b
>  
>  /* Intel defined MSRs. */
> +#define MSR_IA32_MONITOR_FILTER_SIZE	0x00000006
>  #define MSR_IA32_TSC			0x00000010
>  #define MSR_IA32_PLATFORM_ID		0x00000017
>  #define MSR_IA32_EBL_CR_POWERON		0x0000002a
> @@ -491,6 +510,7 @@
>  #define MSR_IA32_PERF_STATUS		0x00000198
>  #define MSR_IA32_PERF_CTL		0x00000199
>  
> +#define MSR_IA32_UMWAIT_CONTROL		0x000000e1
>  #define MSR_IA32_MPERF			0x000000e7
>  #define MSR_IA32_APERF			0x000000e8
>  
> @@ -498,6 +518,7 @@
>  #define MSR_IA32_THERM_INTERRUPT	0x0000019b
>  #define MSR_IA32_THERM_STATUS		0x0000019c
>  #define MSR_IA32_MISC_ENABLE		0x000001a0
> +#define MSR_IA32_MISC_ENABLE_FAST_STRINGS (1<<0)
>  #define MSR_IA32_MISC_ENABLE_PERF_AVAIL   (1<<7)
>  #define MSR_IA32_MISC_ENABLE_BTS_UNAVAIL  (1<<11)
>  #define MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL (1<<12)
> @@ -508,15 +529,38 @@
>  #define MSR_IA32_MISC_ENABLE_TURBO_DISENGAGE (_AC(1, ULL) << 38)
>  
>  #define MSR_IA32_TSC_DEADLINE		0x000006E0
> +
> +#define MSR_IA32_PM_ENABLE		0x00000770
> +#define MSR_IA32_HWP_CAPABILITIES	0x00000771
> +#define MSR_IA32_HWP_REQUEST_PKG	0x00000772
> +#define MSR_IA32_HWP_INTERRUPT		0x00000773
> +#define MSR_IA32_HWP_REQUEST		0x00000774
> +#define MSR_IA32_PECI_HWP_REQUEST_INFO	0x00000775
> +#define MSR_IA32_HWP_STATUS		0x00000777
> +
> +#define MSR_IA32_PKG_HDC_CTL		0x00000db0
> +#define MSR_IA32_PM_CTL1		0x00000db1
> +#define MSR_IA32_THREAD_STALL		0x00000db2
> +#define MSR_IA32_HW_FEEDBACK_PTR	0x000017d0
> +#define MSR_IA32_HW_FEEDBACK_CONFIG	0x000017d1
> +
> +#define MSR_TEMPERATURE_TARGET		0x000001a2
> +#define MSR_TURBO_RATIO_LIMIT		0x000001ad
> +#define MSR_TURBO_RATIO_LIMIT1		0x000001ae
> +#define MSR_TURBO_RATIO_LIMIT2		0x000001af
> +
>  #define MSR_IA32_ENERGY_PERF_BIAS	0x000001b0
> +#define MSR_IA32_PACKAGE_THERM_STATUS	0x000001b1
> +#define MSR_IA32_PACKAGE_THERM_INTERRUPT 0x000001b2
>  
>  /* Platform Shared Resource MSRs */
> +#define MSR_IA32_PSR_L3_QOS_CFG		0x00000c81
> +#define MSR_IA32_PSR_L2_QOS_CFG		0x00000c82
>  #define MSR_IA32_CMT_EVTSEL		0x00000c8d
>  #define MSR_IA32_CMT_EVTSEL_UE_MASK	0x0000ffff
>  #define MSR_IA32_CMT_CTR		0x00000c8e
>  #define MSR_IA32_PSR_ASSOC		0x00000c8f
> -#define MSR_IA32_PSR_L3_QOS_CFG	0x00000c81
> -#define MSR_IA32_PSR_L3_MASK(n)	(0x00000c90 + (n))
> +#define MSR_IA32_PSR_L3_MASK(n)		(0x00000c90 + (n))
>  #define MSR_IA32_PSR_L3_MASK_CODE(n)	(0x00000c90 + (n) * 2 + 1)
>  #define MSR_IA32_PSR_L3_MASK_DATA(n)	(0x00000c90 + (n) * 2)
>  #define MSR_IA32_PSR_L2_MASK(n)		(0x00000d10 + (n))
> @@ -682,6 +726,8 @@
>  #define MSR_CORE_PERF_GLOBAL_STATUS	0x0000038e
>  #define MSR_CORE_PERF_GLOBAL_CTRL	0x0000038f
>  #define MSR_CORE_PERF_GLOBAL_OVF_CTRL	0x00000390
> +#define MSR_CORE_PERF_GLOBAL_STATUS_SET	0x00000391
> +#define MSR_CORE_PERF_GLOBAL_INUSE	0x00000392
>  
>  /* Intel cpuid spoofing MSRs */
>  #define MSR_INTEL_MASK_V1_CPUID1        0x00000478

Re: [RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

[Jan Beulich]

On 25.10.2023 21:29, Edwin Török wrote:
> To more easily lookup the semantics of these MSRs add references to
> vendor manuals.

As much as I appreciate the intention, ...

> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -13,6 +13,16 @@
>   * Blocks of related constants should be sorted by MSR index.  The constant
>   * names should be as concise as possible, and the bit names may have an
>   * abbreviated name.  Exceptions will be considered on a case-by-case basis.
> + *
> + * References:
> + * - https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
> + *    Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific registers
> + *    Chapter 2, "Model-Specific Registers (MSRs)"

... at least Intel's URL has changed several times over the years. Volume
and chapter numbers change even more frequently. Any such is liable to go
stale at some point.

Jan

> + * - https://developer.amd.com/resources/developer-guides-manuals/
> + *    AMD64 Architecture Programmer’s Manual Volume 2: System Programming
> + *    Appendix A "MSR Cross-Reference"
> + *
>   */
>  
>  #define MSR_P5_MC_ADDR                      0

Re: [RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Jan Beulich]

On 25.10.2023 21:29, Edwin Török wrote:
> Dom0 should always be able to read this MSR: it is useful when
> investigating performance issues in production.

While I'm not outright opposed, I'm also not convinced. At the very least
...

> Although the count is Thread scoped, in practice all cores were observed
> to return the same count (perhaps due to implementation details of SMM),
> so do not require the cpu to be pinned in order to read it.

... this, even if matching your observations, is going to be properly
misleading in case counts end up diverging.

> This MSR exists on Intel since Nehalem.
> 
> Backport: 4.15+

If this was a backporting candidate, I think a Fixes: tag would need
to indicate what's being fixed here. Otherwise this is merely a new
feature.

> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -641,6 +641,9 @@
>  #define MSR_NHL_LBR_SELECT		0x000001c8
>  #define MSR_NHL_LASTBRANCH_TOS		0x000001c9
>  
> +/* Nehalem and newer other MSRs */
> +#define MSR_SMI_COUNT                       0x00000034

See my comment on the other patch regarding additions here.

> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -139,6 +139,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>          *val = msrs->misc_features_enables.raw;
>          break;
>  
> +    case MSR_SMI_COUNT:
> +        if ( cp->x86_vendor != X86_VENDOR_INTEL )
> +            goto gp_fault;
> +        if ( is_hardware_domain(d) && !rdmsr_safe(msr, *val) )
> +            break;
> +        return X86EMUL_UNHANDLEABLE;

Why #GP for non-Intel but UNHANDLEABLE for DomU?

Jan

Re: [RFC PATCH 03/22] x86/msr: always allow a pinned Dom0 to read any unknown MSR

[Jan Beulich]

On 25.10.2023 21:29, Edwin Török wrote:
> This can be useful if you realize you have to inspect the value of an
> MSR in production, without having to change into a new Xen first that
> handles the MSR.

Yet on a non-pinned Dom0 you'd still be lost. Since iirc we generally
advise against pinning, I wonder of how much use such a change would be,
when it effectively undoes what we deliberately did a while ago.

> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1933,6 +1933,9 @@ static int cf_check svm_msr_read_intercept(
>          break;
>  
>      default:
> +        if ( is_hwdom_pinned_vcpu(v) && !rdmsr_safe(msr, *msr_content) )
> +            break;
> +
>          if ( d->arch.msr_relaxed && !rdmsr_safe(msr, tmp) )
>          {
>              *msr_content = 0;

If we went as far as undoing some of what was done, I'd then wonder
whether instead we should mandate relaxed mode to be enabled on such a
Dom0. Then, instead of returning fake 0 here, the actual value could
be returned in the specific case of (pinned?) Dom0.

Jan

Re: [RFC PATCH 03/22] x86/msr: always allow a pinned Dom0 to read any unknown MSR

[Edwin Torok]

> On 30 Oct 2023, at 16:29, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 25.10.2023 21:29, Edwin Török wrote:
>> This can be useful if you realize you have to inspect the value of an
>> MSR in production, without having to change into a new Xen first that
>> handles the MSR.
> 
> Yet on a non-pinned Dom0 you'd still be lost. Since iirc we generally
> advise against pinning,

You can temporarily pin while debugging the issue (e.g. pin just 1 CPU from Dom0, and "walk" all your physical CPUs with it if you have to,
so that you query them all), e.g. with 'xl vcpu-pin'.
Although that is more invasive than reading a value.
 
Or alternatively have another (privileged) interface to read the MSR for a given core without exposing it to any guests, that way you don't affect the running system at all
(which would be preferable in a production environment), i.e. a Xen equivalent of 'rdmsr'.

> I wonder of how much use such a change would be,
> when it effectively undoes what we deliberately did a while ago.
> 
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -1933,6 +1933,9 @@ static int cf_check svm_msr_read_intercept(
>>         break;
>> 
>>     default:
>> +        if ( is_hwdom_pinned_vcpu(v) && !rdmsr_safe(msr, *msr_content) )
>> +            break;
>> +
>>         if ( d->arch.msr_relaxed && !rdmsr_safe(msr, tmp) )
>>         {
>>             *msr_content = 0;
> 
> If we went as far as undoing some of what was done, I'd then wonder
> whether instead we should mandate relaxed mode to be enabled on such a
> Dom0. Then, instead of returning fake 0 here, the actual value could
> be returned in the specific case of (pinned?) Dom0.


Can relaxed mode be enabled at runtime? I'd be happy with either solution, but it should be something that can be enabled at runtime
(if you have to reboot Xen then you may lose the bug repro that you want to gather more information on).
Although changing such a setting in a production environment may still be risky, because the guest will then become very confused that it has previously read some 0s, now there are some real values, and later when you flip the switch off it gets 0s again.

Best regards,
--Edwin

Re: [RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Edwin Torok]

> On 30 Oct 2023, at 16:20, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 25.10.2023 21:29, Edwin Török wrote:
>> Dom0 should always be able to read this MSR: it is useful when
>> investigating performance issues in production.
> 
> While I'm not outright opposed, I'm also not convinced. At the very least
> ...
> 
>> Although the count is Thread scoped, in practice all cores were observed
>> to return the same count (perhaps due to implementation details of SMM),
>> so do not require the cpu to be pinned in order to read it.
> 
> ... this, even if matching your observations, is going to be properly
> misleading in case counts end up diverging.
> 
>> This MSR exists on Intel since Nehalem.
>> 
>> Backport: 4.15+
> 
> If this was a backporting candidate, I think a Fixes: tag would need
> to indicate what's being fixed here.


I used the Backport tag to indicate what is the oldest release that it is backportable to.
IIRC the choices were:
* 4.0+ for issues that were present for a long time (didn't look further back than that in history), so there isn't any particular commit that introduces the problem, it was like that from the very beginning, i.e. not a regression.

* 4.13+ for issues affecting only new CPU support (I think that is the release that added Icelake support). I can attempt to find the commit that added Icelake support and mark them as Fixes: that commit (although there might be several of them)

* 4.15+ for bugs introduced by the default read-write msr changes


> Otherwise this is merely a new
> feature.
> 

Prior to the default rdwrmsr changes it was possible to read any MSR, so I consider it a bug that after the default rdwrmsr changes you can no longer do that, it takes away a valuable debugging tool.
I can attempt to find a more specific commit to indicate with Fixes:

>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -641,6 +641,9 @@
>> #define MSR_NHL_LBR_SELECT 0x000001c8
>> #define MSR_NHL_LASTBRANCH_TOS 0x000001c9
>> 
>> +/* Nehalem and newer other MSRs */
>> +#define MSR_SMI_COUNT                       0x00000034
> 
> See my comment on the other patch regarding additions here.
> 
>> --- a/xen/arch/x86/msr.c
>> +++ b/xen/arch/x86/msr.c
>> @@ -139,6 +139,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t *val)
>>         *val = msrs->misc_features_enables.raw;
>>         break;
>> 
>> +    case MSR_SMI_COUNT:
>> +        if ( cp->x86_vendor != X86_VENDOR_INTEL )
>> +            goto gp_fault;
>> +        if ( is_hardware_domain(d) && !rdmsr_safe(msr, *val) )
>> +            break;
>> +        return X86EMUL_UNHANDLEABLE;
> 
> Why #GP for non-Intel but UNHANDLEABLE for DomU?

I wanted to match the behaviour of the 'default:' case statement, although looking at the other MSR handling code in this file they just usually gp_fault,
with the exception of the MSR_K8* code that returns UNHANDLEABLE, so if condition here could be simplified (e.g. follow how MSR_AMD_PATCHLEVEL does it).

Best regards,
--Edwin 

> 
> Jan

Re: [RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Jan Beulich]

On 31.10.2023 10:42, Edwin Torok wrote:
>> On 30 Oct 2023, at 16:20, Jan Beulich <jbeulich@suse.com> wrote:
>> On 25.10.2023 21:29, Edwin Török wrote:
>>> Dom0 should always be able to read this MSR: it is useful when
>>> investigating performance issues in production.
>>
>> While I'm not outright opposed, I'm also not convinced. At the very least
>> ...
>>
>>> Although the count is Thread scoped, in practice all cores were observed
>>> to return the same count (perhaps due to implementation details of SMM),
>>> so do not require the cpu to be pinned in order to read it.
>>
>> ... this, even if matching your observations, is going to be properly
>> misleading in case counts end up diverging.
>>
>>> This MSR exists on Intel since Nehalem.
>>>
>>> Backport: 4.15+
>>
>> If this was a backporting candidate, I think a Fixes: tag would need
>> to indicate what's being fixed here.
> 
> 
> I used the Backport tag to indicate what is the oldest release that it is backportable to.
> IIRC the choices were:
> * 4.0+ for issues that were present for a long time (didn't look further back than that in history), so there isn't any particular commit that introduces the problem, it was like that from the very beginning, i.e. not a regression.
> 
> * 4.13+ for issues affecting only new CPU support (I think that is the release that added Icelake support). I can attempt to find the commit that added Icelake support and mark them as Fixes: that commit (although there might be several of them)
> 
> * 4.15+ for bugs introduced by the default read-write msr changes
> 
> 
>> Otherwise this is merely a new
>> feature.
>>
> 
> Prior to the default rdwrmsr changes it was possible to read any MSR, so I consider it a bug that after the default rdwrmsr changes you can no longer do that, it takes away a valuable debugging tool.

As said elsewhere, making MSRs generally inaccessible was a deliberate change.
I don't think any of us x86 maintainers has so far considered that as introducing
a bug. MSRs being accessible as a debugging tool may be worthwhile to have as an
optional feature (see my suggestion elsewhere as to a possible way to approach
this), but I don't think this can be taken as an indication that we should revert
back to "blind" exposure.

Jan

Re: [RFC PATCH 03/22] x86/msr: always allow a pinned Dom0 to read any unknown MSR

[Jan Beulich]

On 31.10.2023 10:31, Edwin Torok wrote:
>> On 30 Oct 2023, at 16:29, Jan Beulich <jbeulich@suse.com> wrote:
>> On 25.10.2023 21:29, Edwin Török wrote:
>>> This can be useful if you realize you have to inspect the value of an
>>> MSR in production, without having to change into a new Xen first that
>>> handles the MSR.
>>
>> Yet on a non-pinned Dom0 you'd still be lost. Since iirc we generally
>> advise against pinning,
> 
> You can temporarily pin while debugging the issue (e.g. pin just 1 CPU from Dom0, and "walk" all your physical CPUs with it if you have to,
> so that you query them all), e.g. with 'xl vcpu-pin'.
> Although that is more invasive than reading a value.
>  
> Or alternatively have another (privileged) interface to read the MSR for a given core without exposing it to any guests, that way you don't affect the running system at all
> (which would be preferable in a production environment), i.e. a Xen equivalent of 'rdmsr'.

The interface we have (XENPF_resource_op) is, despite being privileged,
deliberately (so far at least) not permitting access to arbitrary MSRs.

In our old XenoLinux forward port we had an extension to the msr.ko
module to allow pCPU-based MSR accesses (and I had a private extension
to the rdmsr/wrmsr user space tools making use of that), but even that
would have been subject to restrictions enforced by Xen as to which
MSRs are accessible.

>> I wonder of how much use such a change would be,
>> when it effectively undoes what we deliberately did a while ago.
>>
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -1933,6 +1933,9 @@ static int cf_check svm_msr_read_intercept(
>>>         break;
>>>
>>>     default:
>>> +        if ( is_hwdom_pinned_vcpu(v) && !rdmsr_safe(msr, *msr_content) )
>>> +            break;
>>> +
>>>         if ( d->arch.msr_relaxed && !rdmsr_safe(msr, tmp) )
>>>         {
>>>             *msr_content = 0;
>>
>> If we went as far as undoing some of what was done, I'd then wonder
>> whether instead we should mandate relaxed mode to be enabled on such a
>> Dom0. Then, instead of returning fake 0 here, the actual value could
>> be returned in the specific case of (pinned?) Dom0.
> 
> 
> Can relaxed mode be enabled at runtime?

Not right now, no. But a hypfs control could certainly be added, with
suitable justification.

> I'd be happy with either solution, but it should be something that can be enabled at runtime
> (if you have to reboot Xen then you may lose the bug repro that you want to gather more information on).
> Although changing such a setting in a production environment may still be risky, because the guest will then become very confused that it has previously read some 0s, now there are some real values, and later when you flip the switch off it gets 0s again.

Indeed. If you flipped such a control for any domain at runtime, you'd
better first check that this wouldn't cause any such issues.

Jan

Re: [RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

[Andrew Cooper]

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

On 30/10/2023 4:15 pm, Jan Beulich wrote:
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -13,6 +13,16 @@
>>   * Blocks of related constants should be sorted by MSR index.  The constant
>>   * names should be as concise as possible, and the bit names may have an
>>   * abbreviated name.  Exceptions will be considered on a case-by-case basis.
>> + *
>> + * References:
>> + * - https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
>> + *    Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific registers
>> + *    Chapter 2, "Model-Specific Registers (MSRs)"
> ... at least Intel's URL has changed several times over the years. Volume
> and chapter numbers change even more frequently. Any such is liable to go
> stale at some point.

https://intel.com/sdm

This one has been valid for roughly the lifetime of intel.com, and is
committed to stay so.

>
> Jan
>
>> + * - https://developer.amd.com/resources/developer-guides-manuals/

whereas AMD really have broken this one, and don't seem to be showing
any urgency in unbreaking it...  Right now there is no landing page at
all for manuals.

~Andrew

[-- Attachment #2: Type: text/html, Size: 2267 bytes --]

Re: [RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

[Edwin Torok]

> On 31 Oct 2023, at 11:34, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> On 30/10/2023 4:15 pm, Jan Beulich wrote:
>> 
>>> --- a/xen/arch/x86/include/asm/msr-index.h
>>> +++ b/xen/arch/x86/include/asm/msr-index.h
>>> @@ -13,6 +13,16 @@
>>> * Blocks of related constants should be sorted by MSR index. The constant
>>> * names should be as concise as possible, and the bit names may have an
>>> * abbreviated name. Exceptions will be considered on a case-by-case basis.
>>> + *
>>> + * References:
>>> + * - https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
>>> + * Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific registers
>>> + * Chapter 2, "Model-Specific Registers (MSRs)"
>>> 
>> ... at least Intel's URL has changed several times over the years. Volume
>> and chapter numbers change even more frequently. Any such is liable to go
>> stale at some point.
> 
> https://intel.com/sdm
> 
> This one has been valid for roughly the lifetime of intel.com, and is committed to stay so.

That is useful to know, I'll update the URL.

> 
>> 
>> Jan
>> 
>> 
>>> + * - https://developer.amd.com/resources/developer-guides-manuals/
> 
> whereas AMD really have broken this one, and don't seem to be showing any urgency in unbreaking it...  Right now there is no landing page at all for manuals.
> 


Linux commits appear to reference a certain bugzilla that has the manuals uploaded: https://bugzilla.kernel.org/show_bug.cgi?id=206537
(although they will go stale in another way, e.g. I see no 2023 manuals there, but at least you know which manual a given commit referenced).
Although referencing someone else's bugzilla in the Xen codebase wouldn't be a nice thing to do, so if we do this it'd probably have to be something hosted on Xen infra.

For now I'll probably drop the URL and just keep the name (so at least you'd know what to search for).


Best regards,
--Edwin

> ~Andrew

Re: [RFC PATCH 02/22] x86/msr: implement MSR_SMI_COUNT for Dom0 on Intel

[Edwin Torok]

> On 31 Oct 2023, at 09:57, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 31.10.2023 10:42, Edwin Torok wrote:
>>> On 30 Oct 2023, at 16:20, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 25.10.2023 21:29, Edwin Török wrote:
>>>> Dom0 should always be able to read this MSR: it is useful when
>>>> investigating performance issues in production.
>>> 
>>> While I'm not outright opposed, I'm also not convinced. At the very least
>>> ...
>>> 
>>>> Although the count is Thread scoped, in practice all cores were observed
>>>> to return the same count (perhaps due to implementation details of SMM),
>>>> so do not require the cpu to be pinned in order to read it.
>>> 
>>> ... this, even if matching your observations, is going to be properly
>>> misleading in case counts end up diverging.
>>> 
>>>> This MSR exists on Intel since Nehalem.
>>>> 
>>>> Backport: 4.15+
>>> 
>>> If this was a backporting candidate, I think a Fixes: tag would need
>>> to indicate what's being fixed here.
>> 
>> 
>> I used the Backport tag to indicate what is the oldest release that it is backportable to.
>> IIRC the choices were:
>> * 4.0+ for issues that were present for a long time (didn't look further back than that in history), so there isn't any particular commit that introduces the problem, it was like that from the very beginning, i.e. not a regression.
>> 
>> * 4.13+ for issues affecting only new CPU support (I think that is the release that added Icelake support). I can attempt to find the commit that added Icelake support and mark them as Fixes: that commit (although there might be several of them)
>> 
>> * 4.15+ for bugs introduced by the default read-write msr changes
>> 
>> 
>>> Otherwise this is merely a new
>>> feature.
>>> 
>> 
>> Prior to the default rdwrmsr changes it was possible to read any MSR, so I consider it a bug that after the default rdwrmsr changes you can no longer do that, it takes away a valuable debugging tool.
> 
> As said elsewhere, making MSRs generally inaccessible was a deliberate change.
> I don't think any of us x86 maintainers has so far considered that as introducing
> a bug. MSRs being accessible as a debugging tool may be worthwhile to have as an
> optional feature (see my suggestion elsewhere as to a possible way to approach
> this), but I don't think this can be taken as an indication that we should revert
> back to "blind" exposure.
> 


This particular patch (unlike the other one that is more general for all MSRs) is about one specific MSR, and makes it accessible to Dom0 only in a read-only way.
That is very different from blindly exposing it to all guests.
Anyway let me get something ready for master first, and then distributions that package Xen can decide whether to backport this patch or not, I'll replace Backport: 4.X with Backportable: 4.X (and add a Fixes: line) to indicate that this patch is recommended to be backported (for anyone who uses that version of Xen in production).

[I'm not saying that this patch is complete, as you've indicated there is more work required to make it work correctly wrt to pinning vs non-pinning, and I'll try to require a pinned Dom0 core in my next version]

Best regards,
--Edwin