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

[Toshi Kani]

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.
Hence, we cannot let mtrr_bp_init() to call pat_init() when MTRR is
disabled.

Thanks,
-Toshi