Andi,
On Mon, 22 Feb 2016, Andi Kleen wrote:
> Thomas Gleixner <[email protected]> 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
