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 <[email protected]>
---
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
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.
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
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.
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
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.
Commit-ID: 964d978433a4b9aa1368ff71227ca0027dd1e32f
Gitweb: https://git.kernel.org/tip/964d978433a4b9aa1368ff71227ca0027dd1e32f
Author: Suravee Suthikulpanit <[email protected]>
AuthorDate: Wed, 13 Jun 2018 13:43:10 -0500
Committer: Thomas Gleixner <[email protected]>
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 <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
---
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;
}