Re: [Qemu-devel] [PATCH v5 1/4] cpus: Define callback for QEMU "nmi" command

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Tue, 10 Jun 2014 16:18:00 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> This introduces an NMI (Non Maskable Interrupt) nmi_monitor_handler()
> callback to the CPU class. It is called from QMP's "nmi" command and
> performs an action required to cause debug crash dump on in-kernel
> debugger invocation. The callback returns Error**.
> 
> This adds support for it in qmp_inject_nmi(). Since no architecture
> supports it at the moment, there is no change in behaviour.
> 
> This changes inject-nmi command description for HMP and QMP.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v5:
> * s/given guest's (CPU|VCPU)/default CPU/
> * nmi_monitor_handler() now returns Error**
> 
> 
> v4:
> * s/\<nmi\>/nmi_monitor_handler/
> 
> v3:
> * actual nmi() enablement moved from last patch to first patch
> * changed description for QMP command too
> ---
>  cpus.c            | 9 ++++++++-
>  hmp-commands.hx   | 6 ++----
>  include/qom/cpu.h | 1 +
>  qapi-schema.json  | 4 ++--
>  qmp-commands.hx   | 3 +--
>  5 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index dd7ac13..b9d6602 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1495,6 +1495,13 @@ void qmp_inject_nmi(Error **errp)
>          }
>      }
>  #else
> -    error_set(errp, QERR_UNSUPPORTED);
> +    CPUState *cs = qemu_get_cpu(monitor_get_cpu_index());
> +    CPUClass *cc = CPU_GET_CLASS(cs);

Just wondering: Is CPU_GET_CLASS(NULL) really safe?

> +
> +    if (cs && cc->nmi_monitor_handler) {

Or is cs == NULL simply not possible, and the code should check for
cc != NULL here instead?

> +        cc->nmi_monitor_handler(cs, errp);
> +    } else {
> +        error_set(errp, QERR_UNSUPPORTED);
> +    }
>  #endif
>  }