Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932367AbcCOVb7 (ORCPT ); Tue, 15 Mar 2016 17:31:59 -0400 Received: from mx2.suse.de ([195.135.220.15]:50327 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750701AbcCOVb4 (ORCPT ); Tue, 15 Mar 2016 17:31:56 -0400 Date: Tue, 15 Mar 2016 22:31:53 +0100 From: "Luis R. Rodriguez" To: Toshi Kani Cc: "Luis R. Rodriguez" , Borislav Petkov , "mingo@kernel.org" , "hpa@zytor.com" , "tglx@linutronix.de" , "jgross@suse.com" , "paul.gortmaker@windriver.com" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , xen-devel@lists.xenproject.org, boris.ostrovsky@oracle.com Subject: Re: [PATCH 1/2] x86/mm/pat: Change pat_disable() to emulate PAT table Message-ID: <20160315213153.GB1990@wotan.suse.de> References: <1457671546-13486-1-git-send-email-toshi.kani@hpe.com> <1457671546-13486-2-git-send-email-toshi.kani@hpe.com> <20160311091229.GA4347@pd.tnic> <1457713660.6393.55.camel@hpe.com> <20160311155439.GF4312@pd.tnic> <1457724504.6393.151.camel@hpe.com> <20160312115544.GA23410@pd.tnic> <20160315002921.GG25147@wotan.suse.de> <1458011476.6393.327.camel@hpe.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1458011476.6393.327.camel@hpe.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5216 Lines: 162 On Mon, Mar 14, 2016 at 09:11:16PM -0600, Toshi Kani wrote: > On Tue, 2016-03-15 at 01:29 +0100, Luis R. Rodriguez wrote: > > I like this approach more as it stuff more PAT setup on its own type > > of calls, but: > > > > On Sat, Mar 12, 2016 at 12:55:44PM +0100, Borislav Petkov wrote: > > > diff --git a/arch/x86/kernel/cpu/mtrr/main.c > > > b/arch/x86/kernel/cpu/mtrr/main.c > > > index 10f8d4796240..5c442b4bd52a 100644 > > > --- a/arch/x86/kernel/cpu/mtrr/main.c > > > +++ b/arch/x86/kernel/cpu/mtrr/main.c > > > @@ -759,8 +761,11 @@ void __init mtrr_bp_init(void) > > > ? } > > > ? } > > > ? > > > - if (!mtrr_enabled()) > > > + if (!__mtrr_enabled) { > > > ? pr_info("MTRR: Disabled\n"); > > > + pat_disable("PAT disabled by MTRR"); > > > + pat_setup(); > > > + } > > > ?} > > > > This hunk would break PAT on Xen. > > Can you try the attached patches? ?They apply on top of my original patch- > set. ?With this change, PAT code generally supports Xen, and the PAT init > code in Xen is now removed. ?If they look OK, I will reorganize the patch > series. I don't have time to test this at this time but on a cursory review this should in theory work, a few nitpicks: > > Thanks, > -Toshi > From: Toshi Kani > > Add support of PAT emulation that matches with the PAT MSR. > > --- > arch/x86/mm/pat.c | 73 +++++++++++++++++++++++++++++++---------------------- > 1 file changed, 43 insertions(+), 30 deletions(-) > > diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c > index 1ff8aa9..565a478 100644 > --- a/arch/x86/mm/pat.c > +++ b/arch/x86/mm/pat.c > @@ -40,7 +40,7 @@ > static bool boot_cpu_done; > > static int __read_mostly __pat_enabled = IS_ENABLED(CONFIG_X86_PAT); > -static void pat_disable_init(void); > +static void pat_emu_init(void); > > void pat_disable(const char *reason) > { > @@ -52,7 +52,7 @@ void pat_disable(const char *reason) > __pat_enabled = 0; > pr_info("x86/PAT: %s\n", reason); > > - pat_disable_init(); > + pat_emu_init(); > } > > static int __init nopat(char *str) > @@ -239,40 +239,53 @@ static void pat_ap_init(u64 pat) > wrmsrl(MSR_IA32_CR_PAT, pat); > } > > -static void pat_disable_init(void) > +static void pat_emu_init(void) > { > - u64 pat; > - static int disable_init_done; > + u64 pat = 0; > + static int emu_init_done; > > - if (disable_init_done) > + if (emu_init_done) > return; > > - /* > - * No PAT. Emulate the PAT table that corresponds to the two > - * cache bits, PWT (Write Through) and PCD (Cache Disable). This > - * setup is the same as the BIOS default setup when the system > - * has PAT but the "nopat" boot option has been specified. This > - * emulated PAT table is used when MSR_IA32_CR_PAT returns 0. > - * > - * PTE encoding: > - * > - * PCD > - * |PWT PAT > - * || slot > - * 00 0 WB : _PAGE_CACHE_MODE_WB > - * 01 1 WT : _PAGE_CACHE_MODE_WT > - * 10 2 UC-: _PAGE_CACHE_MODE_UC_MINUS > - * 11 3 UC : _PAGE_CACHE_MODE_UC > - * > - * NOTE: When WC or WP is used, it is redirected to UC- per > - * the default setup in __cachemode2pte_tbl[]. > - */ > - pat = PAT(0, WB) | PAT(1, WT) | PAT(2, UC_MINUS) | PAT(3, UC) | > - PAT(4, WB) | PAT(5, WT) | PAT(6, UC_MINUS) | PAT(7, UC); > + if (cpu_has_pat) { First, this is not emulation, to be clear. > + /* > + * CPU supports PAT. Initialize the PAT table to match with > + * the PAT MSR value. This setup is used by "nopat" boot Did you mean "nomtrr" option ? If not why would "nopat" land you here and if "nopat" was used and you ended up here why would we want to go ahead and read the MSR to keep the PAT set up ? > + * option, or by virtual machine environments which do not > + * support MTRRs but support PAT. This might end up supporting PAT for some other virtual environments ;) > + * > + * If the MSR returns 0, it is considered invalid and emulate > + * as No PAT. > + */ > + rdmsrl(MSR_IA32_CR_PAT, pat); > + } > + > + if (!pat) { > + /* > + * No PAT. Emulate the PAT table that corresponds to the two > + * cache bits, PWT (Write Through) and PCD (Cache Disable). > + * This setup is also the same as the BIOS default setup. > + * > + * PTE encoding: > + * > + * PCD > + * |PWT PAT > + * || slot > + * 00 0 WB : _PAGE_CACHE_MODE_WB > + * 01 1 WT : _PAGE_CACHE_MODE_WT > + * 10 2 UC-: _PAGE_CACHE_MODE_UC_MINUS > + * 11 3 UC : _PAGE_CACHE_MODE_UC > + * > + * NOTE: When WC or WP is used, it is redirected to UC- per > + * the default setup in __cachemode2pte_tbl[]. > + */ > + pat = PAT(0, WB) | PAT(1, WT) | PAT(2, UC_MINUS) | PAT(3, UC) | > + PAT(4, WB) | PAT(5, WT) | PAT(6, UC_MINUS) | PAT(7, UC); > + } a) pat_emu_init() -- this should probably be renamed to something that includes not just emulation as for virtual environments that's not true and using emulation is very misleading for Xen. b) Will this mean that then you can support bare metal with no MTRR but with PAT enabled too? c) Why not just split this up to enable PAT init to be a first class citizen ? Luis