linux-kernel - Re: [PATCH] x86/AMD: Fix LLC ID for AMD Fam17h systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b2f643aa-d161-0dbc-500c-d86426c85170@amd.com>
Date:   Thu, 27 Oct 2016 09:58:51 -0400
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     LKML <linux-kernel@...r.kernel.org>, <x86@...nel.org>, <bp@...e.de>
Subject: Re: [PATCH] x86/AMD: Fix LLC ID for AMD Fam17h systems

>> +			/*
>> +			 * LLC is at the Core Complex level.
>> +			 * Core Complex Id is ApicId[3].
>> +			 */
>> +			else if (c->x86 == 0x17)
>> +				per_cpu(cpu_llc_id, cpu) = c->initial_apicid >> 3;
> 
> This whole if/else block lacks curly braces. See:
> 
>   https://marc.info/?l=linux-kernel&m=147351236615103
> 

Okay, understood.

>> -	/*
>> -	 * Fix percpu cpu_llc_id here as LLC topology is different
>> -	 * for Fam17h systems.
>> -	 */
>> -	 if (c->x86 != 0x17 || !cpuid_edx(0x80000006))
>> -		return;
>> -
>> -	socket_id	= (c->apicid >> bits) - 1;
>> -	core_complex_id	= (c->apicid & ((1 << bits) - 1)) >> 3;
>> -
>> -	per_cpu(cpu_llc_id, cpu) = (socket_id << 3) | core_complex_id;
> 
> So if I've read the patch correctly then the trivial fix for fam17h would
> have been:
> 
>> +	per_cpu(cpu_llc_id, cpu) = c->initial_apicid >> 3;
> 
> Right?
> 

Right.

> And this one liner wants to be a seperate patch with a proper
> explanation. And that simple hunk can be tagged for stable.
> 

Okay, I'll make it a separate patch. It still wouldn't be a one-liner
because then socket_id and core_complex_id would be left unused and
should be removed.

> The rest of the patch is cleanup and improvement and want's to be seperated
> out and explained proper.
> 

Okay, will do.

Thanks,
Yazen