[PATCH v2] powerpc/book3s/hash: Drop _PAGE_PRIVILEGED from PAGE_NONE
Christophe Leroy
christophe.leroy at csgroup.eu
Fri Dec 1 21:44:21 AEDT 2023
Le 01/12/2023 à 11:35, Michael Ellerman a écrit :
> "Aneesh Kumar K.V" <aneesh.kumar at linux.ibm.com> writes:
>> There used to be a dependency on _PAGE_PRIVILEGED with pte_savedwrite.
>> But that got dropped by
>> commit 6a56ccbcf6c6 ("mm/autonuma: use can_change_(pte|pmd)_writable() to replace savedwrite")
>>
>> With the change in this patch numa fault pte (pte_protnone()) gets mapped as regular user pte
>> with RWX cleared (no-access) whereas earlier it used to be mapped _PAGE_PRIVILEGED.
>>
>> Hash fault handling code did get some WARN_ON added because those
>> functions are not expected to get called with _PAGE_READ cleared.
>> commit 18061c17c8ec ("powerpc/mm: Update PROTFAULT handling in the page fault path")
>> explains the details.
>
> You say "did get" which makes me think you're talking about the past.
> But I think you're referring to the WARN_ON you are adding in this patch?
>
>> Also revert commit 1abce0580b89 ("powerpc/64s: Fix __pte_needs_flush() false positive warning")
>
> That could be done separately as a follow-up couldn't it? Would reduce
> the diff size.
>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/book3s/64/pgtable.h | 9 +++------
>> arch/powerpc/include/asm/book3s/64/tlbflush.h | 9 ++-------
>> arch/powerpc/mm/book3s64/hash_utils.c | 7 +++++++
>> 3 files changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> index cb77eddca54b..2cc58ac74080 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
>> @@ -17,12 +17,6 @@
>> #define _PAGE_EXEC 0x00001 /* execute permission */
>> #define _PAGE_WRITE 0x00002 /* write access allowed */
>> #define _PAGE_READ 0x00004 /* read access allowed */
>> -#define _PAGE_NA _PAGE_PRIVILEGED
>
>> -#define _PAGE_NAX _PAGE_EXEC
>> -#define _PAGE_RO _PAGE_READ
>> -#define _PAGE_ROX (_PAGE_READ | _PAGE_EXEC)
>> -#define _PAGE_RW (_PAGE_READ | _PAGE_WRITE)
>> -#define _PAGE_RWX (_PAGE_READ | _PAGE_WRITE | _PAGE_EXEC)
>
> Those are unrelated I think?
They are related. Those exists only because _PAGE_NA is different from
the one defined in asm/pgtable-masks.h
As soon as you remove _PAGE_PRIVILEGED from _PAGE_NA, everything become
standard and is taken from asm/pgtable-masks.h
>
>> #define _PAGE_PRIVILEGED 0x00008 /* kernel access only */
>> #define _PAGE_SAO 0x00010 /* Strong access order */
>> #define _PAGE_NON_IDEMPOTENT 0x00020 /* non idempotent memory */
>> @@ -529,6 +523,9 @@ static inline bool pte_user(pte_t pte)
>> }
>>
>> #define pte_access_permitted pte_access_permitted
>> +/*
>> + * execute-only mappings return false
>> + */
>
> That would fit better in the existing comment block inside the function
> I think. Normally this location would be a function description comment.
>
>> static inline bool pte_access_permitted(pte_t pte, bool write)
>> {
>> /*
> ie. here
>
> cheers
More information about the Linuxppc-dev
mailing list