On Thu, Jul 21, 2016 at 05:54:33PM +0200, Igor Mammedov wrote: > Instead use QTAIL's tqe_prev field to detect if cpu's been > placed in list by cpu_exec_init() which is always set if > QTAIL element is in list. > > Fixes SIGSEGV on failure path in case cpu_index is assigned > by board and cpu.relalize() fails before cpu_exec_init() is called. > > In follow up patches, cpu_index will be assigned by boards that > support cpu hot(un)plug and need stable cpu_index that doesn't > depend on order cpus are created/removed. > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > Reported-by: David Gibson <da...@gibson.dropbear.id.au>
Looks correct, although I wonder a bit about changing QTAILQ_REMOVE()
for everyone for the sake of this one use case.
> ---
> include/qemu/queue.h | 2 ++
> exec.c | 4 ++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/qemu/queue.h b/include/qemu/queue.h
> index c2b6c81..2c2c74b 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -407,6 +407,7 @@ struct {
> \
> else \
> (head)->tqh_last = (elm)->field.tqe_prev; \
> *(elm)->field.tqe_prev = (elm)->field.tqe_next; \
> + (elm)->field.tqe_prev = NULL; \
> } while (/*CONSTCOND*/0)
>
> #define QTAILQ_FOREACH(var, head, field) \
> @@ -430,6 +431,7 @@ struct {
> \
> #define QTAILQ_EMPTY(head) ((head)->tqh_first == NULL)
> #define QTAILQ_FIRST(head) ((head)->tqh_first)
> #define QTAILQ_NEXT(elm, field) ((elm)->field.tqe_next)
> +#define QTAILQ_IN_USE(elm, field) ((elm)->field.tqe_prev)
>
> #define QTAILQ_LAST(head, headname) \
> (*(((struct headname *)((head)->tqh_last))->tqh_last))
> diff --git a/exec.c b/exec.c
> index 2f57c62..8c5da32 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -643,8 +643,8 @@ void cpu_exec_exit(CPUState *cpu)
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> cpu_list_lock();
> - if (cpu->cpu_index == -1) {
> - /* cpu_index was never allocated by this @cpu or was already freed.
> */
> + if (!QTAILQ_IN_USE(cpu, node)) {
> + /* there is nothing to undo since cpu_exec_init() hasn't been called
> */
> cpu_list_unlock();
> return;
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
