| Message ID | 20180113103816.4861-5-palmer@dabbelt.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,01/17] Skeleton documentation for the RISC-V port | expand |
On Sat, 13 Jan 2018, Palmer Dabbelt wrote: > +@node RISC-V > +@appendixsec RISC-V-specific Facilities > + > +Cache management facilities specific to RISC-V systems that implement the Linux > +ABI are declared in @file{sysdeps/unix/sysv/linux/riscv/sys/cachectl.h}. This manual is for *users*, which means it should reference the installed header sys/cachectl.h, not the location within the glibc source tree. > + > +@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long int flags) That's @var{start}, @var{end}, @var{flags} in the @deftypefun line. > +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} > +Enforce ordering between stores and instruction cache fetches. The flags And then again @var{flags} (an argument, not a variable) in the description of the function's semantics. > +variable is used to determine if this applies just to the local thread or to > +all threads in the system. > +@end deftypefun "used to determine" how? You need to say explicitly what the semantics of all defined values of that argument are. You also need to say what the semantics of the other arguments are; generally I'd expect documentation for a function to refer explicitly to each argument, @var{start}, @var{end} and @var{flags}, in defining the function semantics in terms of those arguments.
On Mon, 15 Jan 2018 08:49:05 PST (-0800), joseph@codesourcery.com wrote: > On Sat, 13 Jan 2018, Palmer Dabbelt wrote: > >> +@node RISC-V >> +@appendixsec RISC-V-specific Facilities >> + >> +Cache management facilities specific to RISC-V systems that implement the Linux >> +ABI are declared in @file{sysdeps/unix/sysv/linux/riscv/sys/cachectl.h}. > > This manual is for *users*, which means it should reference the installed > header sys/cachectl.h, not the location within the glibc source tree. > >> + >> +@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long int flags) > > That's @var{start}, @var{end}, @var{flags} in the @deftypefun line. > >> +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} >> +Enforce ordering between stores and instruction cache fetches. The flags > > And then again @var{flags} (an argument, not a variable) in the > description of the function's semantics. > >> +variable is used to determine if this applies just to the local thread or to >> +all threads in the system. >> +@end deftypefun > > "used to determine" how? You need to say explicitly what the semantics of > all defined values of that argument are. You also need to say what the > semantics of the other arguments are; generally I'd expect documentation > for a function to refer explicitly to each argument, @var{start}, > @var{end} and @var{flags}, in defining the function semantics in terms of > those arguments. Sorry about that, that documentation was terrible. Hopefully this is a bit better. diff --git a/manual/platform.texi b/manual/platform.texi index f6ca97ebf9d8..6caa988a02c2 100644 --- a/manual/platform.texi +++ b/manual/platform.texi @@ -121,11 +121,15 @@ when it is not allowed, the priority is set to medium. @appendixsec RISC-V-specific Facilities Cache management facilities specific to RISC-V systems that implement the Linux -ABI are declared in @file{sysdeps/unix/sysv/linux/riscv/sys/cachectl.h}. +ABI are declared in @file{sys/cachectl.h}. -@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long int flags) +@deftypefun {void} __riscv_flush_icache(void *@var{__start}, void *@var{__end}, + unsigned long int @var{__flags}) @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} -Enforce ordering between stores and instruction cache fetches. The flags -variable is used to determine if this applies just to the local thread or to -all threads in the system. +Enforce ordering between stores and instruction cache fetches. The range of +addresses over which ordering is enforced is specified by @var{__start} and +@var{__end}. The @var{__flags} variable controls how the ordering is enforced, +with @code{SYS_RISCV_FLUSH_ICACHE_LOCAL} specifying that the ordering is +enforced only with respect to the local instruction stream (as opposed to all +instruction streams). @end deftypefun
On Mon, 22 Jan 2018, Palmer Dabbelt wrote: > -@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long > int flags) > +@deftypefun {void} __riscv_flush_icache(void *@var{__start}, void > *@var{__end}, > + unsigned long int @var{__flags}) No __ on parameter names in the manual, only in the installed header. And you can't split the @deftypefun line like that. (Texinfo supports a syntax with @ at end of line when such an @def* line is split, but I don't think glibc's own scripts processing .texi files handle that.) > @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} > -Enforce ordering between stores and instruction cache fetches. The flags > -variable is used to determine if this applies just to the local thread or to > -all threads in the system. > +Enforce ordering between stores and instruction cache fetches. The range of > +addresses over which ordering is enforced is specified by @var{__start} and > +@var{__end}. The @var{__flags} variable controls how the ordering is > enforced, As I said before I think this should be called an argument not a variable. > +with @code{SYS_RISCV_FLUSH_ICACHE_LOCAL} specifying that the ordering is > +enforced only with respect to the local instruction stream (as opposed to all > +instruction streams). And what other values are valid and what do they mean? Is 0 the only other alternative value, meaning ordering over all cores? Are other values reserved for future use? You seem to describe only one value, SYS_RISCV_FLUSH_ICACHE_LOCAL, without saying anything about any other values such as 0.
On Mon, 22 Jan 2018 13:58:00 PST (-0800), joseph@codesourcery.com wrote: > On Mon, 22 Jan 2018, Palmer Dabbelt wrote: > >> -@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long >> int flags) >> +@deftypefun {void} __riscv_flush_icache(void *@var{__start}, void >> *@var{__end}, >> + unsigned long int @var{__flags}) > > No __ on parameter names in the manual, only in the installed header. And > you can't split the @deftypefun line like that. (Texinfo supports a > syntax with @ at end of line when such an @def* line is split, but I don't > think glibc's own scripts processing .texi files handle that.) Thanks. >> @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} >> -Enforce ordering between stores and instruction cache fetches. The flags >> -variable is used to determine if this applies just to the local thread or to >> -all threads in the system. >> +Enforce ordering between stores and instruction cache fetches. The range of >> +addresses over which ordering is enforced is specified by @var{__start} and >> +@var{__end}. The @var{__flags} variable controls how the ordering is >> enforced, > > As I said before I think this should be called an argument not a variable. Sorry, I missed that. >> +with @code{SYS_RISCV_FLUSH_ICACHE_LOCAL} specifying that the ordering is >> +enforced only with respect to the local instruction stream (as opposed to all >> +instruction streams). > > And what other values are valid and what do they mean? Is 0 the only > other alternative value, meaning ordering over all cores? Are other > values reserved for future use? You seem to describe only one value, > SYS_RISCV_FLUSH_ICACHE_LOCAL, without saying anything about any other > values such as 0. How does this look? @node RISC-V @appendixsec RISC-V-specific Facilities Cache management facilities specific to RISC-V systems that implement the Linux ABI are declared in @file{sys/cachectl.h}. @deftypefun {void} __riscv_flush_icache(void *@var{start}, void *@var{end}, unsigned long int @var{flags}) @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} Enforce ordering between stores and instruction cache fetches. The range of addresses over which ordering is enforced is specified by @var{start} and @var{end}. The @var{flags} argument controls the extent of this ordering, with the default behavior (a @var{flags} value of 0) being to enforce the fence on all threads in the current process. Setting the @code{SYS_RISCV_FLUSH_ICACHE_LOCAL} bit allows users to indicate that enforcing ordering on only the current thread is necessary. All other flag bits are reserved.
On Mon, 22 Jan 2018, Palmer Dabbelt wrote:
> How does this look?
Seems reasonable.
diff --git a/manual/platform.texi b/manual/platform.texi index cb166641fb71..f6ca97ebf9d8 100644 --- a/manual/platform.texi +++ b/manual/platform.texi @@ -6,6 +6,7 @@ @menu * PowerPC:: Facilities Specific to the PowerPC Architecture +* RISC-V:: Facilities Specific to the RISC-V Architecture @end menu @node PowerPC @@ -115,3 +116,16 @@ problem-state programs. If the program priority is medium high when the time interval expires or if an attempt is made to set the priority to medium high when it is not allowed, the priority is set to medium. @end deftypefun + +@node RISC-V +@appendixsec RISC-V-specific Facilities + +Cache management facilities specific to RISC-V systems that implement the Linux +ABI are declared in @file{sysdeps/unix/sysv/linux/riscv/sys/cachectl.h}. + +@deftypefun {void} __riscv_flush_icache(void *start, void *end, unsigned long int flags) +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}} +Enforce ordering between stores and instruction cache fetches. The flags +variable is used to determine if this applies just to the local thread or to +all threads in the system. +@end deftypefun
I don't know if this is actually the right place to put this: it's a Linux-specific function, and while users can call it we'd really prefer they use rely on GCC to emit it when necessary from via its cache flushing intrinsics. 2018-01-13 Palmer Dabbelt <palmer@sifive.com> * manual/platform.texi: Add RISC-V documenation for __riscv_flush_icache. --- manual/platform.texi | 14 ++++++++++++++ 1 file changed, 14 insertions(+)