On Mon, Jan 31, 2022 at 02:52:47PM -0800, Nick Desaulniers wrote: > Add text to memory-barriers.txt and deprecated.rst to denote that > volatile-qualifying an asm statement is not a substitute for either a > compiler barrier (``barrier();``) or a clobber list. > > This way we can point to this in code that strengthens existing > volatile-qualified asm statements to use a compiler barrier. > > Suggested-by: Kees Cook <keescook@xxxxxxxxxxxx> > Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > --- > Example: https://godbolt.org/z/8PW549zz9 > > Documentation/memory-barriers.txt | 24 ++++++++++++++++++++++++ > Documentation/process/deprecated.rst | 17 +++++++++++++++++ > 2 files changed, 41 insertions(+) > > diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt > index b12df9137e1c..f3908c0812da 100644 > --- a/Documentation/memory-barriers.txt > +++ b/Documentation/memory-barriers.txt > @@ -1726,6 +1726,30 @@ of optimizations: > respect the order in which the READ_ONCE()s and WRITE_ONCE()s occur, > though the CPU of course need not do so. > > + (*) Similarly, the compiler is within its rights to reorder instructions > + around an asm statement so long as clobbers are not violated. For example, > + > + asm volatile (""); > + flag = true; > + > + May be modified by the compiler to: > + > + flag = true; > + asm volatile (""); > + > + Marking an asm statement as volatile is not a substitute for barrier(), > + and is implicit for asm goto statements and asm statements that do not > + have outputs (like the above example). Prefer either: > + > + asm ("":::"memory"); > + flag = true; > + > + Or: > + > + asm (""); > + barrier(); > + flag = true; > + I like this! > (*) The compiler is within its rights to invent stores to a variable, > as in the following example: > > diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst > index 388cb19f5dbb..432816e2f79e 100644 > --- a/Documentation/process/deprecated.rst > +++ b/Documentation/process/deprecated.rst > @@ -329,3 +329,20 @@ struct_size() and flex_array_size() helpers:: > instance->count = count; > > memcpy(instance->items, source, flex_array_size(instance, items, instance->count)); > + > +Volatile Qualified asm Statements > +================================= I would open with an example, like: Instead of:: volatile asm("..."); just use:: asm("..."); > + > +According to `the GCC docs on inline asm > +https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile`_: > + > + asm statements that have no output operands and asm goto statements, > + are implicitly volatile. Does this mean "volatile" _is_ needed when there are operands, etc? > + > +For many uses of asm statements, that means adding a volatile qualifier won't > +hurt (making the implicit explicit), but it will not strengthen the semantics > +for such cases where it would have been implied. Care should be taken not to > +confuse ``volatile`` with the kernel's ``barrier()`` macro or an explicit > +clobber list. See [memory-barriers]_ for more info on ``barrier()``. > + > +.. [memory-barriers] Documentation/memory-barriers.txt > -- > 2.35.0.rc2.247.g8bbb082509-goog > -- Kees Cook
