On Tue, Feb 1, 2022 at 1:32 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > On Mon, 31 Jan 2022 at 23:53, Nick Desaulniers <ndesaulniers@xxxxxxxxxx> 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 > > Similar to what? Was this intended to be the second bullet point > rather than the first? Similar to the previous bullet point. This isn't the first use of `Similarly, ` in this document. > > > + 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 would expect the memory clobber to only hazard against the > assignment of flag if it results in a store, but looking at your > Godbolt example, this appears to apply even if flag is kept in a > register. > > Is that behavior documented/codified anywhere? Or are we relying on > compiler implementation details here? https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile "Note that the compiler can move even volatile asm instructions relative to other code, including across jump instructions." > > > > (*) 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 > > +================================= > > + > > +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. > > + > > +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 > > -- Thanks, ~Nick Desaulniers
