| Message ID | c94bc336-fdee-43af-540e-06e0904d8db7@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perfc: assorted adjustments | expand |
On 03/12/2021 12:06, Jan Beulich wrote: > This has gone out of sync over time, resulting in NPF and XSETBV exits > incrementing the same counter. Introduce a simplistic mechanism to > hopefully keep things in better sync going forward. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > Given their large (and growing) number, I wonder whether we shouldn't > fold "SVMexits" and "vmexits". They can't both be active at the same > time. Oh yeah - that's just silly having them split like this, especially as there's no associated element name. > --- a/xen/include/asm-x86/perfc_defn.h > +++ b/xen/include/asm-x86/perfc_defn.h > @@ -11,8 +11,8 @@ PERFCOUNTER_ARRAY(exceptions, > PERFCOUNTER_ARRAY(vmexits, "vmexits", VMX_PERF_EXIT_REASON_SIZE) > PERFCOUNTER_ARRAY(cause_vector, "cause vector", VMX_PERF_VECTOR_SIZE) > > -#define VMEXIT_NPF_PERFC 141 > -#define SVM_PERF_EXIT_REASON_SIZE (1+141) > +#define VMEXIT_NPF_PERFC 143 > +#define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1) How does this work in the first place? perfc_incra() is still passed 1024. Furthermore, it's already worse than this. 401/402 are AVIC exits, and 403 is an SEV exit. We've also gained -2 as "busy" for transient SEV events too. ~Andrew
On 17.12.2021 16:02, Andrew Cooper wrote: > On 03/12/2021 12:06, Jan Beulich wrote: >> This has gone out of sync over time, resulting in NPF and XSETBV exits >> incrementing the same counter. Introduce a simplistic mechanism to >> hopefully keep things in better sync going forward. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> Given their large (and growing) number, I wonder whether we shouldn't >> fold "SVMexits" and "vmexits". They can't both be active at the same >> time. > > Oh yeah - that's just silly having them split like this, especially as > there's no associated element name. Okay, will do. Albeit I was thinking to add naming to xenperf ... >> --- a/xen/include/asm-x86/perfc_defn.h >> +++ b/xen/include/asm-x86/perfc_defn.h >> @@ -11,8 +11,8 @@ PERFCOUNTER_ARRAY(exceptions, >> PERFCOUNTER_ARRAY(vmexits, "vmexits", VMX_PERF_EXIT_REASON_SIZE) >> PERFCOUNTER_ARRAY(cause_vector, "cause vector", VMX_PERF_VECTOR_SIZE) >> >> -#define VMEXIT_NPF_PERFC 141 >> -#define SVM_PERF_EXIT_REASON_SIZE (1+141) >> +#define VMEXIT_NPF_PERFC 143 >> +#define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1) > > How does this work in the first place? perfc_incra() is still passed 1024. In case VMEXIT_NPF: perfc_incra(svmexits, VMEXIT_NPF_PERFC); I don't see any use of 1024. And the earlier blanket perfc_incra(svmexits, exit_reason); doesn't do anything afaict, due to how perfc_incra() works. > Furthermore, it's already worse than this. > > 401/402 are AVIC exits, and 403 is an SEV exit. In which way is this "worse"? We don't support either AVIC or SEV (which is a shame in particular for the former, but what do you do with vendors having given up all engagement). > We've also gained -2 as "busy" for transient SEV events too. I'm sorry, I'm not enough up to speed with SEV yet to even vaguely know what you're referring to here. Jan
--- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -2644,6 +2644,10 @@ void svm_vmexit_handler(struct cpu_user_ goto out; } + /* Note: "+2" to account for VMEXIT_NPF_PERFC. */ +#ifdef SVM_PERF_EXIT_REASON_SIZE + BUILD_BUG_ON(SVM_PERF_EXIT_REASON_SIZE != VMEXIT_LAST + 2); +#endif perfc_incra(svmexits, exit_reason); hvm_maybe_deassert_evtchn_irq(); --- a/xen/include/asm-x86/hvm/svm/vmcb.h +++ b/xen/include/asm-x86/hvm/svm/vmcb.h @@ -302,6 +302,7 @@ enum VMEXIT_EXITCODE VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */ VMEXIT_XSETBV = 141, /* 0x8d */ VMEXIT_RDPRU = 142, /* 0x8e */ +#define VMEXIT_LAST VMEXIT_RDPRU VMEXIT_NPF = 1024, /* 0x400, nested paging fault */ VMEXIT_INVALID = -1 }; --- a/xen/include/asm-x86/perfc_defn.h +++ b/xen/include/asm-x86/perfc_defn.h @@ -11,8 +11,8 @@ PERFCOUNTER_ARRAY(exceptions, PERFCOUNTER_ARRAY(vmexits, "vmexits", VMX_PERF_EXIT_REASON_SIZE) PERFCOUNTER_ARRAY(cause_vector, "cause vector", VMX_PERF_VECTOR_SIZE) -#define VMEXIT_NPF_PERFC 141 -#define SVM_PERF_EXIT_REASON_SIZE (1+141) +#define VMEXIT_NPF_PERFC 143 +#define SVM_PERF_EXIT_REASON_SIZE (VMEXIT_NPF_PERFC + 1) PERFCOUNTER_ARRAY(svmexits, "SVMexits", SVM_PERF_EXIT_REASON_SIZE) #endif /* CONFIG_HVM */
This has gone out of sync over time, resulting in NPF and XSETBV exits incrementing the same counter. Introduce a simplistic mechanism to hopefully keep things in better sync going forward. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- Given their large (and growing) number, I wonder whether we shouldn't fold "SVMexits" and "vmexits". They can't both be active at the same time. I wasn't sure about the #ifdef: Using CONFIG_PERF_COUNTERS there would seem slightly odd next to a construct which specifically abstracts away this aspect.