On Fri, Jul 15, 2022 at 6:57 AM 'Fuad Tabba' via kernel-team <kernel-team@xxxxxxxxxxx> wrote: > > 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. Hi Fuad, Thanks for the comments. I'll address them in the next version. --Kalesh > > 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. > > > > -- > 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
