linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Gabriel Paubert <paubert@iram.es>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Madhavan Srinivasan <maddy@linux.ibm.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Darren Hart <dvhart@infradead.org>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Andre Almeida <andrealmeid@igalia.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Laight <david.laight.linux@gmail.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-block@vger.kernel.org
Subject: Re: [PATCH v2 10/10] powerpc/uaccess: Implement masked user access
Date: Mon, 25 Aug 2025 11:40:48 +0200	[thread overview]
Message-ID: <16679d56-5ee0-469d-a11c-475a45a1c2b9@csgroup.eu> (raw)
In-Reply-To: <aKwnMo7UllLZkOcK@lt-gp.iram.es>

Hi Gabriel,

Le 25/08/2025 à 11:04, Gabriel Paubert a écrit :
> [Vous ne recevez pas souvent de courriers de paubert@iram.es. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hi Christophe,
> 
> On Fri, Aug 22, 2025 at 11:58:06AM +0200, Christophe Leroy wrote:
>> Masked user access avoids the address/size verification by access_ok().
>> Allthough its main purpose is to skip the speculation in the
>> verification of user address and size hence avoid the need of spec
>> mitigation, it also has the advantage of reducing the amount of
>> instructions required so it even benefits to platforms that don't
>> need speculation mitigation, especially when the size of the copy is
>> not know at build time.
>>
>> So implement masked user access on powerpc. The only requirement is
>> to have memory gap that faults between the top user space and the
>> real start of kernel area.
>>
>> On 64 bits platforms the address space is divided that way:
>>
>>        0xffffffffffffffff      +------------------+
>>                                |                  |
>>                                |   kernel space   |
>>                                |                  |
>>        0xc000000000000000      +------------------+  <== PAGE_OFFSET
>>                                |//////////////////|
>>                                |//////////////////|
>>        0x8000000000000000      |//////////////////|
>>                                |//////////////////|
>>                                |//////////////////|
>>        0x0010000000000000      +------------------+  <== TASK_SIZE_MAX
>>                                |                  |
>>                                |    user space    |
>>                                |                  |
>>        0x0000000000000000      +------------------+
>>
>> Kernel is always above 0x8000000000000000 and user always
>> below, with a gap in-between. It leads to a 4 instructions sequence:
>>
>>    80: 7c 69 1b 78     mr      r9,r3
>>    84: 7c 63 fe 76     sradi   r3,r3,63
>>    88: 7d 29 18 78     andc    r9,r9,r3
>>    8c: 79 23 00 4c     rldimi  r3,r9,0,1
>>
>> This sequence leaves r3 unmodified when it is below 0x8000000000000000
>> and clamps it to 0x8000000000000000 if it is above.
>>
> 
> This comment looks wrong: the second instruction converts r3 to a
> replicated sign bit of the address ((addr>0)?0:-1) if treating the
> address as signed. After that the code only modifies the MSB of r3. So I
> don't see how r3 could be unchanged from the original value...

Unless I'm missing something, the above rldimi leaves the MSB of r3 
unmodified and replaces all other bits by the same in r9.

This is the code generated by GCC for the following:

	unsigned long mask = (unsigned long)((long)addr >> 63);

	addr = ((addr & ~mask) & (~0UL >> 1)) | (mask & (1UL << 63));


> 
> OTOH, I believe the following 3 instructions sequence would work,
> input address (a) in r3, scratch value (tmp) in r9, both intptr_t:
> 
>          sradi r9,r3,63  ; tmp = (a >= 0) ? 0L : -1L;
>          andc r3,r3,r9   ; a = a & ~tmp; (equivalently a = (a >= 0) ? a : 0)
>          rldimi r3,r9,0,1 ; copy MSB of tmp to MSB of a
> 
> But maybe I goofed...
> 

 From my understanding of rldimi, your proposed code would:
- Keep r3 unmodified when it is above 0x8000000000000000
- Set r3 to 0x7fffffffffffffff when it is below 0x8000000000000000

Extract of ppc64 ABI:

rldimi RA,RS,SH,MB

The contents of register RS are rotated 64 left SH bits.
A mask is generated having 1-bits from bit MB
through bit 63− SH and 0-bits elsewhere. The rotated
data are inserted into register RA under control of the
generated mask.




  reply	other threads:[~2025-08-25  9:40 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22  9:57 [PATCH v2 00/10] powerpc: " Christophe Leroy
2025-08-22  9:57 ` [PATCH v2 01/10] iter: Avoid barrier_nospec() in copy_from_user_iter() Christophe Leroy
2025-08-22  9:57 ` [PATCH v2 02/10] uaccess: Add speculation barrier to copy_from_user_iter() Christophe Leroy
2025-08-22 13:46   ` Linus Torvalds
2025-08-22 14:11     ` Giorgi Tchankvetadze
2025-08-22 18:53     ` David Laight
2025-08-22  9:57 ` [PATCH v2 03/10] uaccess: Add masked_user_{read/write}_access_begin Christophe Leroy
2025-08-24 15:08   ` Thomas Gleixner
2025-08-22  9:58 ` [PATCH v2 04/10] powerpc/uaccess: Move barrier_nospec() out of allow_read_{from/write}_user() Christophe Leroy
2025-08-22  9:58 ` [PATCH v2 05/10] powerpc/uaccess: Remove unused size and from parameters from allow_access_user() Christophe Leroy
2025-08-22  9:58 ` [PATCH v2 06/10] powerpc/uaccess: Remove {allow/prevent}_{read/write/read_write}_{from/to/}_user() Christophe Leroy
2025-08-22  9:58 ` [PATCH v2 07/10] powerpc/uaccess: Refactor user_{read/write/}_access_begin() Christophe Leroy
2025-08-22  9:58 ` [PATCH v2 08/10] powerpc/32s: Fix segments setup when TASK_SIZE is not a multiple of 256M Christophe Leroy
2025-08-22  9:58 ` [PATCH v2 09/10] powerpc/32: Automatically adapt TASK_SIZE based on constraints Christophe Leroy
2025-08-22 12:04   ` David Laight
2025-08-22  9:58 ` [PATCH v2 10/10] powerpc/uaccess: Implement masked user access Christophe Leroy
2025-08-25  9:04   ` Gabriel Paubert
2025-08-25  9:40     ` Christophe Leroy [this message]
2025-08-25 10:18       ` Gabriel Paubert

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=16679d56-5ee0-469d-a11c-475a45a1c2b9@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=akpm@linux-foundation.org \
    --cc=andrealmeid@igalia.com \
    --cc=brauner@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@stgolabs.net \
    --cc=david.laight.linux@gmail.com \
    --cc=dvhart@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=paubert@iram.es \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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