On 31.03.17 at 16:46, <mohit.gambhir@xxxxxxxxxx> wrote:
This patch masks .AnyThread bits in IA32_FIXED_CTR_CTRL MSR for all
versions of Intel Arhcitectural Performance Monitoring. Note that
.AnyThread bit (21) is already masked in IA32_PERFEVTSELx MSRs since
hyperthreading is not exposed to guests and Intel SDM discourages the use of
.AnyThread bit in virtualized environments (per section 18.2.3.1
AnyThread Counting and Software Evolution)
All nice and presumably correct, but the main thing is missing: The
bits aren't defined prior to version 3 afaics, so ...
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -979,8 +979,7 @@ int __init core2_vpmu_init(void)
full_width_write = (caps >> 13) & 1;
fixed_ctrl_mask = ~((1ull << (fixed_pmc_cnt * FIXED_CTR_CTRL_BITS)) - 1);
- if ( version == 2 )
- fixed_ctrl_mask |= 0x444;
+ fixed_ctrl_mask |= 0x444;
... the main thing to explain is why removing the conditional is
(a) correct and (b) necessary (going through the uses of the
variable I can see (a) to be true, but not (b)). And of course it
would be quite helpful if the literal number changed to a
manifest constant at once, or a comment was attached to
clarify what the number represents.