Re: [PATCH 2/2] x86/mtrr: Refactor PAT initialization code

[Luis R. Rodriguez]

On Fri, Mar 11, 2016 at 11:57:12AM -0700, Toshi Kani wrote:
> On Fri, 2016-03-11 at 10:24 +0100, Borislav Petkov wrote:
> > On Thu, Mar 10, 2016 at 09:45:46PM -0700, Toshi Kani wrote:
> > > MTRR manages PAT initialization as it implements a rendezvous
> > > handler that initializes PAT as part of MTRR initialization.
> > > 
> > > When CPU does not support MTRR, ex. qemu32 virtual CPU, MTRR
> > > simply skips PAT init, which causes PAT left enabled without
> > > initialization.  Also, get_mtrr_state() calls pat_init() on
> > > BSP even if MTRR is disabled by its MSR.  This causes pat_init()
> > > be called on BSP only.
> > 
> > So I don't understand what all this hoopla is all about: why can't you
> > simply call pat_disable() in mtrr_ap_init() and be done with it?
> > 
> > void mtrr_ap_init(void)
> > {
> >         if (!mtrr_enabled()) {
> > 		pat_disable();
> >                 return;
> > 	}
> > 
> > ?
> 
> No, it does not fix it. The problem in this particular case, i.e. MTRR
> disabled by its MSR, is that mtrr_bp_init() calls pat_init() (as PAT
> enabled) and initializes PAT on BSP. After APs are launched, we need the
> MTRR's rendezvous handler to initialize PAT on APs to be consistent with
> BSP. However, MTRR rendezvous handler is no-op since MTRR is disabled.

This seems like a hack on enabling PAT through MTRR code, can we have
a PAT rendezvous handler on its own, or provide a generic rendezvous
handler that lets you deal with whatever interfaces need setup. Then
conflicts can just be negotiated early.

What I'm after is seeing if we can ultimately disable MTRR on kernel
code but still have PAT enabled. I realize you've mentioned BIOS code
may use some MTRR setup code but this is only true for some systems.
I know for a fact Xen cannot use MTRR, it seems qemu32 does not enable
it either. So why not have the ability to skip through its set up ?

I'll also note Xen managed to enable PAT only without enabling MTRR,
this was done through pat_init_cache_modes() -- not sure if this can
be leveraged for qemu32...

  Luis