Re: [PATCH] prctl.2: Add PR_RISCV_SET_ICACHE_FLUSH_CTX

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6937 bytes --]

Hi Charlie,

On Mon, Feb 12, 2024 at 08:36:56PM -0500, Charlie Jenkins wrote:
> On Sun, Feb 11, 2024 at 07:38:39PM +0100, Alejandro Colomar wrote:
> > Hi Charlie,
> > 
> > On Wed, Jan 24, 2024 at 06:59:04PM -0800, Charlie Jenkins wrote:
> > > I have proposed and documented the PR_RISCV_SET_ICACHE_FLUSH_CTX flag
> > > for prctl(2) to LKML. It has been reviewed and is expected to land
> > > during the Linux version 6.9 merge window. This adds the relevant
> > > documentation from that patch.
> > > 
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > 
> > This patch triggers a few warnings:
> > 
> > 	$ make lint check -k
> > 	TROFF	.tmp/man/man2/prctl.2.cat.set
> > 	an.tmac:man2/prctl.2:1155: style: .IR expects at least 2 arguments, got 1
> > 	an.tmac:man2/prctl.2:1157: style: .IR expects at least 2 arguments, got 1
> > 	an.tmac:man2/prctl.2:1179: style: .IR expects at least 2 arguments, got 1
> > 	an.tmac:man2/prctl.2:1196: style: .IR expects at least 2 arguments, got 1
> > 	make: *** [share/mk/build/catman.mk:54: .tmp/man/man2/prctl.2.cat.set] Error 1
> > 	make: *** Deleting file '.tmp/man/man2/prctl.2.cat.set'
> > 	make: Target 'check' not remade because of errors.
> > 
> > See <CONTRIBUTING.d/lint> for that.
> 
> These lint errors didn't show up when I ran it, I must have some bad
> configuration.

Hmm, maybe you have an old version of groff(1).  Don't worry.  :)
BTW, thanks for running them!

Have a lovely day!
Alex

> 
> > 
> > > ---
> > > I have proposed and documented the PR_RISCV_SET_ICACHE_FLUSH_CTX
> > > flag for prctl(2) to LKML. It has been reviewed and is expected to land
> > > during the Linux version 6.9 merge window.
> > > ---
> > >  man2/prctl.2 | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 58 insertions(+)
> > > 
> > > diff --git a/man2/prctl.2 b/man2/prctl.2
> > > index f1604a7cb..2889a7195 100644
> > > --- a/man2/prctl.2
> > > +++ b/man2/prctl.2
> > > @@ -1147,6 +1147,64 @@ For further information, see the kernel source file
> > >  (or
> > >  .I Documentation/security/Yama.txt
> > >  before Linux 4.13).
> > > +.\" prctl PR_RISCV_SET_ICACHE_FLUSH_CTX
> > > +.TP
> > > +.BR PR_RISCV_SET_ICACHE_FLUSH_CTX " (since Linux 6.9, RISC-V only)"
> > > +Enable/disable icache flushing instructions in userspace. The ctx and the
> > 
> > 'ctx' isn't a common word.  The first time you mention it you should
> > probably say what it means.  For example:  "The context (ctx) and
> > the ..." and later just use ctx.
> > 
> > > +scope can be provided using
> > > +.IR arg2
> > 
> > This should be I, not IR.
> > 
> > > +and
> > > +.IR arg3
> > > +respectively. When scope is set to
> > 
> > Please use semantic newlines.  See man-pages(7):
> > 
> >    Use semantic newlines
> >      In  the  source of a manual page, new sentences should be started
> >      on new lines, long sentences should be split into lines at clause
> >      breaks (commas, semicolons, colons, and so on), and long  clauses
> >      should be split at phrase boundaries.  This convention, sometimes
> >      known  as  "semantic newlines", makes it easier to see the effect
> >      of patches, which often operate at the level of  individual  sen‐
> >      tences, clauses, or phrases.
> > 
> > > +.B PR_RISCV_SCOPE_PER_PROCESS ,
> > 
> > This should be BR, not B.  See groff_man(7).
> > 
> > > +all threads in the process are permitted to emit icache flushing
> > > +instructions. Whenever any thread in the process is migrated, the
> > > +corresponding hart's icache will be guaranteed to be consistent with
> > > +instruction storage. Note this does not enforce any guarantees outside of
> > 
> > 'Note' is superfluous.  You can just remove that word.  "This does not
> > enforce ...".
> > 
> > > +migration. If a thread modifies an instruction that another thread may
> > > +attempt to execute, the other thread must still emit an icache flushing
> > > +instruction before attempting to execute the potentially modified
> > > +instruction. This must be performed by the userspace program.
> > 
> > We say "user space"; two words (see man-pages(7)).  In this case, since
> > it's an adjective, it would be "user-space", with a hyphen.
> > 
> > > +.IP
> > > +In per-thread context (eg. scope is set to
> > > +.B PR_RISCV_SCOPE_PER_THREAD )
> > 
> > The comma should go with the parenthesis.  Otherwise, you'll see a
> > spurious space in between.
> > 
> > > +, only the thread calling this function is permitted to emit icache
> > > +flushing instructions. When the thread is migrated, the corresponding
> > > +hart's icache will be guaranteed to be consistent with instruction storage.
> > > +.IP
> > > +On kernels configured without SMP, this function is a nop as migrations
> > > +across harts will not occur.
> > > +.IP
> > > +The following values for
> > > +.IR arg2
> > > +can be specified:
> > > +.RS
> > > +.TP
> > > +.BR PR_RISCV_CTX_SW_FENCEI_ON " (since Linux 6.9)"
> > > +Allow fence.i in userspace.
> > 
> > "user space".
> > 
> > > +.TP
> > > +.BR PR_RISCV_CTX_SW_FENCEI_OFF " (since Linux 6.9)"
> > > +Disallow fence.i in userspace. All threads in a process will be affected
> > 
> > .
> > 
> > > +when scope is set to
> > > +.B PR_RISCV_SCOPE_PER_PROCESS .
> > > +Therefore, caution must be taken -- only use this flag when you can
> > 
> > Use \[em] for an em dash.  Although in this case I think ':' or ';'
> > would be more appropriate.
> > 
> > Also, I think 'only' would go better right before "when".
> > 
> > > +guarantee that no thread in the process will emit fence.i from this point
> > > +onward.
> > > +.RE
> > > +.IP
> > > +The following values for
> > > +.IR arg3
> > > +can be specified:
> > > +.RS
> > > +.TP
> > > +.BR PR_RISCV_SCOPE_PER_PROCESS " (since Linux 6.9)"
> > > +Ensure the icache of any thread in this process is coherent with
> > > +instruction storage upon migration.
> > > +.TP
> > > +.BR PR_RISCV_SCOPE_PER_THREAD " (since Linux 6.9)"
> > > +Ensure the icache of the current thread is coherent with instruction
> > > +storage upon migration.
> > > +.RE
> > >  .\" prctl PR_SET_SECCOMP
> > >  .TP
> > >  .BR PR_SET_SECCOMP " (since Linux 2.6.23)"
> > > 
> > > ---
> > > base-commit: a81e893f2b9316869e6098c3a079c30a48158092
> > > change-id: 20240124-fencei_prctl-c24da2643379
> > > 
> > > Best regards,
> > > -- 
> > > Charlie Jenkins <charlie@rivosinc.com>
> > 
> > Thanks for the patch!
> > 
> > Have a lovely day,
> > Alex
> > 
> > -- 
> > <https://www.alejandro-colomar.es/>
> > Looking for a remote C programming job at the moment.
> 
> Thank you for all of the suggestions! I will make the requested changes.
> 
> - Charlie
> 

-- 
<https://www.alejandro-colomar.es/>
Looking for a remote C programming job at the moment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]