[PATCH v2 0/2] Fix legacy_cache detect on Intel. Add Quark to legacy_cache, document TLB quirk

[PATCH v2 0/2] Fix legacy_cache detect on Intel. Add Quark to legacy_cache, document TLB quirk

[Bryan O'Donoghue]

These two patches cover four things.

First add legacy_cache callback into init_intel() to enable calling of
intel_size_cache() re-enabling detection of PIII Tualatin cache size
and allowing Quark SoC X1000 hooked into this mechanism to similarly
report it's cache size.

Second adding of Quark SoC X1000 to the legacy_cache callback so that
it too will report correct cache size. In this case a 4-way set
associative cache with a 16 byte cache line and 256 lines per tag. 

Third add the Quark SoC X1000 string so that /proc/cpuinfo gives the
correct description of the processor in the "model name" field.

Finally documentation of early TLB flushing behaviour on Quark before
cpu_has_pge() has been switched off. For clarity the existing code works
fine on Quark. This change to setup.c documents the minutiae of why.

Bryan O'Donoghue (2):
  x86: Add cpu_detect_cache_sizes() to init_intel(), add Quark to
    legacy_cache
  x86: Quark: Comment setup_arch() to document TLB/PGE behaviour

 arch/x86/kernel/cpu/intel.c | 23 +++++++++++++++++++++--
 arch/x86/kernel/setup.c     |  9 +++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v2 1/2] x86: Add cpu_detect_cache_sizes() to init_intel(), add Quark to legacy_cache()

[Bryan O'Donoghue]

Intel processors which don't report cache information via cpuid(2)
or cpuid(4) need quirk code in the legacy_cache_size callback to
report this data. For Intel that callback is is intel_size_cache().

This patch enables calling of cpu_detect_cache_sizes() inside of
init_intel() and hence the calling of the legacy_cache callback in
intel_size_cache(). Adding this call will ensure that PIII Tualatin
currently in intel_size_cache() and Quark SoC X1000 being added to
intel_size_cache() in this patch will report their respective cache
sizes.

This model of calling cpu_detect_cache_sizes() is consistent with
AMD/Via/Cirix/Transmeta and Centaur.

Also added is a string to idenitfy the Quark as Quark SoC X1000
giving better and more descriptive output via /proc/cpuinfo

Adding cpu_detect_cache_sizes to init_intel() will enable calling
of intel_size_cache() on Intel processors which currently no code
can reach. Therefore this patch will also re-enable reporting
of PIII Tualatin cache size information as well as add
Quark SoC X1000 support.

Comment text suggested by Thomas Gleixner

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 arch/x86/kernel/cpu/intel.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 74e804d..f4248b7 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -381,7 +381,18 @@ static void init_intel(struct cpuinfo_x86 *c)
 #endif
 	}
 
-	l2 = init_intel_cacheinfo(c);
+	/* Detect legacy cache sizes */
+	cpu_detect_cache_sizes(c);
+
+	/*
+	 * If cache_size has not been initialized via legacy_cache()
+	 * probe it via init_intel_cacheinfo().
+	 */
+	if (c->x86_cache_size == 0)
+		l2 = init_intel_cacheinfo(c);
+	else
+		l2 = c->x86_cache_size;
+
 	if (c->cpuid_level > 9) {
 		unsigned eax = cpuid_eax(10);
 		/* Check for version and the number of counters */
@@ -485,6 +496,13 @@ static unsigned int intel_size_cache(struct cpuinfo_x86 *c, unsigned int size)
 	 */
 	if ((c->x86 == 6) && (c->x86_model == 11) && (size == 0))
 		size = 256;
+
+	/*
+	 * Intel Quark SoC X1000 contains a 4-way set associative
+	 * 16K cache with a 16 byte cache line and 256 lines per tag
+	 */
+	if ((c->x86 == 5) && (c->x86_model == 9))
+		size = 16;
 	return size;
 }
 #endif
@@ -686,7 +704,8 @@ static const struct cpu_dev intel_cpu_dev = {
 			  [3] = "OverDrive PODP5V83",
 			  [4] = "Pentium MMX",
 			  [7] = "Mobile Pentium 75 - 200",
-			  [8] = "Mobile Pentium MMX"
+			  [8] = "Mobile Pentium MMX",
+			  [9] = "Quark SoC X1000",
 		  }
 		},
 		{ .family = 6, .model_names =
-- 
1.9.1

Re: [PATCH v2 1/2] x86: Add cpu_detect_cache_sizes() to init_intel(), add Quark to legacy_cache()

[Thomas Gleixner]

On Sat, 4 Oct 2014, Bryan O'Donoghue wrote:
> @@ -381,7 +381,18 @@ static void init_intel(struct cpuinfo_x86 *c)
>  #endif
>  	}
>  
> -	l2 = init_intel_cacheinfo(c);
> +	/* Detect legacy cache sizes */
> +	cpu_detect_cache_sizes(c);
> +
> +	/*
> +	 * If cache_size has not been initialized via legacy_cache()
> +	 * probe it via init_intel_cacheinfo().
> +	 */
> +	if (c->x86_cache_size == 0)
> +		l2 = init_intel_cacheinfo(c);
> +	else
> +		l2 = c->x86_cache_size;

Looking a bit deeper. This wont work because cpu_detect_cache_sizes()
will set c->x86_cache_size for all cpus with extended_cpuid_level >=
0x80000005, which is the case for everything modern. So this results
in init_intel_cacheinfo() not being called anymore.

So we might need the following conditional:

	l2 = init_intel_cacheinfo(c);
+	if (!c->x86_cache_size) {
+		cpu_detect_cache_sizes(c);
+		l2 = c->x86_cache_size;
+	}

If that does not work because c->x86_cache_size is not 0 after
init_intel_cacheinfo() we really need to make it family/model
dependent.

Thanks,

	tglx

Re: [PATCH v2 1/2] x86: Add cpu_detect_cache_sizes() to init_intel(), add Quark to legacy_cache()

[Bryan O'Donoghue]

On 06/10/14 10:55, Thomas Gleixner wrote:
>> +	 */
>> +	if (c->x86_cache_size == 0)
>> +		l2 = init_intel_cacheinfo(c);
>> +	else
>> +		l2 = c->x86_cache_size;
>
> Looking a bit deeper. This wont work because cpu_detect_cache_sizes()
> will set c->x86_cache_size for all cpus with extended_cpuid_level >=
> 0x80000005, which is the case for everything modern. So this results
> in init_intel_cacheinfo() not being called anymore.

True.

Missed that one.

> So we might need the following conditional:
>
> 	l2 = init_intel_cacheinfo(c);
> +	if (!c->x86_cache_size) {
> +		cpu_detect_cache_sizes(c);
> +		l2 = c->x86_cache_size;
> +	}

Yes - that'll work better.

Also - it only forces a conditional branch - so fewer extra cycles for 
everybody !Quark and !Tualatin - which is preferential. Will incorporate.


Best,
Bryan

[PATCH v2 2/2] x86: Quark: Comment setup_arch() to document TLB/PGE behaviour

[Bryan O'Donoghue]

Quark SoC X1000 advertises Page Global Enable for it's
Translation Lookaside Buffer via cpuid. The silicon does not
in fact support PGE and hence will not flush the TLB when CR4.PGE
is rewritten. The Quark documentation makes clear the necessity to
instead rewrite CR3 in order to flush any TLB entries, irrespective
of the state of CR4.PGE or an individual PTE.PGE

See Intel Quark Core DevMan_001.pdf section 6.4.11

In setup.c setup_arch() the code will load_cr3() and then do a
__flush_tlb_all().

On Quark the entire TLB will be flushed at the load_cr3().
The __flush_tlb_all() have no effect and can be safely ignored.

Later on in the boot process we switch off the flag for cpu_has_pge()
which means that subsequent calls to __flush_tlb_all() will
call __flush_tlb() not __flush_tlb_global() flushing the TLB in the
correct way via load_cr3() not CR4.PGE rewrite

This patch documents the behaviour of flushing the TLB for Quark in
setup_arch()

Comment text suggested by Thomas Gleixner

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
---
 arch/x86/kernel/setup.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 41ead8d..235cfd3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -879,6 +879,15 @@ void __init setup_arch(char **cmdline_p)
 			KERNEL_PGD_PTRS);
 
 	load_cr3(swapper_pg_dir);
+	/*
+	 * Note: Quark X1000 CPUs advertise PGE incorrectly and require
+	 * a cr3 based tlb flush, so the following __flush_tlb_all()
+	 * will not flush anything because the cpu quirk which clears
+	 * X86_FEATURE_PGE has not been invoked yet. Though due to the
+	 * load_cr3() above the TLB has been flushed already. The
+	 * quirk is invoked before subsequent calls to __flush_tlb_all()
+	 * so proper operation is guaranteed.
+	 */
 	__flush_tlb_all();
 #else
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
-- 
1.9.1