Re: [PATCH v3] prctl.2: Add PR_RISCV_SET_ICACHE_FLUSH_CTX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Charlie,

On Fri, Jun 28, 2024 at 07:36:18PM GMT, Charlie Jenkins wrote:
> Document the PR_RISCV_SET_ICACHE_FLUSH_CTX flag for prctl(2) that is
> supported as of Linux 6.10.
> 
> Signed-off-by: Charlie Jenkins <charlie@xxxxxxxxxxxx>
> ---
> Changes in v3:
> - Rebase onto master
> - Add example usage
> - Link to v2: https://lore.kernel.org/r/20240212-fencei_prctl-v2-1-32d940981b05@xxxxxxxxxxxx

Thanks!

> 
> Changes in v2:
> - Update formatting (Alejandro)
> - Link to v1: https://lore.kernel.org/r/20240124-fencei_prctl-v1-1-0bddafcef331@xxxxxxxxxxxx
> ---
>  man/man2/prctl.2                                   |   3 +
>  man/man2const/PR_RISCV_SET_ICACHE_FLUSH_CTX.2const | 149 +++++++++++++++++++++
>  2 files changed, 152 insertions(+)
> 
> diff --git a/man/man2/prctl.2 b/man/man2/prctl.2
> index 6db916587..31a3f9064 100644
> --- a/man/man2/prctl.2
> +++ b/man/man2/prctl.2
> @@ -157,6 +157,8 @@ The first argument can be:
>  .B PR_SET_MDWE
>  .TQ
>  .B PR_GET_MDWE
> +.TQ
> +.B PR_RISCV_SET_ICACHE_FLUSH_CTX
>  .SH RETURN VALUE
>  On success,
>  a nonnegative value is returned.
> @@ -268,4 +270,5 @@ so these operations should be used with care.
>  .BR PR_GET_AUXV (2const),
>  .BR PR_SET_MDWE (2const),
>  .BR PR_GET_MDWE (2const),
> +.BR PR_RISCV_SET_ICACHE_FLUSH_CTX (2const),
>  .BR core (5)
> diff --git a/man/man2const/PR_RISCV_SET_ICACHE_FLUSH_CTX.2const b/man/man2const/PR_RISCV_SET_ICACHE_FLUSH_CTX.2const
> new file mode 100644
> index 000000000..aec16a237
> --- /dev/null
> +++ b/man/man2const/PR_RISCV_SET_ICACHE_FLUSH_CTX.2const
> @@ -0,0 +1,149 @@
> +.\" Copyright 2024 Rivos Inc.
> +.\"
> +.\" SPDX-License-Identifier: Linux-man-pages-copyleft
> +.\"
> +.TH PR_RISCV_SET_ICACHE_FLUSH_CTX 2const (date) "Linux man-pages (unreleased)"
> +.SH NAME
> +PR_RISCV_SET_ICACHE_FLUSH_CTX (since Linux 6.10, RISC-V only)

Let's move that info to STANDARDS and HISTORY.  See for example
PR_SET_FP_MODE(2const):

NAME
     PR_SET_FP_MODE - set the floating point mode of the calling process

...

STANDARDS
     Linux.  MIPS only.

HISTORY
     Linux 4.0 (MIPS).

> +\-
> +Enable/disable icache flushing instructions in userspace.
> +.SH LIBRARY
> +Standard C library
> +.RI ( libc ", " \-lc )
> +.SH SYNOPSIS
> +.nf
> +.BR "#include <linux/prctl.h>" "  /* Definition of " PR_* " constants */"
> +.B #include <sys/prctl.h>
> +.P
> +.B int prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX_ON, ctx, scope);

This should document the types of ctx and scope.

Also, it seems there's a typo:

PR_RISCV_SET_ICACHE_FLUSH_CTX_ON
	|
	v
PR_RISCV_SET_ICACHE_FLUSH_CTX

> +.fi
> +.SH DESCRIPTION
> +The context and the scope can be provided using
> +.I arg2
> +and
> +.I arg3

arg2/3 have a name now.

> +respectively.

You probably wanted to separate a paragraph here?  .P

> +When scope is set to
> +.B PR_RISCV_SCOPE_PER_PROCESS
> +all threads in the process are permitted to emit icache flushing instructions.
> +Whenever any thread in the process is migrated, the corresponding hart's

Please break lines after punctuation.  See man-pages(7):

$ MANWIDTH=72 man man-pages | sed -n '/Use semantic newlines/,/^$/p'
   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 sentences,
     clauses, or phrases.

> +icache will be guaranteed to be consistent with instruction storage.
> +This does not enforce any guarantees outside of 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 user-space program.
> +.P
> +In per-thread context (eg. scope is set to
> +.B PR_RISCV_SCOPE_PER_THREAD )
> +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.
> +.P
> +On kernels configured without SMP, this function is a nop as migrations across

"this function" is a bit ambiguous: does it refer to the
PR_RISCV_SCOPE_PER_THREAD, or PR_RISCV_SET_ICACHE_FLUSH_CTX?  Let's
mention the thing explicitly.

> +harts will not occur.
> +.P
> +The following values for
> +.I arg2

.I ctx

> +can be specified:
> +.RS
> +.TP
> +.BR PR_RISCV_CTX_SW_FENCEI_ON " (since Linux 6.10)"
> +Allow fence.i in user space.
> +.TP
> +.BR PR_RISCV_CTX_SW_FENCEI_OFF " (since Linux 6.10)"
> +Disallow fence.i in user space.
> +All threads in a process will be affected when scope is set to
> +.BR PR_RISCV_SCOPE_PER_PROCESS .
> +Therefore, caution must be taken; use this flag only when you can guarantee
> +that no thread in the process will emit fence.i from this point onward.
> +.RE
> +.IP
> +The following values for
> +.I arg3

.I scope

> +can be specified:
> +.RS
> +.TP
> +.BR PR_RISCV_SCOPE_PER_PROCESS " (since Linux 6.10)"
> +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.10)"
> +Ensure the icache of the current thread is coherent with instruction storage
> +upon migration.
> +.RE
> +

Don't use blank lines in source.  If you want to have some no-ops in the
source, use empty requests:

.

> +.SH EXAMPLE

The section should be called EXAMPLES.  See man-pages(7):

$ man man-pages | grep EXAMPLE
            EXAMPLES
     EXAMPLES
EXAMPLES

> +
> +The following files are meant to be compiled and linked with each other. The
> +modify_instruction() function replaces an add with zero with an add with one,
> +causing the instruction sequence in get_value() to change from returning a zero
> +to returning a one.
> +
> +.SS Program source: cmodx.c
> +\&

Please enclose the program source in some magic comments, which you can
find for example in the source code of membarrier (or any page that has
example programs).  They allow the build system to extract the program,
pass several linters, and compile it, to try and find bugs.

Here would go:

.\" SRC BEGIN (cmodx.c)

> +.EX
> +#include <stdio.h>
> +#include <sys/prctl.h>
> +\&
> +extern int get_value();
> +extern void modify_instruction();
> +\&
> +int main()
> +{
> +    int value = get_value();
> +    printf("Value before cmodx: %d\\n", value);

The escape character is written as \[rs]

> +
> +    // Call prctl before first fence.i is called
> +    prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, PR_RISCV_CTX_SW_FENCEI_ON,
> +          PR_RISCV_SCOPE_PER_PROCESS);
> +    modify_instruction();
> +    // Call prctl after final fence.i is called in process
> +    prctl(PR_RISCV_SET_ICACHE_FLUSH_CTX, PR_RISCV_CTX_SW_FENCEI_OFF,
> +          PR_RISCV_SCOPE_PER_PROCESS);
> +
> +    value = get_value();
> +    printf("Value after cmodx: %d\\n", value);
> +    return 0;
> +}

You forgot to terminate the EX region with

.EE

And let's also close the magic comment region with

.\" SRC END

Have a lovely day!
Alex

> +.SS Program source: cmodx.S
> +.EX
> +\&.option norvc
> +
> +\&.text
> +\&.global modify_instruction
> +modify_instruction:
> +lw a0, new_insn
> +lui a5,%hi(old_insn)
> +sw  a0,%lo(old_insn)(a5)
> +fence.i
> +ret
> +
> +\&.section modifiable, "awx"
> +\&.global get_value
> +get_value:
> +li a0, 0
> +old_insn:
> +addi a0, a0, 0
> +ret
> +
> +\&.data
> +new_insn:
> +addi a0, a0, 1
> +.EE
> +
> +.SS Expected result
> +.EX
> +Value before cmodx: 0
> +Value after cmodx: 1
> +.EE
> +
> +.SH STANDARDS
> +Linux.
> +.SH HISTORY
> +.TP
> +.B PR_RISCV_SCOPE_PER_PROCESS
> +Linux 6.10.
> +.SH SEE ALSO
> +.BR prctl (2)
> 
> ---
> base-commit: d0621648b4b5a356e86cea23e842f2591461f0cf
> change-id: 20240124-fencei_prctl-c24da2643379
> 
> Best regards,
> -- 
> Charlie Jenkins <charlie@xxxxxxxxxxxx>
> 
> 

-- 
<https://www.alejandro-colomar.es/>

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux