On Fri, Feb 11, 2022 at 11:50:46AM +0100, Jan Beulich wrote: > On 11.02.2022 11:47, Roger Pau Monné wrote: > > On Fri, Feb 11, 2022 at 11:32:45AM +0100, Jan Beulich wrote: > >> On 11.02.2022 10:02, Roger Pau Monné wrote: > >>> On Fri, Feb 11, 2022 at 08:23:27AM +0100, Norbert Manthey wrote: > >>>> When re-identifying CPU data, we might use uninitialized data when > >>>> checking for the cache line property to adapt the cache > >>>> alignment. The data that depends on this uninitialized read is > >>>> currently not forwarded. > >>>> > >>>> To avoid problems in the future, initialize the data cpuinfo > >>>> structure before re-identifying the CPU again. > >>>> > >>>> The trace to hit the uninitialized read reported by Coverity is: > >>>> > >>>> bool recheck_cpu_features(unsigned int cpu) > >>>> ... > >>>> struct cpuinfo_x86 c; > >>>> ... > >>>> identify_cpu(&c); > >>>> > >>>> void identify_cpu(struct cpuinfo_x86 *c) > >>>> ... > >>>> generic_identify(c) > >>>> > >>>> static void generic_identify(struct cpuinfo_x86 *c) > >>>> ... > >>> > >>> Would it be more appropriate for generic_identify to also set > >>> x86_cache_alignment like it's done in early_cpu_init? > >>> > >>> generic_identify already re-fetches a bunch of stuff that's also > >>> set by early_cpu_init for the BSP. > >> > >> This would be an option, but how sure are you that there isn't > >> (going to be) another field with similar properties? We better > >> wouldn't require _everything_ to be re-filled in generic_identify(). > > > > So you think generic_identify should call into early_cpu_init, or even > > split the cpuinfo_x86 filling done in early_cpu_init into a non-init > > function that could be called by both generic_identify and > > early_cpu_init? > > No, I think it is quite fine for this to be a two-step process.
But it's not a two step process for all CPUs. It's a two step process for the BSP, that will get it's cpuinfo filled by early_cpu_init first, and then by identify_cpu. OTHO APs will only get the information filled by identify_cpu. Maybe APs don't care about having x86_cache_alignment correctly set? Thanks, Roger.
