Re: [Xen-devel] [PATCH] x86/vpmu_intel: Handle SMT consistently for programmable and fixed counters

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel