lkml.org 
[lkml]   [2022]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] arm64: kcsan: Fix kcsan test_barrier fail and panic
    Hi,

    On Tue, Apr 26, 2022 at 08:17:00AM +0000, Kefeng Wang wrote:
    > As "kcsan: Support detecting a subset of missing memory barriers"
    > introduced KCSAN_STRICT which make kcsan detects more missing memory
    > barrier, but arm64 don't have KCSAN instrumentation for barriers, so
    > the new selftest test_barrier() will fail, then panic.
    >
    > Let's prefix all barriers with __ on arm64, as asm-generic/barriers.h
    > defined the final instrumented version of these barriers, which will
    > fix the above issues.
    >
    > Fixes: dd03762ab608 ("arm64: Enable KCSAN")
    > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

    I don't think the Fixes tag is necessary given this is a new feature
    which depends upon EXPERT, and I'm worried it encourages backporting
    this into a kernel where it would be broken, so I'd prefer we drop that
    tag.

    IIUC when we originially looked at this the instrumentation wasn't safe
    and would violate noinstr requirements. Looking at linux/kcsan-checks.h,
    the comments block for __KCSAN_BARRIER_TO_SIGNAL_FENCE() say that it
    will respect __nokcsan, so it looks like that might be safe now.

    It looks like that's the case as of commit:

    bd3d5bd1a0ad3864 ("kcsan: Support WEAK_MEMORY with Clang where no objtool support exists")

    ... which requires clang 14.0.0+.

    That looks to have gone in concurrently with dd03762ab608, but is
    clearly a prerequisite, so as above I'd strongly prefer we drop the
    Fixes tag.

    It would be good if we could note that explicitly in the commit message.

    Have you eyeballed the generated assembly to verify that this works as
    expected for __nokcsan ?

    Thanks,
    Mark.

    > ---
    > arch/arm64/include/asm/barrier.h | 12 ++++++------
    > include/asm-generic/barrier.h | 4 ++++
    > 2 files changed, 10 insertions(+), 6 deletions(-)
    >
    > diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
    > index 62217be36217..9760a8d4ed0a 100644
    > --- a/arch/arm64/include/asm/barrier.h
    > +++ b/arch/arm64/include/asm/barrier.h
    > @@ -46,13 +46,13 @@
    > #define pmr_sync() do {} while (0)
    > #endif
    >
    > -#define mb() dsb(sy)
    > -#define rmb() dsb(ld)
    > -#define wmb() dsb(st)
    > +#define __mb() dsb(sy)
    > +#define __rmb() dsb(ld)
    > +#define __wmb() dsb(st)
    >
    > -#define dma_mb() dmb(osh)
    > -#define dma_rmb() dmb(oshld)
    > -#define dma_wmb() dmb(oshst)
    > +#define __dma_mb() dmb(osh)
    > +#define __dma_rmb() dmb(oshld)
    > +#define __dma_wmb() dmb(oshst)
    >
    > #define io_stop_wc() dgh()
    >
    > diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
    > index fd7e8fbaeef1..18863c50e9ce 100644
    > --- a/include/asm-generic/barrier.h
    > +++ b/include/asm-generic/barrier.h
    > @@ -38,6 +38,10 @@
    > #define wmb() do { kcsan_wmb(); __wmb(); } while (0)
    > #endif
    >
    > +#ifdef __dma_mb
    > +#define dma_mb() do { kcsan_mb(); __dma_mb(); } while (0)
    > +#endif
    > +
    > #ifdef __dma_rmb
    > #define dma_rmb() do { kcsan_rmb(); __dma_rmb(); } while (0)
    > #endif
    > --
    > 2.27.0
    >

    \
     
     \ /
      Last update: 2022-04-26 14:43    [W:3.059 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site