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? Thanks, Roger.
