[PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Suravee Suthikulpanit]

The current logic incorrectly calculates the LLC ID from the APIC ID.
Unless specified otherwise, the LLC ID should be calculated from
the count order of the number of threads sharing cache.

Fixes: 68091ee7ac3c ("Calculate last level cache ID from number of sharing threads")
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/kernel/cpu/cacheinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 38354c6..0c5fcbd 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -671,7 +671,7 @@ void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, int cpu, u8 node_id)
 			num_sharing_cache = ((eax >> 14) & 0xfff) + 1;
 
 		if (num_sharing_cache) {
-			int bits = get_count_order(num_sharing_cache) - 1;
+			int bits = get_count_order(num_sharing_cache);
 
 			per_cpu(cpu_llc_id, cpu) = c->apicid >> bits;
 		}
-- 
2.7.4

Re: [PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Borislav Petkov]

On Wed, Jun 13, 2018 at 01:43:10PM -0500, Suravee Suthikulpanit wrote:
> The current logic incorrectly calculates the LLC ID from the APIC ID.
> Unless specified otherwise, the LLC ID should be calculated from
> the count order of the number of threads sharing cache.

Don't you mean:

"... should be calculated by removing the Core and Thread ID bits"?

here?

I'm looking at

"2.1.10.2.1.3 ApicId Enumeration Requirements

...

Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
• ApicId[6] = Socket ID.
• ApicId[5:4] = Node ID.
• ApicId[3] = Logical CCX L3 complex ID
• ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : {1'b0,LogicalCoreID[1:0]}.

and in order to get a unique LLC ID, you simply need to shift out the
CoreID and the ThreadId, right?

Or am I misreading it?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Suthikulpanit, Suravee]

Boris,

On 6/18/2018 12:20 PM, Borislav Petkov wrote:
> On Wed, Jun 13, 2018 at 01:43:10PM -0500, Suravee Suthikulpanit wrote:
>> The current logic incorrectly calculates the LLC ID from the APIC ID.
>> Unless specified otherwise, the LLC ID should be calculated from
>> the count order of the number of threads sharing cache.
> 
> Don't you mean:
> 
> "... should be calculated by removing the Core and Thread ID bits"?
> 
> here?
> 
> I'm looking at
> 
> "2.1.10.2.1.3 ApicId Enumeration Requirements
> 
> ...
> 
> Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
> • ApicId[6] = Socket ID.
> • ApicId[5:4] = Node ID.
> • ApicId[3] = Logical CCX L3 complex ID
> • ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : {1'b0,LogicalCoreID[1:0]}.
> 
> and in order to get a unique LLC ID, you simply need to shift out the
> CoreID and the ThreadId, right?
> 
> Or am I misreading it?
> 

This enumeration is only for the family17h model 00-1Fh of hardware 
revision. The patch is intended for the future revision of hardware.

Thanks,
Suravee

Re: [PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Borislav Petkov]

On Mon, Jun 18, 2018 at 01:14:11PM -0500, Suthikulpanit, Suravee wrote:
> This enumeration is only for the family17h model 00-1Fh of hardware
> revision. The patch is intended for the future revision of hardware.

I realized that but the same holds true for the future revision - there
you need to remove thread ID and core ID too, right?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Suthikulpanit, Suravee]

Boris,

On 6/18/2018 1:23 PM, Borislav Petkov wrote:
> On Mon, Jun 18, 2018 at 01:14:11PM -0500, Suthikulpanit, Suravee wrote:
>> This enumeration is only for the family17h model 00-1Fh of hardware
>> revision. The patch is intended for the future revision of hardware.
> 
> I realized that but the same holds true for the future revision - there
> you need to remove thread ID and core ID too, right?
> 

Your understanding is correct. I was not sure about what you were 
referring to earlier. Basically, the number of threads sharing cache is 
used to calculate the amount of right-shifting of APIC ID, which results 
in removing the core and thread ID bits.

Suravee

Re: [PATCH] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[Borislav Petkov]

On Mon, Jun 18, 2018 at 01:46:11PM -0500, Suthikulpanit, Suravee wrote:
> Your understanding is correct. I was not sure about what you were referring
> to earlier. Basically, the number of threads sharing cache is used to
> calculate the amount of right-shifting of APIC ID, which results in removing
> the core and thread ID bits.

Yap, that's exactly what I was hinting at, thanks. I'll add it to the
commit message.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[tip:x86/urgent] x86/CPU/AMD: Fix LLC ID bit-shift calculation

[tip-bot for Suravee Suthikulpanit]

Commit-ID:  964d978433a4b9aa1368ff71227ca0027dd1e32f
Gitweb:     https://git.kernel.org/tip/964d978433a4b9aa1368ff71227ca0027dd1e32f
Author:     Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
AuthorDate: Wed, 13 Jun 2018 13:43:10 -0500
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 22 Jun 2018 21:21:49 +0200

x86/CPU/AMD: Fix LLC ID bit-shift calculation

The current logic incorrectly calculates the LLC ID from the APIC ID.

Unless specified otherwise, the LLC ID should be calculated by removing
the Core and Thread ID bits from the least significant end of the APIC
ID. For more info, see "ApicId Enumeration Requirements" in any Fam17h
PPR document.

[ bp: Improve commit message. ]

Fixes: 68091ee7ac3c ("Calculate last level cache ID from number of sharing threads")
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1528915390-30533-1-git-send-email-suravee.suthikulpanit@amd.com
---
 arch/x86/kernel/cpu/cacheinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/cacheinfo.c b/arch/x86/kernel/cpu/cacheinfo.c
index 38354c66df81..0c5fcbd998cf 100644
--- a/arch/x86/kernel/cpu/cacheinfo.c
+++ b/arch/x86/kernel/cpu/cacheinfo.c
@@ -671,7 +671,7 @@ void cacheinfo_amd_init_llc_id(struct cpuinfo_x86 *c, int cpu, u8 node_id)
 			num_sharing_cache = ((eax >> 14) & 0xfff) + 1;
 
 		if (num_sharing_cache) {
-			int bits = get_count_order(num_sharing_cache) - 1;
+			int bits = get_count_order(num_sharing_cache);
 
 			per_cpu(cpu_llc_id, cpu) = c->apicid >> bits;
 		}