From: Aneesh Kumar K V <aneesh.kumar@linux•ibm.com>
To: Nicholas Piggin <npiggin@gmail•com>,
linuxppc-dev@lists•ozlabs.org, mpe@ellerman•id.au,
christophe.leroy@csgroup•eu
Subject: Re: [RFC PATCH] powerpc/book3s/hash: Drop _PAGE_PRIVILEGED from PAGE_NONE
Date: Mon, 13 Nov 2023 21:16:28 +0530 [thread overview]
Message-ID: <4faeb598-87b1-4aca-90cb-3890f95ae7b6@linux.ibm.com> (raw)
In-Reply-To: <CWXNRTMZNWQG.1SU5NB1QVDLO1@wheely>
On 11/13/23 5:17 PM, Nicholas Piggin wrote:
> On Mon Nov 13, 2023 at 8:45 PM AEST, Aneesh Kumar K V wrote:
....
>>>> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
>>>> index ad2afa08e62e..b2eda22195f0 100644
>>>> --- a/arch/powerpc/mm/book3s64/hash_utils.c
>>>> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
>>>> @@ -310,9 +310,26 @@ unsigned long htab_convert_pte_flags(unsigned long pteflags, unsigned long flags
>>>> else
>>>> rflags |= 0x3;
>>>> }
>>>> + WARN_ON(!(pteflags & _PAGE_RWX));
>>>> } else {
>>>> if (pteflags & _PAGE_RWX)
>>>> rflags |= 0x2;
>>>> + else {
>>>> + /*
>>>> + * PAGE_NONE will get mapped to 0b110 (slb key 1 no access)
>>>> + * We picked 0b110 instead of 0b000 so that slb key 0 will
>>>> + * get only read only access for the same rflags.
>>>> + */
>>>> + if (mmu_has_feature(MMU_FTR_KERNEL_RO))
>>>> + rflags |= (HPTE_R_PP0 | 0x2);
>>>> + /*
>>>> + * rflags = HPTE_R_N
>>>> + * Without KERNEL_RO feature this will result in slb
>>>> + * key 0 with read/write. But ISA only supports that.
>>>> + * There is no key 1 no-access and key 0 read-only
>>>> + * pp bit support.
>>>> + */
>>>> + }
>>>> if (!((pteflags & _PAGE_WRITE) && (pteflags & _PAGE_DIRTY)))
>>>> rflags |= 0x1;
>>>> }
>>>
>>
>> V2 is also dropping the above change, because we will never have hash table entries inserted.
>>
>> This is added to commit message.
>>
>> 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.
>
> Should it be a WARN_ON_ONCE? Any useful way to recover from it? Could
> the added WARN come with some comments from that commit that explain
> it?
>
This should never happen unless someone messed up check_pte_access(). The WARN_ON() is a way
to remind me not to add no access ppp mapping in the htab_convert_pte_flags() as I did above.
Initially I was planning to add only a comment, but then in the rare case of we getting it wrong
it is nice to do a console log.
-aneesh
prev parent reply other threads:[~2023-11-13 15:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-02 13:23 [RFC PATCH] powerpc/book3s/hash: Drop _PAGE_PRIVILEGED from PAGE_NONE Aneesh Kumar K.V
2023-11-13 10:16 ` Nicholas Piggin
2023-11-13 10:45 ` Aneesh Kumar K V
2023-11-13 11:47 ` Nicholas Piggin
2023-11-13 15:46 ` Aneesh Kumar K V [this message]
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=4faeb598-87b1-4aca-90cb-3890f95ae7b6@linux.ibm.com \
--to=aneesh.kumar@linux$(echo .)ibm.com \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mpe@ellerman$(echo .)id.au \
--cc=npiggin@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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