Allthough kernel text is always mapped with BATs, we still have
inittext mapped with pages, so TLB miss handling is required
when CONFIG_DEBUG_PAGEALLOC or CONFIG_KFENCE is set.
The final solution should be to set a BAT that also maps inittext
but that BAT then needs to be cleared at end of init, and it will
require more changes to be able to do it properly.
As DEBUG_PAGEALLOC or KFENCE are debugging, performance is not a big
deal so let's fix it simply for now to enable easy stable application.
Reported-by: Maxime Bizon <redacted>
Fixes: 035b19a15a98 ("powerpc/32s: Always map kernel text and rodata with BATs")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/kernel/head_book3s_32.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
That's unrelated to this patch.
The problem is linked to patch c988cfd38e48 ("powerpc/32: use
set_memory_attr()"), which changed from using __set_pte_at() to using
set_memory_attr() which uses set_pte_at().
set_pte_at() has additional checks and shall not be used to updating an
existing PTE.
Wondering if I should just use __set_pte_at() instead like in the past,
or do like commit 9f7853d7609d ("powerpc/mm: Fix set_memory_*() against
concurrent accesses") and use pte_update()
Michael, Aneesh, any suggestion ?
Thanks
Christophe
That's unrelated to this patch.
The problem is linked to patch c988cfd38e48 ("powerpc/32: use
set_memory_attr()"), which changed from using __set_pte_at() to using
set_memory_attr() which uses set_pte_at().
set_pte_at() has additional checks and shall not be used to updating an
existing PTE.
Wondering if I should just use __set_pte_at() instead like in the past,
or do like commit 9f7853d7609d ("powerpc/mm: Fix set_memory_*() against
concurrent accesses") and use pte_update()
Michael, Aneesh, any suggestion ?
The motivation for using pte_update() in that commit is that it does the
update atomically and also handles flushing the HPTE for 64-bit Hash.
But the books/32 version of pte_update() doesn't do that. In fact
there's some HPTE handling in __set_pte_at(), but then also a comment
saying it's handling in a subsequent flush_tlb_xxx().
So that doesn't really help make a decision :)
On the other hand, could you convert those set_memory_attr() calls to
change_memory_attr() and then eventually drop the former?
cheers
That's unrelated to this patch.
The problem is linked to patch c988cfd38e48 ("powerpc/32: use
set_memory_attr()"), which changed from using __set_pte_at() to using
set_memory_attr() which uses set_pte_at().
set_pte_at() has additional checks and shall not be used to updating an
existing PTE.
Wondering if I should just use __set_pte_at() instead like in the past,
or do like commit 9f7853d7609d ("powerpc/mm: Fix set_memory_*() against
concurrent accesses") and use pte_update()
Michael, Aneesh, any suggestion ?
The motivation for using pte_update() in that commit is that it does the
update atomically and also handles flushing the HPTE for 64-bit Hash.
But the books/32 version of pte_update() doesn't do that. In fact
there's some HPTE handling in __set_pte_at(), but then also a comment
saying it's handling in a subsequent flush_tlb_xxx().
So that doesn't really help make a decision :)
On the other hand, could you convert those set_memory_attr() calls to
change_memory_attr() and then eventually drop the former?
Sure, that's probably the best.
Initially I had to implement that set_memory_attr() variant because
change_memory_attr() was doing a pte_clear() that was "sawing off the
branch we're sitting on". In extenso mark_rodata_ro() couldn't use
change_memory_attr() to change the text section to read-only because
mark_rodata_ro() is itself in the text section.
But now that change_memory_attr() is using pte_update() instead of going
via a pte_clear(), it's possible to use it, so that's what I'll do.
Thanks for the idea.
Christophe
Michael, ping.
Le 07/12/2021 à 07:10, Christophe Leroy a écrit :
quoted hunk
Allthough kernel text is always mapped with BATs, we still have
inittext mapped with pages, so TLB miss handling is required
when CONFIG_DEBUG_PAGEALLOC or CONFIG_KFENCE is set.
The final solution should be to set a BAT that also maps inittext
but that BAT then needs to be cleared at end of init, and it will
require more changes to be able to do it properly.
As DEBUG_PAGEALLOC or KFENCE are debugging, performance is not a big
deal so let's fix it simply for now to enable easy stable application.
Reported-by: Maxime Bizon <redacted>
Fixes: 035b19a15a98 ("powerpc/32s: Always map kernel text and rodata with BATs")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>
---
arch/powerpc/kernel/head_book3s_32.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Michael Ellerman <hidden> Date: 2022-02-18 02:24:37
On Tue, 7 Dec 2021 06:10:05 +0000, Christophe Leroy wrote:
Allthough kernel text is always mapped with BATs, we still have
inittext mapped with pages, so TLB miss handling is required
when CONFIG_DEBUG_PAGEALLOC or CONFIG_KFENCE is set.
The final solution should be to set a BAT that also maps inittext
but that BAT then needs to be cleared at end of init, and it will
require more changes to be able to do it properly.
[...]