Centaur CPUs enumerate the cache topology in the same way as Intel CPUs,
but the functionality is unused so far.
The Centaur init code also misses to initialize x86_info::max_cores, so
the CPU topology can't be described correctly.
Initialize x86_cpuinfo:max_core and invoke init_intel_cacheinfo() to
make CPU and cache topology information avaliable and correct.
Signed-off-by: David Wang <[email protected]>
Changes from v1 to v2:
*1 replace centaur_num_cpu_cores with early_init_centaur_mc according to
suggestions from tglx;
*2 call cpu_detect_cache_sizes when init_intel_cacheinfo returns 0. For
some very old Centaur CPUs can't support the mechanisms defined in init_
intel_cacheinfo.
---
arch/x86/kernel/cpu/centaur.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index 80d5110..367540b 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -96,8 +96,25 @@ enum {
EAMD3D = 1<<20,
};
+static void early_init_centaur_mc(struct cpuinfo_x86 *c)
+{
+#ifdef CONFIG_SMP
+ unsigned int eax, ebx, ecx, edx;
+
+ if (c->cpuid_level < 4)
+ return;
+
+ cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
+ if (eax & 0x1f)
+ c->x86_max_cores = (eax >> 26) + 1;
+ else
+ return;
+#endif
+}
+
static void early_init_centaur(struct cpuinfo_x86 *c)
{
+ early_init_centaur_mc(c);
switch (c->x86) {
#ifdef CONFIG_X86_32
case 5:
@@ -146,6 +163,7 @@ static void centaur_detect_vmx_virtcap(struct cpuinfo_x86 *c)
static void init_centaur(struct cpuinfo_x86 *c)
{
+ unsigned int l2 = 0;
#ifdef CONFIG_X86_32
char *name;
u32 fcr_set = 0;
@@ -161,6 +179,17 @@ static void init_centaur(struct cpuinfo_x86 *c)
#endif
early_init_centaur(c);
+ l2 = init_intel_cacheinfo(c);
+
+ /* Detect legacy cache sizes if init_intel_cacheinfo did not */
+ if (l2 == 0) {
+ cpu_detect_cache_sizes(c);
+ }
+
+#ifdef CONFIG_X86_32
+ detect_ht(c);
+#endif
+
if (c->cpuid_level > 9) {
unsigned int eax = cpuid_eax(10);
--
1.9.1
On Wed, 25 Apr 2018, David Wang wrote:
>
> +static void early_init_centaur_mc(struct cpuinfo_x86 *c)
> +{
> +#ifdef CONFIG_SMP
> + unsigned int eax, ebx, ecx, edx;
> +
> + if (c->cpuid_level < 4)
> + return;
> +
> + cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
> + if (eax & 0x1f)
> + c->x86_max_cores = (eax >> 26) + 1;
> + else
> + return;
> +#endif
> +}
My review comment from last time still stands:
> > This is a bad copy of intel_num_cpu_cores(). See for the subtle
> > difference. Please rename the intel function and move it to common.c
In other words:
Make a patch which moves intel_num_cpu_cores() into common.c. Rename the
function into something like detect_num_cpu_cores() and fix up the call
site in intel.c.
Then add your patch and use the very same function.
> +
> static void early_init_centaur(struct cpuinfo_x86 *c)
> {
> + early_init_centaur_mc(c);
> switch (c->x86) {
> #ifdef CONFIG_X86_32
> case 5:
> @@ -146,6 +163,7 @@ static void centaur_detect_vmx_virtcap(struct cpuinfo_x86 *c)
>
> static void init_centaur(struct cpuinfo_x86 *c)
> {
> + unsigned int l2 = 0;
> #ifdef CONFIG_X86_32
> char *name;
> u32 fcr_set = 0;
> @@ -161,6 +179,17 @@ static void init_centaur(struct cpuinfo_x86 *c)
> #endif
> early_init_centaur(c);
>
> + l2 = init_intel_cacheinfo(c);
> +
> + /* Detect legacy cache sizes if init_intel_cacheinfo did not */
> + if (l2 == 0) {
> + cpu_detect_cache_sizes(c);
> + }
Aside of the pointless parentheses, this really wants to be cleaned up as
well.
init_intel_cacheinfo() is called from the intel init code and it does the
same silly thing.
So the right thing to do is in a separate patch first
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -679,12 +679,6 @@ static void init_intel(struct cpuinfo_x8
l2 = init_intel_cacheinfo(c);
- /* Detect legacy cache sizes if init_intel_cacheinfo did not */
- if (l2 == 0) {
- cpu_detect_cache_sizes(c);
- l2 = c->x86_cache_size;
- }
-
if (c->cpuid_level > 9) {
unsigned eax = cpuid_eax(10);
/* Check for version and the number of counters */
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -802,6 +802,11 @@ unsigned int init_intel_cacheinfo(struct
c->x86_cache_size = l3 ? l3 : (l2 ? l2 : (l1i+l1d));
+ /* Detect legacy cache sizes if the above did not work */
+ if (!l2) {
+ cpu_detect_cache_sizes(c);
+ l2 = c->x86_cache_size;
+ }
return l2;
}
Hmm?
tglx
> -----Original Mail-----
> Sender: Thomas Gleixner [mailto:[email protected]]
> Time: 2018??4??26?? 17:12
> Receiver: David Wang <[email protected]>
> CC: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; brucechang@via-
> alliance.com; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v2] x86/centaur: report correct CPU/cache topology
>
> On Wed, 25 Apr 2018, David Wang wrote:
> >
> > +static void early_init_centaur_mc(struct cpuinfo_x86 *c) { #ifdef
> > +CONFIG_SMP
> > + unsigned int eax, ebx, ecx, edx;
> > +
> > + if (c->cpuid_level < 4)
> > + return;
> > +
> > + cpuid_count(4, 0, &eax, &ebx, &ecx, &edx);
> > + if (eax & 0x1f)
> > + c->x86_max_cores = (eax >> 26) + 1;
> > + else
> > + return;
> > +#endif
> > +}
>
> My review comment from last time still stands:
>
> > > This is a bad copy of intel_num_cpu_cores(). See for the subtle
> > > difference. Please rename the intel function and move it to common.c
>
> In other words:
>
> Make a patch which moves intel_num_cpu_cores() into common.c. Rename
> the function into something like detect_num_cpu_cores() and fix up the
call
> site in intel.c.
>
> Then add your patch and use the very same function.
>
OK. I got it.
> > +
> > static void early_init_centaur(struct cpuinfo_x86 *c) {
> > + early_init_centaur_mc(c);
> > switch (c->x86) {
> > #ifdef CONFIG_X86_32
> > case 5:
> > @@ -146,6 +163,7 @@ static void centaur_detect_vmx_virtcap(struct
> > cpuinfo_x86 *c)
> >
> > static void init_centaur(struct cpuinfo_x86 *c) {
> > + unsigned int l2 = 0;
> > #ifdef CONFIG_X86_32
> > char *name;
> > u32 fcr_set = 0;
> > @@ -161,6 +179,17 @@ static void init_centaur(struct cpuinfo_x86 *c)
> > #endif
> > early_init_centaur(c);
> >
> > + l2 = init_intel_cacheinfo(c);
> > +
> > + /* Detect legacy cache sizes if init_intel_cacheinfo did not */
> > + if (l2 == 0) {
> > + cpu_detect_cache_sizes(c);
> > + }
>
> Aside of the pointless parentheses, this really wants to be cleaned up as
well.
>
> init_intel_cacheinfo() is called from the intel init code and it does the
same
> silly thing.
>
> So the right thing to do is in a separate patch first
>
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -679,12 +679,6 @@ static void init_intel(struct cpuinfo_x8
>
> l2 = init_intel_cacheinfo(c);
>
> - /* Detect legacy cache sizes if init_intel_cacheinfo did not */
> - if (l2 == 0) {
> - cpu_detect_cache_sizes(c);
> - l2 = c->x86_cache_size;
> - }
> -
> if (c->cpuid_level > 9) {
> unsigned eax = cpuid_eax(10);
> /* Check for version and the number of counters */
> --- a/arch/x86/kernel/cpu/intel_cacheinfo.c
> +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
> @@ -802,6 +802,11 @@ unsigned int init_intel_cacheinfo(struct
>
> c->x86_cache_size = l3 ? l3 : (l2 ? l2 : (l1i+l1d));
>
> + /* Detect legacy cache sizes if the above did not work */
> + if (!l2) {
> + cpu_detect_cache_sizes(c);
> + l2 = c->x86_cache_size;
> + }
> return l2;
> }
>
> Hmm?
>
> tglx
>
OK. I got it.
Patch v3 will be sent later.
Thank you.
---
David