On Thu, Jul 31, 2025 at 05:01:37PM -0700, Dave Hansen wrote: > On 7/31/25 16:45, Sohil Mehta wrote: > > On 7/9/2025 10:00 AM, Dave Hansen wrote: > >> On 7/7/25 01:03, Kirill A. Shutemov wrote: > >>> Instead of moving setup_cr_pinning() below efi_enter_virtual_mode() in > >>> arch_cpu_finalize_init(), defer it until core initcall. > >> What are the side effects of this move? Are there other benefits? What > >> are the risks? > >> > > Picking this up from Kirill.. Reevaluating this, core_initcall() seems > > too late for setup_cr_pinning(). > > > > We need to have CR pinning completed, and the associated static key > > enabled before AP bring up. start_secondary()->cr4_init() depends on the > > cr_pinning static key to initialize CR4 for APs. > > Sure, if you leave cr4_init() completely as-is. > > 'cr4_pinned_bits' should be set by the boot CPU. Secondary CPUs should > also read 'cr4_pinned_bits' when setting up their own cr4's, > unconditionally, independent of 'cr_pinning'. > > The thing I think we should change is the pinning _enforcement_. The > easiest way to do that is to remove the static_branch_likely() in > cr4_init() and then delay flipping the static branch until just before > userspace starts. Yeah, this is fine from my perspective. The goal with the pinning was about keeping things safe in the face of an attack from userspace that managed to get at MSR values and keeping them from being trivially changed. -- Kees Cook
