Messages in this thread | | | Subject | Re: [PATCH v2 1/2] x86/amd: Refactor topology extension related code | From | Suravee Suthikulpanit <> | Date | Mon, 24 Jul 2017 10:28:34 +0700 |
| |
Boris,
On 7/22/17 23:12, Borislav Petkov wrote: > On Fri, Jul 21, 2017 at 09:00:38PM -0500, Suravee Suthikulpanit wrote: >> Refactoring in preparation for subsequent changes. >> There is no functional change. >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> --- >> arch/x86/kernel/cpu/amd.c | 79 ++++++++++++++++++++++++++--------------------- >> 1 file changed, 44 insertions(+), 35 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c >> index bb5abe8..74d8d7c 100644 >> --- a/arch/x86/kernel/cpu/amd.c >> +++ b/arch/x86/kernel/cpu/amd.c >> @@ -297,54 +297,63 @@ static int nearby_node(int apicid) >> #endif >> >> /* >> - * Fixup core topology information for >> - * (1) AMD multi-node processors >> - * Assumption: Number of cores in each internal node is the same. >> - * (2) AMD processors supporting compute units >> + * Get topology information via X86_FEATURE_TOPOEXT >> */ >> -#ifdef CONFIG_SMP >> -static void amd_get_topology(struct cpuinfo_x86 *c) >> +static void __get_topoext(struct cpuinfo_x86 *c) >> { >> - u8 node_id; >> + u32 eax, ebx, ecx, edx; >> int cpu = smp_processor_id(); >> >> - /* get information required for multi-node processors */ >> - if (boot_cpu_has(X86_FEATURE_TOPOEXT)) { >> - u32 eax, ebx, ecx, edx; >> + cpuid(0x8000001e, &eax, &ebx, &ecx, &edx); >> >> - cpuid(0x8000001e, &eax, &ebx, &ecx, &edx); >> + smp_num_siblings = ((ebx >> 8) & 0xff) + 1; >> >> - node_id = ecx & 0xff; > > When reviewers ask you about a preparatory cleanup patch, you don't > sneak in changes in it - you *only* *move* the code so that the change > is *absolutely* comprehensible. Ontop you do changes. Don't tell me you > didn't know that!
I know that we should not sneak in change. I might have missed something here.
Are you referring to the part that I moved the "node_id = ecx & 0xff" from the top level of the function to inside the "if/else" logic where it is the only place that used within this new refactored __get_topoext() and there is nothing changed functionally? If that's really the case, I'll fix it.
Thanks, Suravee
| |