Re: [Qemu-devel] [PATCH v3 1/4] cpus: Define NMI callback

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 10 Jun 2014 18:29:43 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 10/06/2014 16:48, Luiz Capitulino ha scritto:
> > > The s390 restart interrupt is a per-vcpu interrupt, which we really
> > > don't want to inject on _all_ vcpus. OTOH, we want to inject that
> > > interrupt on any vcpu - we don't care which one it is. So I'd really
> > > like an "inject nmi on default cpu" option.
> >
> > We could define a default CPU for the command. What isn't going to work
> > is to use the human monitor's "current CPU" concept (set with the cpu
> > command).
> 
> It isn't going to work, but to me it seems like a bug.  Why was the NMI 
> command even converted to QAPI if it cannot work?  

It works by sending the NMI to all CPUs. That's the solution we found to
avoid creating a dependency between a QMP command and HMP w/o breaking the
nmi command compatibility. If this is not right, I'd have expected people
to complain as I know this has been used for quite some time to generate
crash dumps.

All I'm suggesting is to add a new QMP command, say inject-nmi-cpu, which
takes a CPU index as an argument. This solves any concern on compatibility
Alex talked about, gives a chance to design the command the way we like and
doesn't create a dependency between a QMP command and HMP (which would be
a design flaw).

The QAPI had two main motivations. Automatically generate open-coded json
present in all QMP commands and decouple QMP from HMP, as the two were
tight coupled in the past. Having the two decoupled is very important so
that QMP doesn't depend on HMP's global garbage (good for concurrency and
for allowing to have a different HMP on top of QMP). Also, we were planning
having a QMP-only socket, session support and other features. Being able to
build QMP w/o building the monitor would be a good milestone.

Making a QMP command depend on HMP is step back for all this planning and
a big de-motivator for all the hard work we put on separating the two.

Again, I wonder if I'm missing something, but I see it as a very simple change
to add a new command. Note that we can do whatever we want in hmp_inject_nmi(),
we can use the new command with whatever semantics we like.