Re: [PATCH 3/5] KVM: x86: Use MTRR macros to define possible MTRR MSR ranges

[Sean Christopherson <seanjc@google.com>]

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.