Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch v5 06/19] x86/cpu: Provide a sane leaf 0xb/0x1f parser | Date | Mon, 12 Feb 2024 15:17:45 +0100 |
| |
On Tue, Jan 30 2024 at 20:31, Borislav Petkov wrote: > On Tue, Jan 23, 2024 at 01:53:39PM +0100, Thomas Gleixner wrote: >> +static inline bool topo_subleaf(struct topo_scan *tscan, u32 leaf, u32 subleaf, > > "parse_topo_subleaf"? > > With a verb in the name... > >> + unsigned int *last_dom) >> +{ >> + unsigned int dom, maxtype; >> + const unsigned int *map; >> + struct { >> + // eax > > Can we please not use those yucky // comments together with the > multiline ones?
TBH, the // comment style is really better for struct definitions. It's denser and easier to parse.
// eax u32 x2apic_shift : 5, // Number of bits to shift APIC ID right // for the topology ID at the next level : 27; // Reserved // ebx u32 num_processors : 16, // Number of processors at current level : 16; // Reserved
versus:
/* eax */ u32 x2apic_shift : 5, /* * Number of bits to shift APIC ID right * for the topology ID at the next level */ : 27; /* Reserved */
/* ebx */ u32 num_processors : 16, /* Number of processors at current level */ : 16; /* Reserved */
Especially x2apic_shift is horrible and the comments of EBX are visually impaired while with the C++ comments x2apic_shift looks natural and the EBX comments are just open to the right and therefore simpler.
>> + if (!tscan->dom_shifts[TOPO_SMT_DOMAIN] && tscan->dom_ncpus[TOPO_SMT_DOMAIN] > 1) { >> + unsigned int sft = get_count_order(tscan->dom_ncpus[TOPO_SMT_DOMAIN]); >> + >> + pr_warn_once(FW_BUG "CPUID leaf 0x%x subleaf 0 has shift level 0 but %u CPUs\n", >> + leaf, tscan->dom_ncpus[TOPO_SMT_DOMAIN]); > > Do you really wanna warn about that? Hoping that someone would do > something about it while there's time...?
If it's caught in early testing, this should be fixed, no?
Thanks,
tglx
| |