| Message ID | 20230222033021.983168-2-guoren@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | riscv: Add GENERIC_ENTRY support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 13 and now 13 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 2475 this patch: 2475 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 17341 this patch: 17341 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 729 and now 729 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 2 this patch: 2 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | fail | ERROR: Macros with complex values should be enclosed in parentheses |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
guoren@kernel.org writes: > From: Lai Jiangshan <laijs@linux.alibaba.com> > > Using __noinstr_section() doesn't automatically disable all > instrumentations on the section. Inhibition for some > instrumentations requires extra code. I.E. KPROBES explicitly > avoids instrumenting on .noinstr.text. Guo, the generic entry series doesn't apply cleanly on riscv/for-next >6.2-rc1, and this patch is the issue. Could you do a respin (potentially w/o this patch)? Cheers, Björn
On Wed, Feb 22, 2023 at 11:56 AM <guoren@kernel.org> wrote: > > From: Lai Jiangshan <laijs@linux.alibaba.com> > > Using __noinstr_section() doesn't automatically disable all > instrumentations on the section. Inhibition for some > instrumentations requires extra code. I.E. KPROBES explicitly > avoids instrumenting on .noinstr.text. > > Suggested-by: Nick Desaulniers <ndesaulniers@google.com> > Suggested-by: Peter Zijlstra <peterz@infradead.org> > Reviewed-by: Miguel Ojeda <ojeda@kernel.org> > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Björn Töpel <bjorn@rivosinc.com> > Tested-by: Jisheng Zhang <jszhang@kernel.org> > Tested-by: Guo Ren <guoren@kernel.org> > Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> Hello The change has been made upstream as a part of the commit 2b5a0e425e6e(objtool/idle: Validate __cpuidle code as noinstr). https://lore.kernel.org/r/20230112195540.373461409@infradead.org Thanks Lai > --- > include/linux/compiler_types.h | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h > index 7c1afe0f4129..0a2ca5755be7 100644 > --- a/include/linux/compiler_types.h > +++ b/include/linux/compiler_types.h > @@ -231,12 +231,19 @@ struct ftrace_likely_data { > #define __no_sanitize_or_inline __always_inline > #endif > > -/* Section for code which can't be instrumented at all */ > -#define noinstr \ > - noinline notrace __attribute((__section__(".noinstr.text"))) \ > - __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \ > +/* > + * Using __noinstr_section() doesn't automatically disable all instrumentations > + * on the section. Inhibition for some instrumentations requires extra code. > + * I.E. KPROBES explicitly avoids instrumenting on .noinstr.text. > + */ > +#define __noinstr_section(section) \ > + noinline notrace __section(section) __no_profile \ > + __no_kcsan __no_sanitize_address __no_sanitize_coverage \ > __no_sanitize_memory > > +/* Section for code which can't be instrumented at all */ > +#define noinstr __noinstr_section(".noinstr.text") > + > #endif /* __KERNEL__ */ > > #endif /* __ASSEMBLY__ */ > -- > 2.36.1 >
On Wed, Mar 22, 2023 at 10:46 PM Björn Töpel <bjorn@kernel.org> wrote: > > guoren@kernel.org writes: > > > From: Lai Jiangshan <laijs@linux.alibaba.com> > > > > Using __noinstr_section() doesn't automatically disable all > > instrumentations on the section. Inhibition for some > > instrumentations requires extra code. I.E. KPROBES explicitly > > avoids instrumenting on .noinstr.text. > > Guo, the generic entry series doesn't apply cleanly on > riscv/for-next >6.2-rc1, and this patch is the issue. It has been merged in palmer/for-next. Thx for taking care. 54b3948f381c (HEAD -> for-next, palmer/for-next) Merge patch series "riscv: Add GENERIC_ENTRY support" 45b32b946a97 riscv: entry: Consolidate general regs saving/restoring ab9164dae273 riscv: entry: Consolidate ret_from_kernel_thread into ret_from_fork 0bf298ad2b61 riscv: entry: Remove extra level wrappers of trace_hardirqs_{on,off} f0bddf50586d riscv: entry: Convert to generic entry d0db02c62879 riscv: entry: Add noinstr to prevent instrumentation inserted 8574bf8d0ddd riscv: ptrace: Remove duplicate operation > > Could you do a respin (potentially w/o this patch)? > > > Cheers, > Björn
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 7c1afe0f4129..0a2ca5755be7 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -231,12 +231,19 @@ struct ftrace_likely_data { #define __no_sanitize_or_inline __always_inline #endif -/* Section for code which can't be instrumented at all */ -#define noinstr \ - noinline notrace __attribute((__section__(".noinstr.text"))) \ - __no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \ +/* + * Using __noinstr_section() doesn't automatically disable all instrumentations + * on the section. Inhibition for some instrumentations requires extra code. + * I.E. KPROBES explicitly avoids instrumenting on .noinstr.text. + */ +#define __noinstr_section(section) \ + noinline notrace __section(section) __no_profile \ + __no_kcsan __no_sanitize_address __no_sanitize_coverage \ __no_sanitize_memory +/* Section for code which can't be instrumented at all */ +#define noinstr __noinstr_section(".noinstr.text") + #endif /* __KERNEL__ */ #endif /* __ASSEMBLY__ */