linux-kernel - Re: [PATCH 03/11] x86 topology: Add CPUID.1F multi-die/package support

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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Feb 2019 13:31:44 +0800
From:   Like Xu <like.xu@...ux.intel.com>
To:     Brice Goglin <Brice.Goglin@...ia.fr>, Len Brown <lenb@...nel.org>,
        x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, Len Brown <len.brown@...el.com>,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH 03/11] x86 topology: Add CPUID.1F multi-die/package
 support

On 2019/2/24 18:04, Brice Goglin wrote:
> Le 19/02/2019 à 04:40, Len Brown a écrit :
>> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
>> index ccd1f2a8e557..4250a87f57db 100644
>> --- a/arch/x86/kernel/smpboot.c
>> +++ b/arch/x86/kernel/smpboot.c
>> @@ -393,6 +393,7 @@ static bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
>>   		int cpu1 = c->cpu_index, cpu2 = o->cpu_index;
>>   
>>   		if (c->phys_proc_id == o->phys_proc_id &&
>> +		    c->cpu_die_id == o->cpu_die_id &&
>>   		    per_cpu(cpu_llc_id, cpu1) == per_cpu(cpu_llc_id, cpu2)) {
>>   			if (c->cpu_core_id == o->cpu_core_id)
>>   				return topology_sane(c, o, "smt");
>> @@ -404,6 +405,7 @@ static bool match_smt(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
>>   		}
>>   
>>   	} else if (c->phys_proc_id == o->phys_proc_id &&
>> +		   c->cpu_die_id == o->cpu_die_id &&
>>   		   c->cpu_core_id == o->cpu_core_id) {
>>   		return topology_sane(c, o, "smt");
>>   	}
>> @@ -461,7 +463,7 @@ static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
>>    */
>>   static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
>>   {
>> -	if (c->phys_proc_id == o->phys_proc_id)
>> +	if (c->cpu_die_id == o->cpu_die_id)
>>   		return true;
>>   	return false;
>>   }
> 
> Hello Len,
> 
> I am testing your patches in a VM with CPUID.1f QEMU patches (author Like Xu is CC'ed),
> booted to get 2 packages with 1 NUMA node each and 2 dies each:
> 
> -smp cpus=16,threads=2,cores=2,dies=2,sockets=2 -numa node,cpus=0-7,nodeid=0 -numa node,cpus=8-15,nodeid=1
> 
> sysfs files expose wrong information:
> core_siblings contains threads of the local die AND of die with same number in other processors,
> eg cpu 0-3 and 8-11 instead of 0-3 only.
> 
> The issue is that you seem to assume that die ids will always be unique across multiple packages.

I don't think the intel MCP topolohgy has this global uniqueness 
assumption for die_id according to CPUID.1F SDM.

> Is this a valid assumption? If so, QEMU patches should be fixed.
> If not, I fixed the issue by changing match_die() to check both phys_proc_id and cpu_die_id:
> 
> static bool match_die(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
> {
> 	if (c->phys_proc_id == o->phys_proc_id && c->cpu_die_id == o->cpu_die_id)
>   		return true;
> 	return false;
> }
> 
> Thanks
> Brice
> 
> 
>