Re: [PATCH v2] x86: Quark: Add if/else to setup_arch for Quark TLB bug

["Bryan O'Donoghue" <pure.logic@nexus-software.ie>]

On 26/09/14 21:59, Henrique de Moraes Holschuh wrote:

> I'm confused, now.
>
> Wasn't the other patch -- which just added a comment -- the one selected as
> a better fix, because there is absolutely no point in calling __flush_tlb()
> on Quark X1000 *right after* you just flushed the TLB [on these processors]
> by doing a load_cr3() ?
>
> Should this one ([PATCH v2] x86: Quark: Add if/else to setup_arch for Quark
> TLB bug) be ignored?  Or should the other one which just adds a comment be
> ignored?

Hi Henrique.

My view is that the CR3 load should have flushed the TLB in it's entirety.

Ong Boong Leong said that a discussion he had which included HPA 
concluded with a flush of the TLB being required after the CR3 reload.

The current code

a) Writes to CR3.
On Quark that will flush the entire TLB
Ref: Quark SoC X1000 Core Developer's Manual section 6.4.11

On other processors this won't invalidate the entire TLB under every 
scenario. For example a PTE with PGE on may not be flushed.

Ref: Intel 64 and IA32 Architectures .. vol (3a, 3b, 3c) 325384.pdf 
section 4.10.4.1

b) __flush_tlb_all
Depending on the value of cpu_has_pge() - which will be true for ~all 
x86 processors will call

native_write_cr4(cr4 & ~X86_CR4_PGE);

According to the same section "4.10.4.1" this will nuke the rest of the 
TLB entries even with PGE bits set in the PTEs

So the steps a & b are the appropriate sets to take to entirely flush 
the TLB for most processors i.e. one's that support PGE

The suggestion is that on Quark we should flush the TLB again.

The documentation doesn't really support that statement but, my view is 
OK if we are going to accommodate that position let's not force a third 
TLB flush on everybody who isn't running a Quark.

 From the technical/documentation standpoint I don't think Quark is 
forcing a step c - if we do decide to add a __flush_tlb() call for the 
sake of Quark anyway - then let's not foist the extra cycles on every 
other x86 processor out there.

My preference is

1. Just comment the code as is to explain why it works for Quark.

If that's not good enough for people then

2. if/else the flow so that Quark does __flush_tlb() and the rest of the 
world does a __flush_tlb_all()


--
BOD