Messages in this thread | | | Date | Mon, 22 Feb 2016 20:05:54 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [patch V2 11/28] x86/topology: Create logical package id |
| |
Andi,
On Mon, 22 Feb 2016, Andi Kleen wrote: > Thomas Gleixner <tglx@linutronix.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
| |