On Fri, Jun 22, 2018 at 03:35:18PM +0000, Thomas Gleixner wrote: > On Fri, 22 Jun 2018, Kirill A. Shutemov wrote: > > On Fri, Jun 22, 2018 at 02:05:47PM +0000, Thomas Gleixner wrote: > > > On Tue, 12 Jun 2018, Kirill A. Shutemov wrote: > > > > > > > __pgtable_l5_enabled shouldn't be needed after system has booted, we can > > > > mark it as __initdata, but it requires preparation. > > > > > > > > This patch moves early cpu initialization into a separate translation > > > > unit. This limits effect of USE_EARLY_PGTABLE_L5 to less code. > > > > > > > > Without the change cpu_init() uses __pgtable_l5_enabled. cpu_init() is > > > > not __init function and it leads to section mismatch. > > > > > > > > Signed-off-by: Kirill A. Shutemov <kirill.shute...@linux.intel.com> > > > > Reviewed-by: Thomas Gleixner <t...@linutronix.de> > > > > > > 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. > 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") ? I can send it as part of the patchset. -- Kirill A. Shutemov