Re: [Qemu-devel] [PATCH v4 2/4] target-s390x: Migrate to new nmi_monitor_handler() CPU callback

[Cornelia Huck]

On Thu,  5 Jun 2014 00:25:59 +1000
Alexey Kardashevskiy <address@hidden> wrote:

> This defines a nmi_monitor_handler() callback for s390 CPU class.
> 
> This removes #ifdef s390 branch in qmp_inject_nmi so new s390's
> nmi_monitor_handler() callback is going to be used for NMI.
> 
> Since nmi_monitor_handler()-calling code is platform independent,
> CPUState::cpu_index is used instead of S390CPU::env.cpu_num.
> There should not be any change in behaviour as both @cpu_index and
> @cpu_num are global CPU numbers.
> 
> Also, s390_cpu_restart() takes care of preforming operations in
> the specific CPU thread so no extra measure is required here either.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>

Acked-by: Cornelia Huck <address@hidden>

> ---
> Changes:
> v4:
> * s/\<nmi\>/nmi_monitor_handler/
> 
> v3:
> * now contains both old code removal and new code insertion, easier to
> track changes
> 
> ---
> Is there any good reason to have @cpu_num in addition to @cpu_index?
> Just asking :)

Seems to be historical afaics. The not-merged cpu hotplug code mucks
around with it a bit, but it would probably be safe to use cpu_index
instead.

> ---
>  cpus.c             | 14 --------------
>  target-s390x/cpu.c |  6 ++++++
>  2 files changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index c0b8918..53ae516 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1480,20 +1480,6 @@ void qmp_inject_nmi(Error **errp)
>              apic_deliver_nmi(cpu->apic_state);
>          }
>      }
> -#elif defined(TARGET_S390X)
> -    CPUState *cs;
> -    S390CPU *cpu;
> -
> -    CPU_FOREACH(cs) {
> -        cpu = S390_CPU(cs);
> -        if (cpu->env.cpu_num == monitor_get_cpu_index()) {
> -            if (s390_cpu_restart(S390_CPU(cs)) == -1) {
> -                error_set(errp, QERR_UNSUPPORTED);
> -                return;
> -            }
> -            break;
> -        }
> -    }
>  #else
>      CPUState *cs = qemu_get_cpu(monitor_get_cpu_index());
>      CPUClass *cc = CPU_GET_CLASS(cs);
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index c3082b7..267cfa4 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -160,6 +160,11 @@ static void s390_cpu_full_reset(CPUState *s)
>      tlb_flush(s, 1);
>  }
> 
> +static int s390_cpu_nmi_monitor_handler(CPUState *cs)
> +{
> +    return s390_cpu_restart(S390_CPU(cs));
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static void s390_cpu_machine_reset_cb(void *opaque)
>  {
> @@ -245,6 +250,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void 
> *data)
>      scc->cpu_reset = s390_cpu_reset;
>      scc->initial_cpu_reset = s390_cpu_initial_reset;
>      cc->reset = s390_cpu_full_reset;
> +    cc->nmi_monitor_handler = s390_cpu_nmi_monitor_handler;
>      cc->has_work = s390_cpu_has_work;
>      cc->do_interrupt = s390_cpu_do_interrupt;
>      cc->dump_state = s390_cpu_dump_state;