linux-kernel - Re: [patch V2 11/28] x86/topology: Create logical package id

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]

Message-ID: <alpine.DEB.2.11.1602221959250.2477@nanos>
Date:	Mon, 22 Feb 2016 20:05:54 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Andi Kleen <andi@...stfloor.org>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>, x86@...nel.org,
	Borislav Petkov <bp@...en8.de>,
	Stephane Eranian <eranian@...gle.com>,
	Harish Chegondi <harish.chegondi@...el.com>,
	Kan Liang <kan.liang@...el.com>,
	Andi Kleen <andi.kleen@...el.com>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>
Subject: Re: [patch V2 11/28] x86/topology: Create logical package id

Andi,

On Mon, 22 Feb 2016, Andi Kleen wrote:
> Thomas Gleixner <tglx@...utronix.de> writes:
> > +	if (c->cpuid_level >= 0x00000001) {
> > +		u32 eax, ebx, ecx, edx;
> > +
> > +		cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
> 
> Use cpuid_edx()

That does not give me EBX ...
 
> > +		/*
> > +		 * If HTT (EDX[28]) is set EBX[16:23] contain the number of
> > +		 * apicids which are reserved per package. Store the resulting
> > +		 * shift value for the package management code.
> > +		 */
> > +		if (edx & (1U << 28))
> > +			c->x86_coreid_bits = get_count_order((ebx >> 16) & 0xff);
> > +	}
> > +++ b/arch/x86/kernel/cpu/proc.c
> > @@ -12,6 +12,7 @@ static void show_cpuinfo_core(struct seq
> >  {
> >  #ifdef CONFIG_SMP
> >  	seq_printf(m, "physical id\t: %d\n", c->phys_proc_id);
> > +	seq_printf(m, "logical id\t: %d\n", c->logical_proc_id);
> 
> 
> I'm not sure it makes sense to export this. What good would it be for
> the user?
>
> If it was it would need to be documented somewhere. But I would
> just drop it and keep it kernel internal.

You are right. We print already when we change the package number, so it can
be retrieved from dmesg.
 
> FWIW every time something is added to this file it usually breaks
> some (dumb) programs.

Ok, did not think about that.
 
> > +	/*
> > +	 * Today neither Intel nor AMD support heterogenous systems. That
> > +	 * might change in the future....
> > +	 */
> > +	ncpus = boot_cpu_data.x86_max_cores * smp_num_siblings;
> > +	__max_logical_packages = DIV_ROUND_UP(nr_cpu_ids, ncpus);
> 
> FWIW Hypervisors can do nearly everything today.
> 
> I assume your code handles it.

It should. At least as long as nr_cpu_ids is sufficiently large.
 
> Let's hope that the Hypervisors always set up the correct CPUID now
> for their sibling configuration. If they don't with this change
> some CPUs would be suddenly lost.

The ones I looked at are doing is sane. Famous last words :)
 
> Would it be worth to have a kernel option where the maximum can be overriden
> in case this happens?

Let's wait for it to happen. It's done in no time, but if not needed it's just
ballast.

Thanks,

	tglx