On Thu, Mar 10, 2016 at 09:45:45PM -0700, Toshi Kani wrote:
> Since 'commit 9cd25aac1f44 ("x86/mm/pat: Emulate PAT when it
> is disabled")', we emulate a PAT table when PAT is disabled.
> This requires pat_init() be called even if PAT is disabled,
> which revealed a long standing issue that PAT is left enabled
> without calling pat_init() at all.
>
> pat_init() is called from MTRR code since it relies on MTRR's
> rendezvous handler to initialize PAT for all APs . However,
> when CPU does not support MTRR, ex. qemu32's virtual CPU, MTRR
> is set disabled and does not call pat_init(). There is no
> interface available for MTRR to disable PAT, either.
>
> Change pat_disable() to a regular function (from an inline func)
> so that MTRR can call it to disable PAT when MTRR is disabled.
> pat_disable() sets PAT disabled, and calls pat_disable_init()
> to emulate the PAT table.
>
> link: https://lkml.org/lkml/2016/3/10/402
> Reported-by: Paul Gortmaker <paul.gortma...@windriver.com>
> Signed-off-by: Toshi Kani <toshi.k...@hpe.com>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: Luis R. Rodriguez <mcg...@suse.com>
> Cc: Juergen Gross <jgr...@suse.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: H. Peter Anvin <h...@zytor.com>
> Cc: Thomas Gleixner <t...@linutronix.de>
> ---
> arch/x86/include/asm/pat.h | 1 +
> arch/x86/mm/pat.c | 84
> +++++++++++++++++++++++++++-----------------
> 2 files changed, 52 insertions(+), 33 deletions(-)
>
> diff --git a/arch/x86/include/asm/pat.h b/arch/x86/include/asm/pat.h
> index ca6c228..016142b 100644
> --- a/arch/x86/include/asm/pat.h
> +++ b/arch/x86/include/asm/pat.h
> @@ -5,6 +5,7 @@
> #include <asm/pgtable_types.h>
>
> bool pat_enabled(void);
> +void pat_disable(const char *reason);
> extern void pat_init(void);
> void pat_init_cache_modes(u64);
>
> diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> index f4ae536..1ff8aa9 100644
> --- a/arch/x86/mm/pat.c
> +++ b/arch/x86/mm/pat.c
> @@ -40,11 +40,19 @@
> static bool boot_cpu_done;
>
> static int __read_mostly __pat_enabled = IS_ENABLED(CONFIG_X86_PAT);
> +static void pat_disable_init(void);
>
> -static inline void pat_disable(const char *reason)
> +void pat_disable(const char *reason)
> {
> + if (boot_cpu_done) {
> + pr_info("x86/PAT: PAT cannot be disabled after initialized\n");
pr_err()
> + return;
> + }
> +
> __pat_enabled = 0;
> pr_info("x86/PAT: %s\n", reason);
> +
> + pat_disable_init();
Why can't you call pat_init() here simply? It checks pat_enabled(). You
can call it pat_setup() or so if it looks confusing to call an init
function in a disable function...
Then you don't have to add yet another static disable_init_done but rely on
boot_cpu_done which gets set in pat_init().
Also, I don't see the static_cpu_has() check I suggested yesterday - we need to
check the feature bits if PAT gets disabled early on some old Intels.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
