Skip to contentSkip to conversation

x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'

PATCH Superseded Alexander Potapenko 3 participants 4 messages May 8, 2018 – May 9, 2018

Subsystems (2)
Memory management x86

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Alexander Potapenko

author
#0 lore raw

PATCH v2 1/1 x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'

Clang builds with defconfig started crashing after commit fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")
This was caused by introducing a new global access in __startup_64().

Code in __startup_64() can be relocated during execution, but the compiler doesn't have to generate PC-relative relocations when accessing globals from that function. Clang actually does not generate them, which leads to boot-time crashes. To work around this problem, every global pointer must be adjusted using fixup_pointer().

Signed-off-by Alexander Potapenko <[email protected]>
Fixes fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")

---
v2: better patch description, added a comment to __startup_64()

---
 arch/x86/kernel/head64.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
arch/x86/kernel/head64.c +10 -1
index 0c408f8c4ed4..9223792f6d0e 100644
@@ -104,6 +104,13 @@ static bool __head check_la57_support(unsigned long physaddr)
 }
 #endif
 
+
+/* Code in __startup_64() can be relocated during execution, but the compiler
+ * doesn't have to generate PC-relative relocations when accessing globals from
+ * that function. Clang actually does not generate them, which leads to
+ * boot-time crashes. To work around this problem, every global pointer must
+ * be adjusted using fixup_pointer().
+ */
 unsigned long __head __startup_64(unsigned long physaddr,
 				  struct boot_params *bp)
 {

@@ -113,6 +120,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
 	p4dval_t *p4d;
 	pudval_t *pud;
 	pmdval_t *pmd, pmd_entry;
+	pteval_t *mask_ptr;
 	bool la57;
 	int i;
 	unsigned int *next_pgt_ptr;

@@ -196,7 +204,8 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
 	pmd_entry = __PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL;
 	/* Filter out unsupported __PAGE_KERNEL_* bits: */
-	pmd_entry &= __supported_pte_mask;
+	mask_ptr = (pteval_t *)fixup_pointer(&__supported_pte_mask, physaddr);
+	pmd_entry &= *mask_ptr;
 	pmd_entry += sme_get_me_mask();
 	pmd_entry +=  physaddr;
Signature · 1 line
2.17.0.441.gb46fe60e1d-goog
maintainer Reviewed-by on v2 1/1
#1 lore raw
Quoted 9 lines · Alexander Potapenko

Clang builds with defconfig started crashing after commit fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")
This was caused by introducing a new global access in __startup_64().

Code in __startup_64() can be relocated during execution, but the compiler doesn't have to generate PC-relative relocations when accessing globals from that function. Clang actually does not generate them, which leads to boot-time crashes. To work around this problem, every global pointer must be adjusted using fixup_pointer().

Looks good to me. Thanks for adding the comment, especially!

Reviewed-by Dave Hansen <[email protected]>
#2 lore raw
In reply to Alexander Potapenko

arch/x86/kernel/head64.c unsigned long __head __startup_64(unsigned long physaddr, · line 207

Quoted 6 lines · Alexander Potapenko
arch/x86/kernel/head64.c +1 -1
@@ -196,7 +204,8 @@ unsigned long __head __startup_64(unsigned long physaddr,
 
 	pmd_entry = __PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL;
 	/* Filter out unsupported __PAGE_KERNEL_* bits: */
-	pmd_entry &= __supported_pte_mask;
+	mask_ptr = (pteval_t *)fixup_pointer(&__supported_pte_mask, physaddr);

Do we really need the cast here?

Signature · 1 line
 Kirill A. Shutemov
author
#3 lore raw
Quoted 2 lines · Kirill A. Shutemov

Review unsigned long __head __startup_64(unsigned long

Quoted 1 line · Alexander Potapenko
(quoted diff) +0 -0
@@ -196,7 +204,8 @@ unsigned long __head __startup_64(unsigned long

physaddr,

Quoted 5 lines

arch/x86/kernel/head64.c unsigned long __head __startup_64(unsigned long physaddr,

Quoted 5 lines
arch/x86/kernel/head64.c +1 -1
      pmd_entry = __PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL;
      /* Filter out unsupported __PAGE_KERNEL_* bits: */
-     pmd_entry &= __supported_pte_mask;
+     mask_ptr = (pteval_t *)fixup_pointer(&__supported_pte_mask,

physaddr);

Quoted 1 line

Do we really need the cast here?

Correct, we do not.

Quoted 2 lines

--
Kirill A. Shutemov

Signature · 10 lines
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg