Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

[Blue Swirl <blauwirbel@gmail.com>]

On Thu, May 26, 2011 at 8:25 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> On Fri, 6 May 2011 18:36:31 +0300
>> Blue Swirl <blauwirbel@gmail.com> wrote:
>>
>>> On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>> > Blue Swirl <blauwirbel@gmail.com> writes:
>>> >
>>> >> On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> >>> On Sat, 30 Apr 2011 09:33:15 +0300
>>> >>> Blue Swirl <blauwirbel@gmail.com> wrote:
>>> >>>
>>> >>>> On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> >>>> > This series introduces the inject-nmi command for QMP, which sends an
>>> >>>> > NMI to _all_ guest's CPUs.
>>> >>>> >
>>> >>>> > Also note that this series changes the human monitor nmi command to use
>>> >>>> > the QMP implementation, which means that it now has a DIFFERENT behavior.
>>> >>>> > Please, check patch 3/3 for details.
>>> >>>>
>>> >>>> As discussed earlier, please change the QMP version for future
>>> >>>> expandability so that instead of single command 'inject-nmi', 'inject'
>>> >>>> takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
>>> >>>> 'inject' should be added.
>>> >>>
>>> >>> I'm not sure I agree with this, because we risky overloading 'inject' the
>>> >>> same way we did with the 'change' command.
>>> >>>
>>> >>> What's 'inject' supposed to do in the future?
>>> >>
>>> >> Inject other IRQs, for example inject nmi could become an alias to
>>> >> something like
>>> >> inject /apic@fee00000:l1int
>>> >> which would be a shorthand for
>>> >> raise /apic@fee00000:l1int
>>> >> lower /apic@fee00000:l1int
>>> >>
>>> >> I think we only need a registration framework for IRQs and other signals.
>>> >
>>> > Yes, we could use nicer infrastructure for modeling IRQs.  No, we
>>> > shouldn't reject Lai's work because it doesn't get us there.  Perfect is
>>> > the enemy of good.
>>> >
>>> > Pick one:
>>> >
>>> > 1. We take inject-nmi now.  Should we get a more general inject command
>>> > like the one you envisage later, we can deprecate inject-nmi, and remove
>>> > it after a suitable grace time.  Big deal.  We get the special problem
>>> > solved now, without really compromising future solutions for the general
>>> > problem.
>>> >
>>> > 2. We reject inject-nmi now.  The itch Lai tries to scratch remains
>>> > unscratched until we get a more general inject command.
>>> >
>>> > 2a. Rejection "motivates" Lai to solve the general problem[*].  Or maybe
>>> > it motivates somebody else.  We get the general problem solved sooner.
>>> > And maybe I get a pony for my birthday, too.
>>> >
>>> > 2b. The general problem remains unsolved along with the special problem.
>>> > We get nothing.
>>>
>>> 2c. Don't add full generic IRQ registration and aliases just now but
>>> handle 'inject' with only 'nmi'. That way we introduce no legacy
>>> baggage to the syntax.
>>
>> Can you give an example on how this is supposed to look like?
>
> No reply.  When you demand a redesign to generalize a simple feature to
> something only you envisage, please explain what exactly you want.
> Documentation to stick into qmp-commands.hx would be a start.  Here's
> the baseline from Luiz, for your editing convenience.
>
>
> inject-nmi
> ----------
>
> Inject an NMI on guest's CPUs.
>
> Arguments: None.
>
> Example:
>
> -> { "execute": "inject-nmi" }
> <- { "return": {} }
>
> Note: inject-nmi is only supported for x86 guest currently, it will
>      returns "Unsupported" error for non-x86 guest.

I think I explained it many times, but let's try again.

inject
----------

Inject a signal on guest machine.

Arguments: signal name.

Example:

-> { "execute": "inject",
"arguments": { "signal": "nmi" } }
<- { "return": {} }

-> { "execute": "inject",
"arguments": { "signal": "/apic@fee00000:l1int" } }
<- { "return": {} }

Note: the set of signals supported depends on the CPU architecture and
board type, unknown or unsupported names will
     return "Unsupported" error.