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

From: Toshi Kani <toshi.kani@hpe.com>
To: "Luis R. Rodriguez" <mcgrof@kernel.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Borislav Petkov <bp@alien8.de>, X86 ML <x86@kernel.org>,
	Paul Stewart <pstew@chromium.org>, Ingo Molnar <mingo@kernel.org>,
	Olof Johansson <olof@lixom.net>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Juergen Gross <jgross@suse.com>,
	Jan Beulich <JBeulich@suse.com>,
	Stuart Hayes <stuart.w.hayes@gmail.com>,
	Yinghai Lu <yinghai@kernel.org>,
	Prarit Bhargava <prarit@redhat.com>
Subject: Re: [PATCH 2/2] x86/mtrr: Refactor PAT initialization code
Date: Tue, 29 Mar 2016 19:07:40 -0600	[thread overview]
Message-ID: <1459300060.6393.757.camel@hpe.com> (raw)
In-Reply-To: <CAB=NE6W2rfCdMoULrTEQooZDy1Yp540Na+H_GMHmULBhyzOH5g@mail.gmail.com>

On Tue, 2016-03-29 at 16:43 -0700, Luis R. Rodriguez wrote:
> On Tue, Mar 29, 2016 at 5:16 PM, Toshi Kani <toshi.kani@hpe.com> wrote:
> > On Tue, 2016-03-29 at 15:12 -0700, Luis R. Rodriguez wrote:
> > > On Tue, Mar 29, 2016 at 2:46 PM, Toshi Kani <toshi.kani@hpe.com>
> > > wrote:
> > > > On Tue, 2016-03-29 at 10:14 -0700, Luis R. Rodriguez wrote:
> > > > > On Fri, Mar 18, 2016 at 2:35 PM, Toshi Kani <toshi.kani@hpe.com>
> > > > > wrote:
> >  :
> > > > > 
> > > > > Do we really need UC for the fan?
> > > > 
> > > > When you say "we", are you referring Xen guests?  Xen guests do not
> > > > need to control the fan, so they do not need UC set in MTRRs.
> > > > 
> > > > In general, yes, MMIO registers need UC when they need to be
> > > > accessed.
> > > 
> > > Curious, what does a BIOS do for fan control when MTRRs are disabled?
> > 
> > You mean, when the kernel modified the MTRR setup and disabled them.
> 
> Nope, but the below is good to know!
> 
> I meant to ask about the case where the option the lets a user go in a
> muck with BIOS settings to disable MTRR e xists and the user disables
> MTRR. What would happen for fan control in such situations? I'd
> imagine such cases allow for a system to exist with proper fan
> control, and allow the kernel to boot without having to deal with the
> pesky MTRRs at all, while PAT lives on, no?

You mean user disables MTRRs from BIOS setup menu?  I am not a BIOS guy,
but I do not think it offers such option when the code depends on it...

> > BIOS would assume the original setup and still access the
> > registers.  This may lead to undefined behavior and may result in a
> > system crash.
> > 
> > > Also what if a BIOS just set MSR_MTRRdefType to uncachable only ?
> > 
> > Many BIOSes actually set the default type to UC.
> 
> Thanks, I asked as I saw my BIOS uses write-back by default. Good to
> know there are different strategies.
> 
> > MTRRs then cover regular memory with WB.
> 
> When you say regular memory you mean everything else we see as RAM? I
> was under the impression we'd only need MTRR for a special range of
> memory, and its up to implementation how they are used. If you can use
> MTRR to change the cache attribute for regular RAM and if this is
> actually a requirement if the default MTRR is UC then one way or
> another a BIOS seems to always require MTRR, either for UC setting for
> fan control or WB for regular RAM, is that right?

Right, in one way or the other, MTRRs set WB to RAM and UC to MMIO.  PAT is
overwritten by MTRRs, so RAM must be set to WB.

Thanks,
-Toshi