From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Nicholas Piggin <npiggin@gmail.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Michael Ellerman <mpe@ellerman.id.au>,
Paul Mackerras <paulus@samba.org>
Cc: linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 1/3] powerpc/nohash: Fix __ptep_set_access_flags() and ptep_set_wrprotect()
Date: Wed, 27 Oct 2021 06:39:18 +0200 [thread overview]
Message-ID: <8e2c89a4-e2e9-e441-b75d-f27bd75221fc@csgroup.eu> (raw)
In-Reply-To: <1635308538.6vye6lbbh8.astroid@bobo.none>
Le 27/10/2021 à 06:23, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of October 26, 2021 3:39 pm:
>> Commit 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
>> changed those two functions to use pte helpers to determine which
>> bits to clear and which bits to set.
>>
>> This change was based on the assumption that bits to be set/cleared
>> are always the same and can be determined by applying the pte
>> manipulation helpers on __pte(0).
>>
>> But on platforms like book3e, the bits depend on whether the page
>> is a user page or not.
>>
>> For the time being it more or less works because of _PAGE_EXEC being
>> used for user pages only and exec right being set at all time on
>> kernel page. But following patch will clean that and output of
>> pte_mkexec() will depend on the page being a user or kernel page.
>>
>> Instead of trying to make an even more complicated helper where bits
>> would become dependent on the final pte value, come back to a more
>> static situation like before commit 26973fa5ac0e ("powerpc/mm: use
>> pte helpers in generic code"), by introducing an 8xx specific
>> version of __ptep_set_access_flags() and ptep_set_wrprotect().
>
> What is this actually fixing? Does it change anything itself, or
> just a preparation patch?
Just a preparation patch I think.
I didn't flag it for stable.
Once patch 2 is applied, __ptep_set_access_flags() doesn't work anymore
without this patch, because then pte_mkexec(__pte(0)) sets SX and clears
UX while pte_mkexec(__pte(~0)) sets UX and clears SX
Christophe
>
> Thanks,
> Nick
>
>>
>> Fixes: 26973fa5ac0e ("powerpc/mm: use pte helpers in generic code")
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> v3: No change
>> v2: New
>> ---
>> arch/powerpc/include/asm/nohash/32/pgtable.h | 17 +++++++--------
>> arch/powerpc/include/asm/nohash/32/pte-8xx.h | 22 ++++++++++++++++++++
>> 2 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
>> index 34ce50da1850..11c6849f7864 100644
>> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
>> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
>> @@ -306,30 +306,29 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
>> }
>>
>> #define __HAVE_ARCH_PTEP_SET_WRPROTECT
>> +#ifndef ptep_set_wrprotect
>> static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr,
>> pte_t *ptep)
>> {
>> - unsigned long clr = ~pte_val(pte_wrprotect(__pte(~0)));
>> - unsigned long set = pte_val(pte_wrprotect(__pte(0)));
>> -
>> - pte_update(mm, addr, ptep, clr, set, 0);
>> + pte_update(mm, addr, ptep, _PAGE_RW, 0, 0);
>> }
>> +#endif
>>
>> +#ifndef __ptep_set_access_flags
>> static inline void __ptep_set_access_flags(struct vm_area_struct *vma,
>> pte_t *ptep, pte_t entry,
>> unsigned long address,
>> int psize)
>> {
>> - pte_t pte_set = pte_mkyoung(pte_mkdirty(pte_mkwrite(pte_mkexec(__pte(0)))));
>> - pte_t pte_clr = pte_mkyoung(pte_mkdirty(pte_mkwrite(pte_mkexec(__pte(~0)))));
>> - unsigned long set = pte_val(entry) & pte_val(pte_set);
>> - unsigned long clr = ~pte_val(entry) & ~pte_val(pte_clr);
>> + unsigned long set = pte_val(entry) &
>> + (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_RW | _PAGE_EXEC);
>> int huge = psize > mmu_virtual_psize ? 1 : 0;
>>
>> - pte_update(vma->vm_mm, address, ptep, clr, set, huge);
>> + pte_update(vma->vm_mm, address, ptep, 0, set, huge);
>>
>> flush_tlb_page(vma, address);
>> }
>> +#endif
>>
>> static inline int pte_young(pte_t pte)
>> {
>> diff --git a/arch/powerpc/include/asm/nohash/32/pte-8xx.h b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
>> index fcc48d590d88..1a89ebdc3acc 100644
>> --- a/arch/powerpc/include/asm/nohash/32/pte-8xx.h
>> +++ b/arch/powerpc/include/asm/nohash/32/pte-8xx.h
>> @@ -136,6 +136,28 @@ static inline pte_t pte_mkhuge(pte_t pte)
>>
>> #define pte_mkhuge pte_mkhuge
>>
>> +static inline pte_basic_t pte_update(struct mm_struct *mm, unsigned long addr, pte_t *p,
>> + unsigned long clr, unsigned long set, int huge);
>> +
>> +static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addr, pte_t *ptep)
>> +{
>> + pte_update(mm, addr, ptep, 0, _PAGE_RO, 0);
>> +}
>> +#define ptep_set_wrprotect ptep_set_wrprotect
>> +
>> +static inline void __ptep_set_access_flags(struct vm_area_struct *vma, pte_t *ptep,
>> + pte_t entry, unsigned long address, int psize)
>> +{
>> + unsigned long set = pte_val(entry) & (_PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_EXEC);
>> + unsigned long clr = ~pte_val(entry) & _PAGE_RO;
>> + int huge = psize > mmu_virtual_psize ? 1 : 0;
>> +
>> + pte_update(vma->vm_mm, address, ptep, clr, set, huge);
>> +
>> + flush_tlb_page(vma, address);
>> +}
>> +#define __ptep_set_access_flags __ptep_set_access_flags
>> +
>> static inline unsigned long pgd_leaf_size(pgd_t pgd)
>> {
>> if (pgd_val(pgd) & _PMD_PAGE_8M)
>> --
>> 2.31.1
>>
>>
next prev parent reply other threads:[~2021-10-27 4:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 5:39 [PATCH 1/3] powerpc/nohash: Fix __ptep_set_access_flags() and ptep_set_wrprotect() Christophe Leroy
2021-10-26 5:39 ` [PATCH 2/3] powerpc/book3e: Fix set_memory_x() and set_memory_nx() Christophe Leroy
2021-10-27 4:44 ` Nicholas Piggin
2021-10-27 4:55 ` Christophe Leroy
2021-10-27 5:27 ` Nicholas Piggin
2021-10-27 5:50 ` Christophe Leroy
2021-10-27 7:15 ` Nicholas Piggin
2021-10-28 11:33 ` Michael Ellerman
2021-10-26 5:39 ` [PATCH 3/3] powerpc/fsl_booke: Fix setting of exec flag when setting TLBCAMs Christophe Leroy
2021-10-27 4:23 ` [PATCH 1/3] powerpc/nohash: Fix __ptep_set_access_flags() and ptep_set_wrprotect() Nicholas Piggin
2021-10-27 4:39 ` Christophe Leroy [this message]
2021-11-02 10:11 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8e2c89a4-e2e9-e441-b75d-f27bd75221fc@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=benh@kernel.crashing.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).