| Message ID | 151520099810.32271.11023910901471332353.stgit@dwillia2-desk3.amr.corp.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/18] asm-generic/barrier: add generic nospec helpers | expand |
On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams <dan.j.williams@intel.com> wrote: > +#ifndef nospec_ptr > +#define nospec_ptr(ptr, lo, hi) \ Do we actually want this horrible interface? It just causes the compiler - or inline asm - to generate worse code, because it needs to compare against both high and low limits. Basically all users are arrays that are zero-based, and where a comparison against the high _index_ limit would be sufficient. But the way this is all designed, it's literally designed for bad code generation for the unusual case, and the usual array case is written in the form of the unusual and wrong non-array case. That really seems excessively stupid. Linus
On Fri, Jan 5, 2018 at 6:55 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams <dan.j.williams@intel.com> wrote: >> +#ifndef nospec_ptr >> +#define nospec_ptr(ptr, lo, hi) \ > > Do we actually want this horrible interface? > > It just causes the compiler - or inline asm - to generate worse code, > because it needs to compare against both high and low limits. > > Basically all users are arrays that are zero-based, and where a > comparison against the high _index_ limit would be sufficient. > > But the way this is all designed, it's literally designed for bad code > generation for the unusual case, and the usual array case is written > in the form of the unusual and wrong non-array case. That really seems > excessively stupid. Yes, it appears we can kill nospec_ptr() and move nospec_array_ptr() to assume 0 based arrays rather than use nospec_ptr.
On Fri, Jan 05, 2018 at 09:23:06PM -0800, Dan Williams wrote: > On Fri, Jan 5, 2018 at 6:55 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > On Fri, Jan 5, 2018 at 5:09 PM, Dan Williams <dan.j.williams@intel.com> wrote: > >> +#ifndef nospec_ptr > >> +#define nospec_ptr(ptr, lo, hi) \ > > > > Do we actually want this horrible interface? > > > > It just causes the compiler - or inline asm - to generate worse code, > > because it needs to compare against both high and low limits. > > > > Basically all users are arrays that are zero-based, and where a > > comparison against the high _index_ limit would be sufficient. > > > > But the way this is all designed, it's literally designed for bad code > > generation for the unusual case, and the usual array case is written > > in the form of the unusual and wrong non-array case. That really seems > > excessively stupid. > > Yes, it appears we can kill nospec_ptr() and move nospec_array_ptr() > to assume 0 based arrays rather than use nospec_ptr. Sounds good to me; I can respin the arm/arm64 implementations accordingly. We can always revisit that if we have non-array cases that need to be covered. Thanks, Mark.
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h index fe297b599b0a..91c3071f49e5 100644 --- a/include/asm-generic/barrier.h +++ b/include/asm-generic/barrier.h @@ -54,6 +54,74 @@ #define read_barrier_depends() do { } while (0) #endif +/* + * Inhibit subsequent speculative memory accesses. + * + * Architectures with a suitable memory barrier should provide an + * implementation. This is non-portable, and generic code should use + * nospec_ptr(). + */ +#ifndef __nospec_barrier +#define __nospec_barrier() do { } while (0) +#endif + +/** + * nospec_ptr() - Ensure a pointer is bounded, even under speculation. + * + * @ptr: the pointer to test + * @lo: the lower valid bound for @ptr, inclusive + * @hi: the upper valid bound for @ptr, exclusive + * + * If @ptr falls in the interval [@lo, @i), returns @ptr, otherwise returns + * NULL. + * + * Architectures which do not provide __nospec_barrier() should override this + * to ensure that ptr falls in the [lo, hi) interval both under architectural + * execution and under speculation, preventing propagation of an out-of-bounds + * pointer to code which is speculatively executed. + */ +#ifndef nospec_ptr +#define nospec_ptr(ptr, lo, hi) \ +({ \ + typeof (ptr) __ret; \ + typeof (ptr) __ptr = (ptr); \ + typeof (ptr) __lo = (lo); \ + typeof (ptr) __hi = (hi); \ + \ + __ret = (__lo <= __ptr && __ptr < __hi) ? __ptr : NULL; \ + \ + __nospec_barrier(); \ + \ + __ret; \ +}) +#endif + +/** + * nospec_array_ptr - Generate a pointer to an array element, ensuring the + * pointer is bounded under speculation. + * + * @arr: the base of the array + * @idx: the index of the element + * @sz: the number of elements in the array + * + * If @idx falls in the interval [0, @sz), returns the pointer to @arr[@idx], + * otherwise returns NULL. + * + * This is a wrapper around nospec_ptr(), provided for convenience. + * Architectures should implement nospec_ptr() to ensure this is the case + * under speculation. + */ +#define nospec_array_ptr(arr, idx, sz) \ +({ \ + typeof(*(arr)) *__arr = (arr); \ + typeof(idx) __idx = (idx); \ + typeof(sz) __sz = (sz); \ + \ + nospec_ptr(__arr + __idx, __arr, __arr + __sz); \ +}) + +#undef __nospec_barrier + #ifndef __smp_mb #define __smp_mb() mb() #endif