On Fri, Nov 06 2020 at 15:29, ira weiny wrote:
> +#ifdef CONFIG_ARCH_HAS_SUPERVISOR_PKEYS
> +/*
> + * PKRS is a per-logical-processor MSR which overlays additional protection
> for
> + * pages which have been mapped with a protection key.
> + *
> + * The register is not maintained with XSAVE so we have to maintain the MSR
> + * value in software during context switch and exception handling.
> + *
> + * Context switches save the MSR in the task struct thus taking that value to
> + * other processors if necessary.
> + *
> + * To protect against exceptions having access to this memory we save the
> + * current running value and set the PKRS value for the duration of the
> + * exception. Thus preventing exception handlers from having the elevated
> + * access of the interrupted task.
> + */
> +noinstr void irq_save_set_pkrs(irqentry_state_t *irq_state, u32 val)
> +{
> + if (!cpu_feature_enabled(X86_FEATURE_PKS))
> + return;
> +
> + irq_state->thread_pkrs = current->thread.saved_pkrs;
> + write_pkrs(INIT_PKRS_VALUE);
Why is this noinstr? Just because it's called from a noinstr function?
Of course the function itself violates the noinstr constraints:
vmlinux.o: warning: objtool: write_pkrs()+0x36: call to do_trace_write_msr()
leaves .noinstr.text section
There is absolutely no reason to have this marked noinstr.
Thanks,
tglx
