All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bryan O'Donoghue" <pure.logic@nexus-software.ie>
To: Ingo Molnar <mingo@kernel.org>
Cc: hpa@zytor.com, mingo@redhat.com, tglx@linutronix.de,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86: Quark: Flush TLB via CR3 not CR4.PGE in setup_arch()
Date: Thu, 25 Sep 2014 10:35:51 +0100	[thread overview]
Message-ID: <5423E1F7.5010000@nexus-software.ie> (raw)
In-Reply-To: <20140925045755.GA20431@gmail.com>

>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
>> index 41ead8d..1d2396a 100644
>> --- a/arch/x86/kernel/setup.c
>> +++ b/arch/x86/kernel/setup.c
>> @@ -879,7 +879,10 @@ void __init setup_arch(char **cmdline_p)
>>   			KERNEL_PGD_PTRS);
>>
>>   	load_cr3(swapper_pg_dir);
>> -	__flush_tlb_all();
>> +	if (boot_cpu_data.x86 == 5 && boot_cpu_data.x86_model == 9)
>> +		__flush_tlb();
>> +	else
>> +		__flush_tlb_all();
>
> So why not make __flush_tlb_all() Quark-quirk-aware and be done
> with it, instead of having to validate every single
> __flush_tlb_all() user?
>
> Quark breaks the x86 'flush all TLBs' semantics - the way to fix
> it is to restore those semantics, not to sprinkle the breakage
> all around the code ...

Hi Ingo.

We have made __flush_tlb_all() Quark aware - because the previous patch 
we applied to arch/x86/kernel/cpu/intel.c

ee1b5b165c0a2f04d2107e634e51f05d0eb107de

+	if (c->x86 == 5 && c->x86_model == 9) {
+		pr_info("Disabling PGE capability bit\n");
+		setup_clear_cpu_cap(X86_FEATURE_PGE);
+	}

will cause cpu_has_pge() to be false and then the flush_tlb_all code 
will take the path we want __flush_tlb not __flush_tlb_global

static inline void __flush_tlb_all(void)
{
         if (cpu_has_pge)
                 __flush_tlb_global();
         else
                 __flush_tlb();
}

The code in setup_arch runs before the cpu_has_pge bit been clobbered.

The commit you did 02276a3a677d681f0cd227d7111c71fdbce23832 just adds a 
comment to setup_arch to indicate the behaviour we are relying on

setup_arch()
{

load_cr3(swapper_pg_dir);	/* this will flush the TLB on Quark */
__flush_tlb_all();		/*cpu_has_pge() is true at this point*/

......

/* this is where we latch the cpu cabability bits */
early_cpu_init();
}

--
BOD

  reply	other threads:[~2014-09-25  9:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 17:07 [PATCH] x86: Quark: Flush TLB via CR3 not CR4.PGE in setup_arch() Bryan O'Donoghue
2014-09-25  4:57 ` Ingo Molnar
2014-09-25  9:35   ` Bryan O'Donoghue [this message]
2014-09-25 14:51     ` Ingo Molnar
2014-09-25 15:04       ` Bryan O'Donoghue
2014-09-25 15:11         ` Ingo Molnar
2014-09-25 16:49           ` Henrique de Moraes Holschuh
2014-09-25 18:28             ` Ingo Molnar
2014-09-25 18:50               ` Bryan O'Donoghue
2014-09-25 18:59                 ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5423E1F7.5010000@nexus-software.ie \
    --to=pure.logic@nexus-software.ie \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.