| Message ID | 20220105050711.67280-1-likexu@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] KVM: x86/pmu: Make top-down.slots event unavailable in supported leaf | expand |
On Wed, Jan 05, 2022, Like Xu wrote: > From: Like Xu <likexu@tencent.com> > > When we choose to disable the fourth fixed counter TOPDOWN.SLOTS, > we need to also reduce the length of the 0AH.EBX bit vector, which > enumerates architecture performance monitoring events, and set > 0AH.EBX.[bit 7] to 1 if the new value of EAX[31:24] is still > 7. > > Fixes: 2e8cd7a3b8287 ("kvm: x86: limit the maximum number of vPMU fixed counters to 3") > Signed-off-by: Like Xu <likexu@tencent.com> > --- > arch/x86/kvm/cpuid.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0b920e12bb6d..1f0131145296 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -782,6 +782,21 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > eax.split.mask_length = cap.events_mask_len; > > edx.split.num_counters_fixed = min(cap.num_counters_fixed, MAX_FIXED_COUNTERS); > + > + /* > + * The 8th Intel pre-defined architectural event (Topdown Slots) will be supported > + * if the 4th fixed counter exists && EAX[31:24] > 7 && EBX[7] = 0. Please wrap at ~80 chars. > + * > + * Currently, KVM needs to set EAX[31:24] < 8 or EBX[7] == 1 > + * to make this event unavailable in a consistent way. > + */ > + if (edx.split.num_counters_fixed < 4) { > + if (eax.split.mask_length > 7) > + eax.split.mask_length--; This will break if there's a bit>7 enumerated in EBX (events_mask) that KVM wants to expose to the guest. It doesn't cause problems today because bits 31:8 are all reserved, but that will not always be the case. We could do if (edx.split.num_counters_fixed < 4) { if (eax.split.mask_length == 7) eax.split.mask_length--; else cap.events_mask |= BIT_ULL(7); } but I don't see any reason to make this more complex than: if (edx.split.num_counters_fixed < 4 && eax.split.mask_length > 7) cap.events_mask |= BIT_ULL(7); > + if (eax.split.mask_length > 7) > + cap.events_mask |= BIT_ULL(7); > + } > + > edx.split.bit_width_fixed = cap.bit_width_fixed; > if (cap.version) > edx.split.anythread_deprecated = 1; > -- > 2.33.1 >
On 1/5/22 06:07, Like Xu wrote: > + /* > + * The 8th Intel pre-defined architectural event (Topdown Slots) will be supported > + * if the 4th fixed counter exists && EAX[31:24] > 7 && EBX[7] = 0. > + * > + * Currently, KVM needs to set EAX[31:24] < 8 or EBX[7] == 1 > + * to make this event unavailable in a consistent way. > + */ > + if (edx.split.num_counters_fixed < 4) { > + if (eax.split.mask_length > 7) > + eax.split.mask_length--; > + if (eax.split.mask_length > 7) > + cap.events_mask |= BIT_ULL(7); > + } > + The first "> 7" is wrong; it should be == 8, shouldn't it? Something like if (edx.split.num_counters_fixed < 4 && eax.split.mask_length >= 8) { if (eax.split.mask_length == 8) eax.split.mask_length--; else cap.events_mask |= BIT_ULL(7); } is what you mean, I think? Paolo
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 0b920e12bb6d..1f0131145296 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -782,6 +782,21 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) eax.split.mask_length = cap.events_mask_len; edx.split.num_counters_fixed = min(cap.num_counters_fixed, MAX_FIXED_COUNTERS); + + /* + * The 8th Intel pre-defined architectural event (Topdown Slots) will be supported + * if the 4th fixed counter exists && EAX[31:24] > 7 && EBX[7] = 0. + * + * Currently, KVM needs to set EAX[31:24] < 8 or EBX[7] == 1 + * to make this event unavailable in a consistent way. + */ + if (edx.split.num_counters_fixed < 4) { + if (eax.split.mask_length > 7) + eax.split.mask_length--; + if (eax.split.mask_length > 7) + cap.events_mask |= BIT_ULL(7); + } + edx.split.bit_width_fixed = cap.bit_width_fixed; if (cap.version) edx.split.anythread_deprecated = 1;