Re: [RFC v4 9/9] Add S390 support for linux-kernel target

[Philipp Rudo <prudo@linux.vnet.ibm.com>]

Hi Yao

On Mon, 19 Jun 2017 14:20:34 +0100
Yao Qi <qiyaoltc@gmail.com> wrote:

> Philipp Rudo <prudo@linux.vnet.ibm.com> writes:
> 
> > +/* S390s kernel stack is split up on several memory locations:
> > +
> > +   - kernel stack (per task)
> > +   - async aka. interrupt stack (per cpu)
> > +   - panic stack (per cpu)
> > +   - restart stack (unique, global)
> > +
> > +   Each memory location is page aligned and has a size of four consecutive
> > +   pages (except the panic stack with only one page).  */
> > +
> > +enum s390_lk_stack_location
> > +{
> > +  S390_LK_STACK_INVALID = -1,
> > +  S390_LK_STACK_USER,
> > +  S390_LK_STACK_KERNEL,
> > +  S390_LK_STACK_ASYNC,
> > +  S390_LK_STACK_PANIC,
> > +  S390_LK_STACK_RESTART
> > +};
> > +  
> 
> Did you consider adding different frame unwinders for frames on
> different memory locations?  Your patches only adds two, but IMO, we
> need more unwinders for different frames,

All those stacks (except user space) are working the same way.  That's why it
is sufficient to only add one unwinder.  In theory it is sufficient to only
distinguish three cases (KERNEL, INVALID and "OTHER") but for the sake of
completeness I also added the other possible cases.  This makes the code only a
little longer but makes extending it (e.g. to compensate a bug as been done for
the kernel stack) much easier.

Philipp

> > +
> > +/* Helper macro for s390_lk_get_stack_location to check for stacks which
> > +   locations are stored in the lowcore.
> > +
> > +      _addr	address to be checked for
> > +      _lc	address of the corresponding lowcore
> > +      _stack	field name of stack in lowcore
> > +      _type	type to be returned if _addr is on _stack
> > +      _size	size of _stack
> > +*/
> > +
> > +#define s390_lk_check_lowcore_stack(_addr, _lc, _stack, _type,
> > _size)	\
> > +
> > do									\
> > +
> > {									\
> > +      CORE_ADDR _##_stack, _##_stack##_top,
> > _##_stack##_bottom;		\
> > +      _##_stack = lk_read_addr ((_lc) + LK_OFFSET (lowcore,
> > _stack));	\
> > +      _##_stack##_top = S390_LK_ROUNDUP (_##_stack,
> > S390_LK_PAGESIZE);	\
> > +      _##_stack##_bottom = _##_stack##_top -
> > (_size);			\
> > +      if ((_addr) <= _##_stack##_top && (_addr) >=
> > _##_stack##_bottom)	\
> > +	return
> > (_type);							\
> > +    }
> > \
> > +  while (0)
> > +
> > +/* Find and return the stack location of ADDR belonging to TASK.  */
> > +
> > +static enum s390_lk_stack_location
> > +s390_lk_get_stack_location (CORE_ADDR task, CORE_ADDR addr)
> > +{
> > +  CORE_ADDR lowcore, top, bottom;
> > +  unsigned int cpu = lk_task_running (task);
> > +
> > +
> > +  /* Kernel stack.  */
> > +  bottom = lk_read_addr (task + LK_OFFSET (task_struct, stack));
> > +  top = bottom + S390_LK_STACKSIZE;
> > +  if (addr <= top && addr >= bottom)
> > +    return S390_LK_STACK_KERNEL;
> > +
> > +  /* A sleeping task only has the kernel stack.  If a sleeping task reaches
> > +     this point ADDR isn't on the stack.  */
> > +  if (cpu == LK_CPU_INVAL)
> > +    return S390_LK_STACK_INVALID;
> > +
> > +  lowcore = s390_lk_get_lowcore (cpu);
> > +
> > +  /* Async aka. interrupt stack.  */
> > +  s390_lk_check_lowcore_stack (addr, lowcore, async_stack,
> > +			       S390_LK_STACK_ASYNC, S390_LK_ASYNCSIZE);
> > +
> > +  /* Panic stack.
> > +     Note: The panic stack only has a size of one page.  */
> > +  s390_lk_check_lowcore_stack (addr, lowcore, panic_stack,
> > +			       S390_LK_STACK_PANIC, S390_LK_PAGESIZE);
> > +
> > +  /* Restart stack.  */
> > +  s390_lk_check_lowcore_stack (addr, lowcore, restart_stack,
> > +			       S390_LK_STACK_RESTART, S390_LK_ASYNCSIZE);
> > +
> > +  return S390_LK_STACK_INVALID;
> > +}  
> 
> The unwinders for different frames should be chained in this order.
>