On Wed, May 03, 2023, Huang, Kai wrote: > On Wed, 2023-05-03 at 11:28 -0700, Sean Christopherson wrote: > > Use the MTRR macros to identify the ranges of possible MTRR MSRs instead > > of bounding the ranges with a mismash of open coded values and unrelated > > MSR indices. Carving out the gap for the machine check MSRs in particular > > is confusing, as it's easy to incorrectly think the case statement handles > > MCE MSRs instead of skipping them. > > > > Drop the range-based funneling of MSRs between the end of the MCE MSRs > > and MTRR_DEF_TYPE, i.e. 0x2A0-0x2FF, and instead handle MTTR_DEF_TYPE as > > the one-off case that it is. > > > > Extract PAT (0x277) as well in anticipation of dropping PAT "handling" > > from the MTRR code. > > > > Keep the range-based handling for the variable+fixed MTRRs even though > > capturing unknown MSRs 0x214-0x24F is arguably "wrong". There is a gap in > > the fixed MTRRs, 0x260-0x267, i.e. the MTRR code needs to filter out > > unknown MSRs anyways,� > > > > Looks a little bit half measure, but ... Yeah, I don't love it, but I couldn't convince myself that precisely identifying known MTRRs was worth the extra effort. > > and using a single range generates marginally better > > code for the big switch statement. > > could you educate why because I am ignorant of compiler behaviour? :) Capturing the entire range instead of filtering out the gaps allows the compiler to handle multiple MSRs with fewer CMP+Jcc checks. E.g. think of it like this (I actually missed a gap) if (msr >= 0x200 && msr <= 0x26f) versus if ((msr >= 0x200 && msr <= 0x213) || (msr == 0x250) || (msr == 0x258) || (msr == 0x259) || (msr >= 0x268 && msr <= 0x26f)) Nothing enormous, and it's not like non-fastpath WRMSR is performance critical, but add in the extra code for precisely capturing the MTRRs in both x86.c _and_ mtrr.c, and IMO it's worth being imprecise.
