This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC v4 9/9] Add S390 support for linux-kernel target
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: Philipp Rudo <prudo at linux dot vnet dot ibm dot com>
- Cc: gdb-patches at sourceware dot org, omair dot javaid at linaro dot org, yao dot qi at linaro dot org, peter dot griffin at linaro dot org, arnez at linux dot vnet dot ibm dot com
- Date: Mon, 19 Jun 2017 14:20:34 +0100
- Subject: Re: [RFC v4 9/9] Add S390 support for linux-kernel target
- Authentication-results: sourceware.org; auth=none
- References: <20170612170836.25174-1-prudo@linux.vnet.ibm.com> <20170612193900.234bc540@ThinkPad>
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 (齐尧)