lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH V8 14/44] x86/pkeys: Introduce pks_write_pkrs()
Date
From: Ira Weiny <ira.weiny@intel.com>

Writing to MSR's is inefficient. Even though the underlying
WRMSR(MSR_IA32_PKRS) is not serializing (see below), writing to the MSR
unnecessarily should be avoided. This is especially true when the value
of the PKS protections is unlikely to change from the default often.

Introduce pks_write_pkrs() which avoids writing the MSR if the pkrs
value has not changed for the CPU. Do this by utilizing a per-cpu
cache. Protect the use of the cached value from preemption by
restricting the use of pks_write_pkrs() to non-preemptable context.
Further restrict it's use to callers which have checked X86_FEATURE_PKS.

The initial value of the MSR is preserved on INIT. While unlikely, the
PKS_INIT_VALUE may be 0 someday which would prevent pks_write_pkrs()
from updating the MSR. Keep the MSR write in pks_setup() to ensure the
MSR is initialized at least one time. Then call pks_write_pkrs() to set
up the per-cache value to ensure it is in sync with the MSR.

It should be noted that the underlying WRMSR(MSR_IA32_PKRS) is not
serializing but still maintains ordering properties similar to WRPKRU.
The current SDM section on PKRS needs updating but should be the same as
that of WRPKRU. So to quote from the WRPKRU text:

WRPKRU will never execute transiently. Memory accesses affected
by PKRU register will not execute (even transiently) until all
prior executions of WRPKRU have completed execution and updated
the PKRU register.

Suggested-by: Dave Hansen <dave.hansen@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes for V8
From Thomas
Remove get/put_cpu_ptr() and make this a 'lower level
call. This makes it preemption unsafe but it is called
mostly where preemption is already disabled. Add this
as a predicate of the call and those calls which need to
can disable preemption.
Add lockdep assert for preemption
Ensure MSR gets written even if the PKS_INIT_VALUE is 0.
Completely re-write the commit message.
s/write_pkrs/pks_write_pkrs/
Split this off into a singular patch

Changes for V7
Create a dynamic pkrs_initial_value in early init code.
Clean up comments
Add comment to macro guard
---
arch/x86/mm/pkeys.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index a5b5b86e97ce..3dce99ef4127 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -209,15 +209,56 @@ u32 pkey_update_pkval(u32 pkval, int pkey, u32 accessbits)

#ifdef CONFIG_ARCH_ENABLE_SUPERVISOR_PKEYS

+static DEFINE_PER_CPU(u32, pkrs_cache);
+
+/*
+ * pks_write_pkrs() - Write the pkrs of the current CPU
+ * @new_pkrs: New value to write to the current CPU register
+ *
+ * Optimizes the MSR writes by maintaining a per cpu cache.
+ *
+ * Context: must be called with preemption disabled
+ * Context: must only be called if PKS is enabled
+ *
+ * It should also be noted that the underlying WRMSR(MSR_IA32_PKRS) is not
+ * serializing but still maintains ordering properties similar to WRPKRU.
+ * The current SDM section on PKRS needs updating but should be the same as
+ * that of WRPKRU. Quote from the WRPKRU text:
+ *
+ * WRPKRU will never execute transiently. Memory accesses
+ * affected by PKRU register will not execute (even transiently)
+ * until all prior executions of WRPKRU have completed execution
+ * and updated the PKRU register.
+ */
+static inline void pks_write_pkrs(u32 new_pkrs)
+{
+ u32 pkrs = __this_cpu_read(pkrs_cache);
+
+ lockdep_assert_preemption_disabled();
+
+ if (pkrs != new_pkrs) {
+ __this_cpu_write(pkrs_cache, new_pkrs);
+ wrmsrl(MSR_IA32_PKRS, new_pkrs);
+ }
+}
+
/*
* PKS is independent of PKU and either or both may be supported on a CPU.
+ *
+ * Context: must be called with preemption disabled
*/
void pks_setup(void)
{
if (!cpu_feature_enabled(X86_FEATURE_PKS))
return;

+ /*
+ * If the PKS_INIT_VALUE is 0 then pks_write_pkrs() could fail to
+ * initialize the MSR. Do a single write here to ensure the MSR is
+ * written at least one time.
+ */
wrmsrl(MSR_IA32_PKRS, PKS_INIT_VALUE);
+ pks_write_pkrs(PKS_INIT_VALUE);
cr4_set_bits(X86_CR4_PKS);
}

--
2.31.1
\
 
 \ /
  Last update: 2022-01-27 18:56    [W:1.837 / U:0.376 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site