LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'
@ 2018-05-08 12:16 Alexander Potapenko
  2018-05-08 14:30 ` Dave Hansen
  0 siblings, 1 reply; 4+ messages in thread
From: Alexander Potapenko @ 2018-05-08 12:16 UTC (permalink / raw)
  To: dave.hansen, mingo, kirill.shutemov
  Cc: linux-mm, linux-kernel, mka, dvyukov, md

Similarly to commit 187e91fe5e91
("x86/boot/64/clang: Use fixup_pointer() to access 'next_early_pgt'"),
'__supported_pte_mask' must be also accessed using fixup_pointer() to
avoid position-dependent relocations.

Signed-off-by: Alexander Potapenko <glider@google.com>
Fixes: fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")
---
 arch/x86/kernel/head64.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 0c408f8c4ed4..1b36ae4d0035 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -113,6 +113,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 +197,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;
 
-- 
2.17.0.441.gb46fe60e1d-goog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'
  2018-05-08 12:16 [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask' Alexander Potapenko
@ 2018-05-08 14:30 ` Dave Hansen
  2018-05-08 14:50   ` Alexander Potapenko
  0 siblings, 1 reply; 4+ messages in thread
From: Dave Hansen @ 2018-05-08 14:30 UTC (permalink / raw)
  To: Alexander Potapenko, mingo, kirill.shutemov
  Cc: linux-mm, linux-kernel, mka, dvyukov, md

On 05/08/2018 05:16 AM, Alexander Potapenko wrote:
> Similarly to commit 187e91fe5e91
> ("x86/boot/64/clang: Use fixup_pointer() to access 'next_early_pgt'"),
> '__supported_pte_mask' must be also accessed using fixup_pointer() to
> avoid position-dependent relocations.
> 
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Fixes: fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")

In the interests of standalone changelogs, I'd really appreciate an
actual explanation of what's going on here.  Your patch makes the code
uglier and doesn't fix anything functional from what I can see.

The other commit has some explanation, so it seems like the rules for
accessing globals in head64.c are different than other files because...
something.

The functional problem here is that it causes insta-reboots?

Do we have anything we can do to keep us from recreating these kinds of
regressions all the time?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'
  2018-05-08 14:30 ` Dave Hansen
@ 2018-05-08 14:50   ` Alexander Potapenko
  2018-05-08 16:25     ` Dave Hansen
  0 siblings, 1 reply; 4+ messages in thread
From: Alexander Potapenko @ 2018-05-08 14:50 UTC (permalink / raw)
  To: dave.hansen
  Cc: Ingo Molnar, Kirill A. Shutemov, Linux Memory Management List,
	LKML, Matthias Kaehlcke, Dmitriy Vyukov, Michael Davidson

On Tue, May 8, 2018 at 4:30 PM Dave Hansen <dave.hansen@linux.intel.com>
wrote:

> On 05/08/2018 05:16 AM, Alexander Potapenko wrote:
> > Similarly to commit 187e91fe5e91
> > ("x86/boot/64/clang: Use fixup_pointer() to access 'next_early_pgt'"),
> > '__supported_pte_mask' must be also accessed using fixup_pointer() to
> > avoid position-dependent relocations.
> >
> > Signed-off-by: Alexander Potapenko <glider@google.com>
> > Fixes: fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")

> In the interests of standalone changelogs, I'd really appreciate an
> actual explanation of what's going on here.  Your patch makes the code
> uglier and doesn't fix anything functional from what I can see.
You're right, sure. I'll send a patch with an updated description.

> The other commit has some explanation, so it seems like the rules for
> accessing globals in head64.c are different than other files because...
> something.
The problem as far as I understand it is that the 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.

> The functional problem here is that it causes insta-reboots?
True.

> Do we have anything we can do to keep us from recreating these kinds of
> regressions all the time?
I'm not really aware of the possible options in the kernel land. Looks like
a task for some objtool-like utility?
As long as these regressions are caught with Clang, setting up a 0day Clang
builder might help.
At least I should've added a comment regarding this to __startup_64() last
time.



-- 
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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask'
  2018-05-08 14:50   ` Alexander Potapenko
@ 2018-05-08 16:25     ` Dave Hansen
  0 siblings, 0 replies; 4+ messages in thread
From: Dave Hansen @ 2018-05-08 16:25 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Ingo Molnar, Kirill A. Shutemov, Linux Memory Management List,
	LKML, Matthias Kaehlcke, Dmitriy Vyukov, Michael Davidson

On 05/08/2018 07:50 AM, Alexander Potapenko wrote:
>>> Similarly to commit 187e91fe5e91
>>> ("x86/boot/64/clang: Use fixup_pointer() to access 'next_early_pgt'"),
>>> '__supported_pte_mask' must be also accessed using fixup_pointer() to
>>> avoid position-dependent relocations.
>>>
>>> Signed-off-by: Alexander Potapenko <glider@google.com>
>>> Fixes: fb43d6cb91ef ("x86/mm: Do not auto-massage page protections")
> 
>> In the interests of standalone changelogs, I'd really appreciate an
>> actual explanation of what's going on here.  Your patch makes the code
>> uglier and doesn't fix anything functional from what I can see.
> You're right, sure. I'll send a patch with an updated description.

Great, thanks!

>> Do we have anything we can do to keep us from recreating these kinds of
>> regressions all the time?
> I'm not really aware of the possible options in the kernel land. Looks like
> a task for some objtool-like utility?
> As long as these regressions are caught with Clang, setting up a 0day Clang
> builder might help.

I've asked the 0day folks if this is doable.  It would be great to see
it added.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-08 16:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 12:16 [PATCH] x86/boot/64/clang: Use fixup_pointer() to access '__supported_pte_mask' Alexander Potapenko
2018-05-08 14:30 ` Dave Hansen
2018-05-08 14:50   ` Alexander Potapenko
2018-05-08 16:25     ` Dave Hansen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).