Messages in this thread | | | Date | Fri, 22 Jun 2018 18:16:05 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH REBASED RESEND] x86/cpu: Move early cpu initialization into a separate translation unit |
| |
On Fri, 22 Jun 2018, Kirill A. Shutemov wrote: > On Fri, Jun 22, 2018 at 03:35:18PM +0000, Thomas Gleixner wrote: > > > > Second thoughts. > > > > > > > > The only place where __pgtable_l5_enabled() is used in common.c is in > > > > early_identify_cpu() which is marked __init. So how is that section > > > > mismatch triggered? > > > > > > Yeah, it's not obvious: > > > > > > cpu_init() > > > load_mm_ldt() > > > ldt_slot_va() > > > LDT_BASE_ADDR > > > LDT_PGD_ENTRY > > > pgtable_l5_enabled() > > > > How is that supposed to work correctly? > > > > start_kernel() > > .... > > trap_init() > > cpu_init() > > > > .... > > check_bugs() > > alternative_instructions() > > > > So the first invocation of cpu_init() on the boot CPU will then use > > static_cpu_has() which is not yet initialized proper. > > Ouch. > > Is there a way to catch such improper static_cpu_has() users? > Silent misbehaviour is risky.
Yes, it is. I don't think we have something in place right now, but we should add it definitely. PeterZ ????
> > So, no. That does not work and the proper fix is: > > > > -unsigned int __pgtable_l5_enabled __initdata; > > +unsigned int __pgtable_l5_enabled __ro_after_init; > > > > and make cpu/common.c use the early variant. The extra 4 bytes storage are > > not a problem and cpu_init() is not a fast path at all. > > Okay, I'll prepare the patch. > > BTW, if we go this path after all, shouldn't we revert these: > > 046c0dbec023 ("x86: Mark native_set_p4d() as __always_inline") > 1ea66554d3b0 ("x86/mm: Mark p4d_offset() __always_inline")
In principle the always inline is fine, but the changelogs are quite misleading and I really regret that I did not take the time to analyse that proper when I applied the patches. At least we have catched it now.
So yes, please send the reverts along. Can you please add a proper root cause analysis for the issues Arnd has observed to the changelogs so we have it documented for later.
Thanks,
tglx
| |