linux-kernel - Re: [PATCH 3/3] powerpc/8xx: Provide ptep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wo45db8d.fsf@mpe.ellerman.id.au>
Date:   Thu, 18 Jun 2020 00:21:22 +1000
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Peter Zijlstra <peterz@...radead.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>
Cc:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Will Deacon <will@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-mm@...ck.org
Subject: Re: [PATCH 3/3] powerpc/8xx: Provide ptep_get() with 16k pages

Peter Zijlstra <peterz@...radead.org> writes:
> On Mon, Jun 15, 2020 at 12:57:59PM +0000, Christophe Leroy wrote:
>> READ_ONCE() now enforces atomic read, which leads to:
>
>> Fixes: 2ab3a0a02905 ("READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses")
>> Cc: Will Deacon <will@...nel.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
>> ---
>>  arch/powerpc/include/asm/nohash/32/pgtable.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
>> index b56f14160ae5..77addb599ce7 100644
>> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
>> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
>> @@ -286,6 +286,16 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
>>  	return __pte(pte_update(mm, addr, ptep, ~0, 0, 0));
>>  }
>>  
>> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
>> +#define __HAVE_ARCH_PTEP_GET
>> +static inline pte_t ptep_get(pte_t *ptep)
>> +{
>> +	pte_t pte = {READ_ONCE(ptep->pte), 0, 0, 0};
>> +
>> +	return pte;
>> +}
>> +#endif
>
> Would it make sense to have a comment with this magic? The casual reader
> might wonder WTH just happened when he stumbles on this :-)

I tried writing a helpful comment but it's too late for my brain to form
sensible sentences.

Christophe can you send a follow-up with a comment explaining it? In
particular the zero entries stand out, it's kind of subtle that those
entries are only populated with the right value when we write to the
page table.

cheers