Hi Kalesh, On Fri, Jul 15, 2022 at 7:11 AM 'Kalesh Singh' via kernel-team <kernel-team@xxxxxxxxxxx> wrote: > > The unwinder code is made reusable so that it can be used to > unwind various types of stacks. One usecase is unwinding the > nVHE hyp stack from the host (EL1) in non-protected mode. This > means that the unwinder must be able to tracnslate HYP stack s/tracnslate/translate > addresses to kernel addresses. > > Add a callback (stack_trace_translate_fp_fn) to allow specifying > the translation function. > > Signed-off-by: Kalesh Singh <kaleshsingh@xxxxxxxxxx> > --- > arch/arm64/include/asm/stacktrace/common.h | 26 ++++++++++++++++++++-- > arch/arm64/kernel/stacktrace.c | 2 +- > 2 files changed, 25 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h > index 0c5cbfdb56b5..5f5d74a286f3 100644 > --- a/arch/arm64/include/asm/stacktrace/common.h > +++ b/arch/arm64/include/asm/stacktrace/common.h > @@ -123,9 +123,22 @@ static inline void unwind_init_common(struct unwind_state *state, > state->prev_fp = 0; > state->prev_type = STACK_TYPE_UNKNOWN; > } > +/** > + * stack_trace_translate_fp_fn() - Translates a non-kernel frame pointer to > + * a kernel address. > + * > + * @fp: the frame pointer to be updated to it's kernel address. > + * @type: the stack type associated with frame pointer @fp > + * > + * Returns true and success and @fp is updated to the corresponding > + * kernel virtual address; otherwise returns false. > + */ Please add a newline before the new block. Also, something which you have done in comment blocks in this patch as well as future patches (so I won't mention them again) is use the opening comment mark /** , which is meant for kernel-doc comments (https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html). However, this block, and many if not most of the others don't seem to be conformant (scripts/kernel-doc -v -none arch/arm64/include/asm/stacktrace/common.h). I think the easiest thing to do is to format them as a normal block: /*. > +typedef bool (*stack_trace_translate_fp_fn)(unsigned long *fp, > + enum stack_type type); > > static inline int unwind_next_common(struct unwind_state *state, > - struct stack_info *info) > + struct stack_info *info, > + stack_trace_translate_fp_fn translate_fp) > { > struct task_struct *tsk = state->task; > unsigned long fp = state->fp; > @@ -159,13 +172,22 @@ static inline int unwind_next_common(struct unwind_state *state, > __set_bit(state->prev_type, state->stacks_done); > } > > + /* Record fp as prev_fp before attempting to get the next fp */ > + state->prev_fp = fp; > + > + /* > + * If fp is not from the current address space perform the necessary > + * translation before dereferencing it to get the next fp. > + */ > + if (translate_fp && !translate_fp(&fp, info->type)) > + return -EINVAL; > + A call to unwind_next_common could fail having updated state->prev_fp as well as state->stacks_done. I think that it might be better to rework it so that there aren't any side effects should a call fail. Thanks, /fuad > /* > * Record this frame record's values and location. The prev_fp and > * prev_type are only meaningful to the next unwind_next() invocation. > */ > state->fp = READ_ONCE(*(unsigned long *)(fp)); > state->pc = READ_ONCE(*(unsigned long *)(fp + 8)); > - state->prev_fp = fp; > state->prev_type = info->type; > > return 0; > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index 834851939364..eef3cf6bf2d7 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -87,7 +87,7 @@ static int notrace unwind_next(struct unwind_state *state) > if (fp == (unsigned long)task_pt_regs(tsk)->stackframe) > return -ENOENT; > > - err = unwind_next_common(state, &info); > + err = unwind_next_common(state, &info, NULL); > if (err) > return err; > > -- > 2.37.0.170.g444d1eabd0-goog > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@xxxxxxxxxxx. > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
