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

Yao Qi
2017-06-19T13:20:37Z

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,

> +
> +/* 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.

--
Yao (齐尧)