On 7/31/2025 5:01 PM, Dave Hansen wrote: > > '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. > Based on the current implementation and some git history [1], it seems that cr_pinning is expected to catch 2 types of issues. One is a malicious user trying to change the CR registers from userspace. But, another scenario is a regular caller to native_write_cr4() *mistakenly* clearing a pinned bit. [1]: https://lore.kernel.org/all/alpine.DEB.2.21.1906141646320.1722@xxxxxxxxxxxxxxxxxxxxxxx/ Could deferring enforcement lead to a scenario where we end up with different CR4 values on different CPUs? Maybe I am misinterpreting this and protecting against in-kernel errors is not a goal. In general, you want to delay the CR pinning enforcement until absolutely needed. I am curious about the motivation. I understand we should avoid doing it at arbitrary points in the boot. But, arch_cpu_finalize_init() and early_initcall() seem to be decent mileposts to me. Are you anticipating that we would need to move setup_cr_pinning() again when another user similar to EFI shows up? > Basically, split up the: > > static void __init setup_cr_pinning(void) > { > cr4_pinned_bits = this_cpu_read(cpu_tlbstate.cr4) & cr4_pinned_mask; > static_key_enable(&cr_pinning.key); > } > > code into its two logical pieces: > > 1. Populate 'cr4_pinned_bits' from the boot CPU so the secondaries can > use it > 2. Enable the static key so pinning enforcement is enabled
