[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/5] VMX: sync VM-exit perf counters with known VM-exit reasons
On 17.12.2021 16:00, Andrew Cooper wrote: > On 03/12/2021 12:05, Jan Beulich wrote: >> --- a/xen/include/asm-x86/hvm/vmx/vmx.h >> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h >> @@ -220,6 +220,8 @@ static inline void pi_clear_sn(struct pi >> #define EXIT_REASON_XSAVES 63 >> #define EXIT_REASON_XRSTORS 64 >> >> +#define EXIT_REASON_LAST EXIT_REASON_XRSTORS >> + > > Given the problems with sentinals like this in the domctl/sysctl > headers, I think this scheme would be less fragile if EXIT_REASON was an > enum. Enums have some "downsides", so I'm not sure I'd want to go that route, at least not right here. > (Also, we seriously need to reduce the scope of these headers. > It's only just dawned on me why Intel uses EXIT_* and AMD uses VMEXIT_*) Funny, isn't it? Otoh the PM specifically uses VMEXIT_*, while the SDM specifically talks about "exit reason" everywhere. So there may be more to this than just the need to avoid name space collisions. And yes, I fully agree with the need of scope reduction. These definitions living in a private header in xen/arch/x86/hvm/vmx/ should be completely sufficient for the build to continue to work. Question is if, while doing so, we wouldn't want to alter the name prefixes (but see above). > Alternatively, what about simply this: > > #define EXIT_REASON_XSAVES 63 > #define EXIT_REASON_XRSTORS 64 > +/* Remember to update VMX_PERF_EXIT_REASON_SIZE too. */ > > ? > > This avoids having yet another sentinel in the mix, and will be visible > in *every* patch review. It also gets rid of the ifdefary in the vmexit > handler. I can do it that way, sure, but then there'll again be no build time check. As long as exit reasons all get added sequentially here, the comment in context should raise enough attention, but what if Intel start a second range like AMD did with NPF? Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |