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