Re: [PATCH v2 6/8] arm64: stacktrace: remove stack type from fp translator

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Aug 08, 2022 at 01:08:42PM +0100, Mark Rutland wrote:
> On Mon, Aug 08, 2022 at 12:55:35PM +0100, Will Deacon wrote:
> > On Fri, Aug 05, 2022 at 01:45:20PM +0100, Mark Rutland wrote:
> 
> > >  /*
> > >   * kvm_nvhe_stack_kern_va - Convert KVM nVHE HYP stack addresses to a kernel VAs
> > >   *
> > > @@ -34,27 +62,31 @@
> > >   * Returns true on success and updates @addr to its corresponding kernel VA;
> > >   * otherwise returns false.
> > >   */
> > > -static bool kvm_nvhe_stack_kern_va(unsigned long *addr,
> > > -				   enum stack_type type)
> > > +static bool kvm_nvhe_stack_kern_va(unsigned long *addr)
> > >  {
> > >  	struct kvm_nvhe_stacktrace_info *stacktrace_info;
> > >  	unsigned long hyp_base, kern_base, hyp_offset;
> > > +	struct stack_info stack;
> > >  
> > >  	stacktrace_info = this_cpu_ptr_nvhe_sym(kvm_stacktrace_info);
> > >  
> > > -	switch (type) {
> > > -	case STACK_TYPE_HYP:
> > > +	stack = stackinfo_get_hyp();
> > > +	if (stackinfo_on_stack(&stack, *addr, 1)) {
> > >  		kern_base = (unsigned long)*this_cpu_ptr(&kvm_arm_hyp_stack_page);
> > >  		hyp_base = (unsigned long)stacktrace_info->stack_base;
> > > -		break;
> > > -	case STACK_TYPE_OVERFLOW:
> > > +		goto found;
> > > +	}
> > > +
> > > +	stack = stackinfo_get_overflow();
> > > +	if (stackinfo_on_stack(&stack, *addr, 1)) {
> > 
> > Why is '1' the appropriate size for these two checks, given that addr points
> > at an unsigned long?
> 
> That's a fair point.
> 
> My original rationale was that this was equivalent to the existing bheaviour,
> since the range has arleady been checked, but I agree it's not nice, and won't
> be right later in the series.
> 
> I'll adjust this to take an explicit size argument; the existing caller can
> pass in sizeof(unsigned long), and it can be reused for other sized objects in
> future.

FWIW, I've locally refactored this to take a size, and wrapped it with:

static bool kvm_nvhe_stack_kern_record_va(unsigned long *addr)
{
        return kvm_nvhe_stack_kern_va(addr, 16);
}

... since the frame record is actually two 8-byte values, and that matches what
unwind_next_frame_record() checks.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel