Message-ID: <1457722632.6393.130.camel@hpe.com>
Subject: Re: [PATCH 2/2] x86/mtrr: Refactor PAT initialization code
From: Toshi Kani <toshi.kani@hpe.com>
To: Borislav Petkov <bp@alien8.de>
Cc: mingo@kernel.org, hpa@zytor.com, tglx@linutronix.de, mcgrof@suse.com,
        jgross@suse.com, paul.gortmaker@windriver.com, x86@kernel.org,
        linux-kernel@vger.kernel.org
Date: Fri, 11 Mar 2016 11:57:12 -0700
In-Reply-To: <20160311092400.GB4347@pd.tnic>
References: <1457671546-13486-1-git-send-email-toshi.kani@hpe.com>
	 <1457671546-13486-3-git-send-email-toshi.kani@hpe.com>
	 <20160311092400.GB4347@pd.tnic>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1292
Lines: 33

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