diff mbox

[2/2,V7] qemu,qmp: add inject-nmi qmp command

Message ID 4D74A974.6090509@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lai Jiangshan March 7, 2011, 9:46 a.m. UTC
None

Comments

Daniel P. Berrangé April 4, 2011, 10:59 a.m. UTC | #1
On Mon, Mar 07, 2011 at 05:46:28PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> Date: Mon, 7 Mar 2011 17:05:15 +0800
> Subject: [PATCH 2/2] qemu,qmp: add inject-nmi qmp command
> 
> inject-nmi command injects an NMI on all CPUs of guest.
> It is only supported for x86 guest currently, it will
> returns "Unsupported" error for non-x86 guest.
> 
> ---
>  hmp-commands.hx |    2 +-
>  monitor.c       |   18 +++++++++++++++++-
>  qmp-commands.hx |   29 +++++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+), 2 deletions(-)

Does anyone have any feedback on this addition, or are all new
QMP patch proposals blocked pending Anthony's QAPI work ?

We'd like to support it in libvirt and thus want it to be
available in QMP, as well as HMP.

> @@ -2566,6 +2566,22 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
>              break;
>          }
>  }
> +
> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    CPUState *env;
> +
> +    for (env = first_cpu; env != NULL; env = env->next_cpu)
> +        cpu_interrupt(env, CPU_INTERRUPT_NMI);
> +
> +    return 0;
> +}
> +#else
> +static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
> +{
> +    qerror_report(QERR_UNSUPPORTED);
> +    return -1;
> +}
>  #endif
>  

Interesting that with HMP you need to specify a single CPU index, but
with QMP it is injecting to all CPUs at once. Is there any compelling
reason why we'd ever need the ability to only inject to a single CPU
from an app developer POV ?

Daniel
Luiz Capitulino April 6, 2011, 5:47 p.m. UTC | #2
On Mon, 04 Apr 2011 08:05:48 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/04/2011 07:54 AM, Avi Kivity wrote:
> > On 04/04/2011 01:59 PM, Daniel P. Berrange wrote:
> >> Interesting that with HMP you need to specify a single CPU index, but
> >> with QMP it is injecting to all CPUs at once. Is there any compelling
> >> reason why we'd ever need the ability to only inject to a single CPU
> >> from an app developer POV ?
> >
> > When a PC has an NMI button, it is (I presume) connected to all CPUs' 
> > LINT1 pin, which is often configured as an NMI input.  So the all-cpu 
> > variant corresponds to real hardware, while the single-cpu variant 
> > doesn't.
> >
> > wrt the app developer POV, the only use I'm aware of is that you can 
> > configure Windows to dump core when the NMI button is pressed and thus 
> > debug driver problems.  It's likely more reliable when sent to all cpus.
> 
> It either needs to be removed from HMP or added to QMP.  HMP shouldn't 
> have more features than QMP (even if those features are non-sensible).

Is anyone against changing HMP behavior to send it to all CPUs?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 6, 2011, 6:03 p.m. UTC | #3
On 04/06/2011 12:47 PM, Luiz Capitulino wrote:
> On Mon, 04 Apr 2011 08:05:48 -0500
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>> On 04/04/2011 07:54 AM, Avi Kivity wrote:
>>> On 04/04/2011 01:59 PM, Daniel P. Berrange wrote:
>>>> Interesting that with HMP you need to specify a single CPU index, but
>>>> with QMP it is injecting to all CPUs at once. Is there any compelling
>>>> reason why we'd ever need the ability to only inject to a single CPU
>>>> from an app developer POV ?
>>> When a PC has an NMI button, it is (I presume) connected to all CPUs'
>>> LINT1 pin, which is often configured as an NMI input.  So the all-cpu
>>> variant corresponds to real hardware, while the single-cpu variant
>>> doesn't.
>>>
>>> wrt the app developer POV, the only use I'm aware of is that you can
>>> configure Windows to dump core when the NMI button is pressed and thus
>>> debug driver problems.  It's likely more reliable when sent to all cpus.
>> It either needs to be removed from HMP or added to QMP.  HMP shouldn't
>> have more features than QMP (even if those features are non-sensible).
> Is anyone against changing HMP behavior to send it to all CPUs?

Makes sense to me.

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Capitulino April 6, 2011, 6:08 p.m. UTC | #4
On Wed, 06 Apr 2011 13:03:37 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 04/06/2011 12:47 PM, Luiz Capitulino wrote:
> > On Mon, 04 Apr 2011 08:05:48 -0500
> > Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >
> >> On 04/04/2011 07:54 AM, Avi Kivity wrote:
> >>> On 04/04/2011 01:59 PM, Daniel P. Berrange wrote:
> >>>> Interesting that with HMP you need to specify a single CPU index, but
> >>>> with QMP it is injecting to all CPUs at once. Is there any compelling
> >>>> reason why we'd ever need the ability to only inject to a single CPU
> >>>> from an app developer POV ?
> >>> When a PC has an NMI button, it is (I presume) connected to all CPUs'
> >>> LINT1 pin, which is often configured as an NMI input.  So the all-cpu
> >>> variant corresponds to real hardware, while the single-cpu variant
> >>> doesn't.
> >>>
> >>> wrt the app developer POV, the only use I'm aware of is that you can
> >>> configure Windows to dump core when the NMI button is pressed and thus
> >>> debug driver problems.  It's likely more reliable when sent to all cpus.
> >> It either needs to be removed from HMP or added to QMP.  HMP shouldn't
> >> have more features than QMP (even if those features are non-sensible).
> > Is anyone against changing HMP behavior to send it to all CPUs?
> 
> Makes sense to me.

So, Lai, in order to get this merged could you please do the following:

 1. Address checkpath.pl errors

 2. Change the HMP to use this implementation, which send the NMI
    to all CPUs

 3. Any other review comments I might be missing :)

> 
> Regards,
> 
> Anthony Liguori
> 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka April 6, 2011, 6:17 p.m. UTC | #5
On 2011-04-06 20:08, Luiz Capitulino wrote:
> On Wed, 06 Apr 2011 13:03:37 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
> 
>> On 04/06/2011 12:47 PM, Luiz Capitulino wrote:
>>> On Mon, 04 Apr 2011 08:05:48 -0500
>>> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>>
>>>> On 04/04/2011 07:54 AM, Avi Kivity wrote:
>>>>> On 04/04/2011 01:59 PM, Daniel P. Berrange wrote:
>>>>>> Interesting that with HMP you need to specify a single CPU index, but
>>>>>> with QMP it is injecting to all CPUs at once. Is there any compelling
>>>>>> reason why we'd ever need the ability to only inject to a single CPU
>>>>>> from an app developer POV ?
>>>>> When a PC has an NMI button, it is (I presume) connected to all CPUs'
>>>>> LINT1 pin, which is often configured as an NMI input.  So the all-cpu
>>>>> variant corresponds to real hardware, while the single-cpu variant
>>>>> doesn't.
>>>>>
>>>>> wrt the app developer POV, the only use I'm aware of is that you can
>>>>> configure Windows to dump core when the NMI button is pressed and thus
>>>>> debug driver problems.  It's likely more reliable when sent to all cpus.
>>>> It either needs to be removed from HMP or added to QMP.  HMP shouldn't
>>>> have more features than QMP (even if those features are non-sensible).
>>> Is anyone against changing HMP behavior to send it to all CPUs?
>>
>> Makes sense to me.
> 
> So, Lai, in order to get this merged could you please do the following:
> 
>  1. Address checkpath.pl errors
> 
>  2. Change the HMP to use this implementation, which send the NMI
>     to all CPUs

HMP is currently x86-only, thus it's probably OK to model it after some
PC feature (though I don't know if there aren't NMI buttons with BP-only
wirings). But will the consolidate version be defined for all
architectures? We should avoid "exporting" x86-specific assumptions.

Jan
Luiz Capitulino April 6, 2011, 7 p.m. UTC | #6
On Wed, 06 Apr 2011 20:17:47 +0200
Jan Kiszka <jan.kiszka@siemens.com> wrote:

> On 2011-04-06 20:08, Luiz Capitulino wrote:
> > On Wed, 06 Apr 2011 13:03:37 -0500
> > Anthony Liguori <anthony@codemonkey.ws> wrote:
> > 
> >> On 04/06/2011 12:47 PM, Luiz Capitulino wrote:
> >>> On Mon, 04 Apr 2011 08:05:48 -0500
> >>> Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >>>
> >>>> On 04/04/2011 07:54 AM, Avi Kivity wrote:
> >>>>> On 04/04/2011 01:59 PM, Daniel P. Berrange wrote:
> >>>>>> Interesting that with HMP you need to specify a single CPU index, but
> >>>>>> with QMP it is injecting to all CPUs at once. Is there any compelling
> >>>>>> reason why we'd ever need the ability to only inject to a single CPU
> >>>>>> from an app developer POV ?
> >>>>> When a PC has an NMI button, it is (I presume) connected to all CPUs'
> >>>>> LINT1 pin, which is often configured as an NMI input.  So the all-cpu
> >>>>> variant corresponds to real hardware, while the single-cpu variant
> >>>>> doesn't.
> >>>>>
> >>>>> wrt the app developer POV, the only use I'm aware of is that you can
> >>>>> configure Windows to dump core when the NMI button is pressed and thus
> >>>>> debug driver problems.  It's likely more reliable when sent to all cpus.
> >>>> It either needs to be removed from HMP or added to QMP.  HMP shouldn't
> >>>> have more features than QMP (even if those features are non-sensible).
> >>> Is anyone against changing HMP behavior to send it to all CPUs?
> >>
> >> Makes sense to me.
> > 
> > So, Lai, in order to get this merged could you please do the following:
> > 
> >  1. Address checkpath.pl errors
> > 
> >  2. Change the HMP to use this implementation, which send the NMI
> >     to all CPUs
> 
> HMP is currently x86-only, thus it's probably OK to model it after some
> PC feature (though I don't know if there aren't NMI buttons with BP-only
> wirings). But will the consolidate version be defined for all
> architectures? We should avoid "exporting" x86-specific assumptions.

Right, but honestly speaking, I don't know how this works for other arches.

So, the best thing to do is to have a general design that can be used
by any architecture. Of course that we can also add a new command later
if needed.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Maydell April 6, 2011, 7:27 p.m. UTC | #7
On 6 April 2011 20:00, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Wed, 06 Apr 2011 20:17:47 +0200
> Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> HMP is currently x86-only, thus it's probably OK to model it after some
>> PC feature (though I don't know if there aren't NMI buttons with BP-only
>> wirings). But will the consolidate version be defined for all
>> architectures? We should avoid "exporting" x86-specific assumptions.
>
> Right, but honestly speaking, I don't know how this works for other arches.
>
> So, the best thing to do is to have a general design that can be used
> by any architecture. Of course that we can also add a new command later
> if needed.

Well, I'm not sure "send a random interrupt to the core" makes
much sense for ARM... So what are we actually trying to model here?
Some sort of way to do "press a front panel switch" via remote monitor
protocol? I guess you could have an API for boards to register any
switches they had...

-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 6, 2011, 7:34 p.m. UTC | #8
On 04/06/2011 02:27 PM, Peter Maydell wrote:
>> Right, but honestly speaking, I don't know how this works for other arches.
>>
>> So, the best thing to do is to have a general design that can be used
>> by any architecture. Of course that we can also add a new command later
>> if needed.
> Well, I'm not sure "send a random interrupt to the core" makes
> much sense for ARM... So what are we actually trying to model here?
> Some sort of way to do "press a front panel switch" via remote monitor
> protocol? I guess you could have an API for boards to register any
> switches they had...

http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm

If an OS is totally hosed (spinning with interrupts disabled), and NMI 
can be used to generate a crash dump.

It's a debug feature and modelling it exactly the way we are probably 
makes sense for other architectures too.  The real semantics are 
basically force guest crash dump.

Regards,

Anthony Liguori

> -- PMM
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka April 7, 2011, 7:29 a.m. UTC | #9
On 2011-04-06 21:34, Anthony Liguori wrote:
> On 04/06/2011 02:27 PM, Peter Maydell wrote:
>>> Right, but honestly speaking, I don't know how this works for other arches.
>>>
>>> So, the best thing to do is to have a general design that can be used
>>> by any architecture. Of course that we can also add a new command later
>>> if needed.
>> Well, I'm not sure "send a random interrupt to the core" makes
>> much sense for ARM... So what are we actually trying to model here?
>> Some sort of way to do "press a front panel switch" via remote monitor
>> protocol? I guess you could have an API for boards to register any
>> switches they had...
> 
> http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
> 
> If an OS is totally hosed (spinning with interrupts disabled), and NMI 
> can be used to generate a crash dump.
> 
> It's a debug feature and modelling it exactly the way we are probably 
> makes sense for other architectures too.  The real semantics are 
> basically force guest crash dump.

Right, it's a debugging tool. And that does not necessarily means it has
to match real hardware. I could imagine scenarios where it could be
helpful to direct the NMI to a specific core, e.g. in AMP setups if only
one OS ran wild.

Jan
Peter Maydell April 7, 2011, 6:10 p.m. UTC | #10
On 6 April 2011 20:34, Anthony Liguori <anthony@codemonkey.ws> wrote:
> http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
>
> If an OS is totally hosed (spinning with interrupts disabled), and NMI can
> be used to generate a crash dump.
>
> It's a debug feature and modelling it exactly the way we are probably makes
> sense for other architectures too.  The real semantics are basically force
> guest crash dump.

Ah, right. (There isn't really an equivalent to this on ARM since
we don't have a real NMI equivalent. So any implementation for ARM
qemu would be board dependent since you could wire a watchdog up to
any interrupt.)

Should we try to pick a command name that says what it's supposed to
do rather than how it happens to be implemented on x86 ?

-- PMM
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 7, 2011, 6:32 p.m. UTC | #11
On 04/07/2011 01:10 PM, Peter Maydell wrote:
> On 6 April 2011 20:34, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
>>
>> If an OS is totally hosed (spinning with interrupts disabled), and NMI can
>> be used to generate a crash dump.
>>
>> It's a debug feature and modelling it exactly the way we are probably makes
>> sense for other architectures too.  The real semantics are basically force
>> guest crash dump.
> Ah, right. (There isn't really an equivalent to this on ARM since
> we don't have a real NMI equivalent. So any implementation for ARM
> qemu would be board dependent since you could wire a watchdog up to
> any interrupt.)
>
> Should we try to pick a command name that says what it's supposed to
> do rather than how it happens to be implemented on x86 ?

Yup, I was thinking the same thing after I sent the note above.  If we 
call it 'force-crash-dump', we can implement it as an NMI on target-i386 
and potentially as something else on a different target.

Regards,

Anthony Liguori

> -- PMM
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov April 7, 2011, 6:51 p.m. UTC | #12
On Thu, Apr 07, 2011 at 01:32:50PM -0500, Anthony Liguori wrote:
> On 04/07/2011 01:10 PM, Peter Maydell wrote:
> >On 6 April 2011 20:34, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >>http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
> >>
> >>If an OS is totally hosed (spinning with interrupts disabled), and NMI can
> >>be used to generate a crash dump.
> >>
> >>It's a debug feature and modelling it exactly the way we are probably makes
> >>sense for other architectures too.  The real semantics are basically force
> >>guest crash dump.
> >Ah, right. (There isn't really an equivalent to this on ARM since
> >we don't have a real NMI equivalent. So any implementation for ARM
> >qemu would be board dependent since you could wire a watchdog up to
> >any interrupt.)
> >
> >Should we try to pick a command name that says what it's supposed to
> >do rather than how it happens to be implemented on x86 ?
> 
> Yup, I was thinking the same thing after I sent the note above.  If
> we call it 'force-crash-dump', we can implement it as an NMI on
> target-i386 and potentially as something else on a different target.
> 
NMI does not have to generate crash dump on every guest we support.
Actually even for windows guest it does not generate one without
tweaking registry. For all I know there is a guest that checks mail when
NMI arrives. Lets give meaningful name, like inject-nmi, for nmi
injection command.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Blue Swirl April 7, 2011, 7:04 p.m. UTC | #13
On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov <gleb@redhat.com> wrote:
> On Thu, Apr 07, 2011 at 01:32:50PM -0500, Anthony Liguori wrote:
>> On 04/07/2011 01:10 PM, Peter Maydell wrote:
>> >On 6 April 2011 20:34, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >>http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
>> >>
>> >>If an OS is totally hosed (spinning with interrupts disabled), and NMI can
>> >>be used to generate a crash dump.
>> >>
>> >>It's a debug feature and modelling it exactly the way we are probably makes
>> >>sense for other architectures too.  The real semantics are basically force
>> >>guest crash dump.
>> >Ah, right. (There isn't really an equivalent to this on ARM since
>> >we don't have a real NMI equivalent. So any implementation for ARM
>> >qemu would be board dependent since you could wire a watchdog up to
>> >any interrupt.)
>> >
>> >Should we try to pick a command name that says what it's supposed to
>> >do rather than how it happens to be implemented on x86 ?
>>
>> Yup, I was thinking the same thing after I sent the note above.  If
>> we call it 'force-crash-dump', we can implement it as an NMI on
>> target-i386 and potentially as something else on a different target.
>>
> NMI does not have to generate crash dump on every guest we support.
> Actually even for windows guest it does not generate one without
> tweaking registry. For all I know there is a guest that checks mail when
> NMI arrives. Lets give meaningful name, like inject-nmi, for nmi
> injection command.

I'd prefer something more generic like these:
raise /apic@fee00000:l1int
lower /i44FX-pcihost/e1000@03.0/pinD

The clumsier syntax shouldn't be a problem, since this would be a
system developer tool.

Some kind of IRQ registration would be needed for this to work without
lots of changes.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov April 7, 2011, 7:17 p.m. UTC | #14
On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
> On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Thu, Apr 07, 2011 at 01:32:50PM -0500, Anthony Liguori wrote:
> >> On 04/07/2011 01:10 PM, Peter Maydell wrote:
> >> >On 6 April 2011 20:34, Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >> >>http://publib.boulder.ibm.com/infocenter/lnxinfo/v3r0m0/index.jsp?topic=/liaai/crashdump/liaaicrashdumpnmiipmi.htm
> >> >>
> >> >>If an OS is totally hosed (spinning with interrupts disabled), and NMI can
> >> >>be used to generate a crash dump.
> >> >>
> >> >>It's a debug feature and modelling it exactly the way we are probably makes
> >> >>sense for other architectures too.  The real semantics are basically force
> >> >>guest crash dump.
> >> >Ah, right. (There isn't really an equivalent to this on ARM since
> >> >we don't have a real NMI equivalent. So any implementation for ARM
> >> >qemu would be board dependent since you could wire a watchdog up to
> >> >any interrupt.)
> >> >
> >> >Should we try to pick a command name that says what it's supposed to
> >> >do rather than how it happens to be implemented on x86 ?
> >>
> >> Yup, I was thinking the same thing after I sent the note above.  If
> >> we call it 'force-crash-dump', we can implement it as an NMI on
> >> target-i386 and potentially as something else on a different target.
> >>
> > NMI does not have to generate crash dump on every guest we support.
> > Actually even for windows guest it does not generate one without
> > tweaking registry. For all I know there is a guest that checks mail when
> > NMI arrives. Lets give meaningful name, like inject-nmi, for nmi
> > injection command.
> 
> I'd prefer something more generic like these:
> raise /apic@fee00000:l1int
> lower /i44FX-pcihost/e1000@03.0/pinD
> 
> The clumsier syntax shouldn't be a problem, since this would be a
> system developer tool.
> 
> Some kind of IRQ registration would be needed for this to work without
> lots of changes.
True. The ability to trigger any interrupt line is very useful for
debugging. I often re-implement it during debug.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 7, 2011, 9:39 p.m. UTC | #15
On 04/07/2011 01:51 PM, Gleb Natapov wrote:
> NMI does not have to generate crash dump on every guest we support.
> Actually even for windows guest it does not generate one without
> tweaking registry. For all I know there is a guest that checks mail when
> NMI arrives.

And for all we know, a guest can respond to an ACPI poweroff event by 
tweeting the star spangled banner but we still call the corresponding 
QMP command system_poweroff.

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 7, 2011, 9:41 p.m. UTC | #16
On 04/07/2011 02:17 PM, Gleb Natapov wrote:
> On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
>> On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>  wrote:
>>
>> I'd prefer something more generic like these:
>> raise /apic@fee00000:l1int
>> lower /i44FX-pcihost/e1000@03.0/pinD
>>
>> The clumsier syntax shouldn't be a problem, since this would be a
>> system developer tool.
>>
>> Some kind of IRQ registration would be needed for this to work without
>> lots of changes.
> True. The ability to trigger any interrupt line is very useful for
> debugging. I often re-implement it during debug.

And it's a good thing to have, but exposing this as the only API to do 
something as simple as generating a guest crash dump is not the 
friendliest thing in the world to do to users.

Regards,

Anthony Liguori

> --
> 			Gleb.
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov April 8, 2011, 5:54 a.m. UTC | #17
On Thu, Apr 07, 2011 at 04:39:58PM -0500, Anthony Liguori wrote:
> On 04/07/2011 01:51 PM, Gleb Natapov wrote:
> >NMI does not have to generate crash dump on every guest we support.
> >Actually even for windows guest it does not generate one without
> >tweaking registry. For all I know there is a guest that checks mail when
> >NMI arrives.
> 
> And for all we know, a guest can respond to an ACPI poweroff event
> by tweeting the star spangled banner but we still call the
> corresponding QMP command system_poweroff.
> 
Correct :) But at least system_poweroff implements ACPI poweroff as
defined by ACPI spec. NMI is not designed as core dump event and is not
used as such by majority of the guests.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov April 8, 2011, 6:04 a.m. UTC | #18
On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
> >On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
> >>On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>  wrote:
> >>
> >>I'd prefer something more generic like these:
> >>raise /apic@fee00000:l1int
> >>lower /i44FX-pcihost/e1000@03.0/pinD
> >>
> >>The clumsier syntax shouldn't be a problem, since this would be a
> >>system developer tool.
> >>
> >>Some kind of IRQ registration would be needed for this to work without
> >>lots of changes.
> >True. The ability to trigger any interrupt line is very useful for
> >debugging. I often re-implement it during debug.
> 
> And it's a good thing to have, but exposing this as the only API to
> do something as simple as generating a guest crash dump is not the
> friendliest thing in the world to do to users.
> 
Well, this is not intended to be used by regular users directly and
management can provide nicer interface for issuing NMI. But really,
my point is that NMI actually generates guest core dump in such rare
cases (only preconfigured Windows guests) that it doesn't warrant to
name command as such. Management is in much better position to implement
functionality with such name since it knows what type of guest it runs
and can tell agent to configure guest accordingly.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Blue Swirl April 8, 2011, 7:17 p.m. UTC | #19
On Fri, Apr 8, 2011 at 9:04 AM, Gleb Natapov <gleb@redhat.com> wrote:
> On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
>> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
>> >On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
>> >>On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>  wrote:
>> >>
>> >>I'd prefer something more generic like these:
>> >>raise /apic@fee00000:l1int
>> >>lower /i44FX-pcihost/e1000@03.0/pinD
>> >>
>> >>The clumsier syntax shouldn't be a problem, since this would be a
>> >>system developer tool.
>> >>
>> >>Some kind of IRQ registration would be needed for this to work without
>> >>lots of changes.
>> >True. The ability to trigger any interrupt line is very useful for
>> >debugging. I often re-implement it during debug.
>>
>> And it's a good thing to have, but exposing this as the only API to
>> do something as simple as generating a guest crash dump is not the
>> friendliest thing in the world to do to users.
>>
> Well, this is not intended to be used by regular users directly and
> management can provide nicer interface for issuing NMI. But really,
> my point is that NMI actually generates guest core dump in such rare
> cases (only preconfigured Windows guests) that it doesn't warrant to
> name command as such. Management is in much better position to implement
> functionality with such name since it knows what type of guest it runs
> and can tell agent to configure guest accordingly.

Does the management need to know about each and every debugging
oriented interface? For example, "info regs",  "info mem", "info irq"
and tracepoints?

I think giving IRQs symbolic names could solve some other problems as
well. Maybe it should be possible to connect IRQs in a configuration
file and even with command line:
-device port90,irqid=p90out -device pckbd,irqid=kbdout -device
and,in=p90out,in=kbdout,out=sreset device system_reset,in=sreset

or

 -device and,in=/i44FX-pcihost/PIIX3/i8042/out1,in=/i44FX-pcihost/PIIX3/p90/out1,out=/QEMU/system_reset
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 8, 2011, 7:32 p.m. UTC | #20
On 04/08/2011 02:17 PM, Blue Swirl wrote:
> On Fri, Apr 8, 2011 at 9:04 AM, Gleb Natapov<gleb@redhat.com>  wrote:
>> On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
>>> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
>>>> On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
>>>>> On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>    wrote:
>>>>>
>>>>> I'd prefer something more generic like these:
>>>>> raise /apic@fee00000:l1int
>>>>> lower /i44FX-pcihost/e1000@03.0/pinD
>>>>>
>>>>> The clumsier syntax shouldn't be a problem, since this would be a
>>>>> system developer tool.
>>>>>
>>>>> Some kind of IRQ registration would be needed for this to work without
>>>>> lots of changes.
>>>> True. The ability to trigger any interrupt line is very useful for
>>>> debugging. I often re-implement it during debug.
>>> And it's a good thing to have, but exposing this as the only API to
>>> do something as simple as generating a guest crash dump is not the
>>> friendliest thing in the world to do to users.
>>>
>> Well, this is not intended to be used by regular users directly and
>> management can provide nicer interface for issuing NMI. But really,
>> my point is that NMI actually generates guest core dump in such rare
>> cases (only preconfigured Windows guests) that it doesn't warrant to
>> name command as such. Management is in much better position to implement
>> functionality with such name since it knows what type of guest it runs
>> and can tell agent to configure guest accordingly.
> Does the management need to know about each and every debugging
> oriented interface? For example, "info regs",  "info mem", "info irq"
> and tracepoints?
>
> I think giving IRQs symbolic names could solve some other problems as
> well. Maybe it should be possible to connect IRQs in a configuration
> file and even with command line:
> -device port90,irqid=p90out -device pckbd,irqid=kbdout -device
> and,in=p90out,in=kbdout,out=sreset device system_reset,in=sreset

You really want devices to have properties and for the device properties 
to be discoverable.  For instance:

struct DeviceInfo
{
      .name = "and",
      .properties = {
           DEFINE_IRQ_IN(AndDevice, in[0]),
           DEFINE_IRQ_IN(AndDevice, in[1]),
           DEFINE_IRQ_OUT(AndDevice, out),
      },
};

And then you can do:

-device port90,id=port90 -device pckbd,id=pckbd \
-device and,in[0]=port90.out,in[1]=pckbd.out,id=reset_and \
-device system_reset.in=reset_and

Regards,

Anthony Liguori

> or
>
>   -device and,in=/i44FX-pcihost/PIIX3/i8042/out1,in=/i44FX-pcihost/PIIX3/p90/out1,out=/QEMU/system_reset
>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Blue Swirl April 8, 2011, 8:07 p.m. UTC | #21
On Fri, Apr 8, 2011 at 10:32 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 04/08/2011 02:17 PM, Blue Swirl wrote:
>>
>> On Fri, Apr 8, 2011 at 9:04 AM, Gleb Natapov<gleb@redhat.com>  wrote:
>>>
>>> On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
>>>>
>>>> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
>>>>>
>>>>> On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
>>>>>>
>>>>>> On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>
>>>>>>  wrote:
>>>>>>
>>>>>> I'd prefer something more generic like these:
>>>>>> raise /apic@fee00000:l1int
>>>>>> lower /i44FX-pcihost/e1000@03.0/pinD
>>>>>>
>>>>>> The clumsier syntax shouldn't be a problem, since this would be a
>>>>>> system developer tool.
>>>>>>
>>>>>> Some kind of IRQ registration would be needed for this to work without
>>>>>> lots of changes.
>>>>>
>>>>> True. The ability to trigger any interrupt line is very useful for
>>>>> debugging. I often re-implement it during debug.
>>>>
>>>> And it's a good thing to have, but exposing this as the only API to
>>>> do something as simple as generating a guest crash dump is not the
>>>> friendliest thing in the world to do to users.
>>>>
>>> Well, this is not intended to be used by regular users directly and
>>> management can provide nicer interface for issuing NMI. But really,
>>> my point is that NMI actually generates guest core dump in such rare
>>> cases (only preconfigured Windows guests) that it doesn't warrant to
>>> name command as such. Management is in much better position to implement
>>> functionality with such name since it knows what type of guest it runs
>>> and can tell agent to configure guest accordingly.
>>
>> Does the management need to know about each and every debugging
>> oriented interface? For example, "info regs",  "info mem", "info irq"
>> and tracepoints?
>>
>> I think giving IRQs symbolic names could solve some other problems as
>> well. Maybe it should be possible to connect IRQs in a configuration
>> file and even with command line:
>> -device port90,irqid=p90out -device pckbd,irqid=kbdout -device
>> and,in=p90out,in=kbdout,out=sreset device system_reset,in=sreset
>
> You really want devices to have properties and for the device properties to
> be discoverable.  For instance:
>
> struct DeviceInfo
> {
>     .name = "and",
>     .properties = {
>          DEFINE_IRQ_IN(AndDevice, in[0]),
>          DEFINE_IRQ_IN(AndDevice, in[1]),
>          DEFINE_IRQ_OUT(AndDevice, out),
>     },
> };
>
> And then you can do:
>
> -device port90,id=port90 -device pckbd,id=pckbd \
> -device and,in[0]=port90.out,in[1]=pckbd.out,id=reset_and \
> -device system_reset.in=reset_and

Exactly. Given a NAND device, we could construct entire machines from
CLI or for example co-simulate SoCs with FPGAs using cells based on
the net lists.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity April 10, 2011, 8:52 a.m. UTC | #22
On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>
> And it's a good thing to have, but exposing this as the only API to do 
> something as simple as generating a guest crash dump is not the 
> friendliest thing in the world to do to users.

nmi is a fine name for something that corresponds to a real-life nmi 
button (often labeled "NMI").

generate-crash-dump is a wrong name for something that doesn't generate 
a crash dump (the guest may not be configured for it, or it may fail to 
work).  I'd expect that to be host-side functionality.
Avi Kivity April 10, 2011, 8:57 a.m. UTC | #23
On 04/08/2011 12:39 AM, Anthony Liguori wrote:
> On 04/07/2011 01:51 PM, Gleb Natapov wrote:
>> NMI does not have to generate crash dump on every guest we support.
>> Actually even for windows guest it does not generate one without
>> tweaking registry. For all I know there is a guest that checks mail when
>> NMI arrives.
>
> And for all we know, a guest can respond to an ACPI poweroff event by 
> tweeting the star spangled banner but we still call the corresponding 
> QMP command system_poweroff.
>

A better name for it would be system_power_button.  While guests that 
blast away national anthems when the button is pressed are rare (no 
doubt to the high localization costs), it's possible to configure an OS 
to go into S3 sleep when the button is pressed (I used to do it when I 
had a desktop at home).
Markus Armbruster April 11, 2011, 7:01 a.m. UTC | #24
Avi Kivity <avi@redhat.com> writes:

> On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>>
>> And it's a good thing to have, but exposing this as the only API to
>> do something as simple as generating a guest crash dump is not the
>> friendliest thing in the world to do to users.
>
> nmi is a fine name for something that corresponds to a real-life nmi
> button (often labeled "NMI").

Agree.

> generate-crash-dump is a wrong name for something that doesn't
> generate a crash dump (the guest may not be configured for it, or it
> may fail to work).

Or the OS uses the NMI button for something else.

>                     I'd expect that to be host-side functionality.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Blue Swirl April 11, 2011, 5:15 p.m. UTC | #25
On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Avi Kivity <avi@redhat.com> writes:
>
>> On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>>>
>>> And it's a good thing to have, but exposing this as the only API to
>>> do something as simple as generating a guest crash dump is not the
>>> friendliest thing in the world to do to users.
>>
>> nmi is a fine name for something that corresponds to a real-life nmi
>> button (often labeled "NMI").
>
> Agree.

We could also introduce an alias mechanism for user friendly names, so
nmi could be used in addition of full path. Aliases could be useful
for device paths as well.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity April 12, 2011, 7:52 a.m. UTC | #26
On 04/11/2011 08:15 PM, Blue Swirl wrote:
> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>  wrote:
> >  Avi Kivity<avi@redhat.com>  writes:
> >
> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
> >>>
> >>>  And it's a good thing to have, but exposing this as the only API to
> >>>  do something as simple as generating a guest crash dump is not the
> >>>  friendliest thing in the world to do to users.
> >>
> >>  nmi is a fine name for something that corresponds to a real-life nmi
> >>  button (often labeled "NMI").
> >
> >  Agree.
>
> We could also introduce an alias mechanism for user friendly names, so
> nmi could be used in addition of full path. Aliases could be useful
> for device paths as well.

Yes.  Perhaps limited to the human monitor.
Blue Swirl April 12, 2011, 6:31 p.m. UTC | #27
On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
> On 04/11/2011 08:15 PM, Blue Swirl wrote:
>>
>> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
>>  wrote:
>> >  Avi Kivity<avi@redhat.com>  writes:
>> >
>> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>> >>>
>> >>>  And it's a good thing to have, but exposing this as the only API to
>> >>>  do something as simple as generating a guest crash dump is not the
>> >>>  friendliest thing in the world to do to users.
>> >>
>> >>  nmi is a fine name for something that corresponds to a real-life nmi
>> >>  button (often labeled "NMI").
>> >
>> >  Agree.
>>
>> We could also introduce an alias mechanism for user friendly names, so
>> nmi could be used in addition of full path. Aliases could be useful
>> for device paths as well.
>
> Yes.  Perhaps limited to the human monitor.

I'd limit all debugging commands (including NMI) to the human monitor.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Capitulino April 13, 2011, 1:08 p.m. UTC | #28
On Tue, 12 Apr 2011 21:31:18 +0300
Blue Swirl <blauwirbel@gmail.com> wrote:

> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
> > On 04/11/2011 08:15 PM, Blue Swirl wrote:
> >>
> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
> >>  wrote:
> >> >  Avi Kivity<avi@redhat.com>  writes:
> >> >
> >> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
> >> >>>
> >> >>>  And it's a good thing to have, but exposing this as the only API to
> >> >>>  do something as simple as generating a guest crash dump is not the
> >> >>>  friendliest thing in the world to do to users.
> >> >>
> >> >>  nmi is a fine name for something that corresponds to a real-life nmi
> >> >>  button (often labeled "NMI").
> >> >
> >> >  Agree.
> >>
> >> We could also introduce an alias mechanism for user friendly names, so
> >> nmi could be used in addition of full path. Aliases could be useful
> >> for device paths as well.
> >
> > Yes.  Perhaps limited to the human monitor.
> 
> I'd limit all debugging commands (including NMI) to the human monitor.

Why?
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Blue Swirl April 13, 2011, 7:56 p.m. UTC | #29
On Wed, Apr 13, 2011 at 4:08 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 12 Apr 2011 21:31:18 +0300
> Blue Swirl <blauwirbel@gmail.com> wrote:
>
>> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
>> > On 04/11/2011 08:15 PM, Blue Swirl wrote:
>> >>
>> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
>> >>  wrote:
>> >> >  Avi Kivity<avi@redhat.com>  writes:
>> >> >
>> >> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>> >> >>>
>> >> >>>  And it's a good thing to have, but exposing this as the only API to
>> >> >>>  do something as simple as generating a guest crash dump is not the
>> >> >>>  friendliest thing in the world to do to users.
>> >> >>
>> >> >>  nmi is a fine name for something that corresponds to a real-life nmi
>> >> >>  button (often labeled "NMI").
>> >> >
>> >> >  Agree.
>> >>
>> >> We could also introduce an alias mechanism for user friendly names, so
>> >> nmi could be used in addition of full path. Aliases could be useful
>> >> for device paths as well.
>> >
>> > Yes.  Perhaps limited to the human monitor.
>>
>> I'd limit all debugging commands (including NMI) to the human monitor.
>
> Why?

Do they have any real use in production environment? Also, we should
have the freedom to change the debugging facilities (for example, to
improve some internal implementation) as we want without regard to
compatibility to previous versions.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Markus Armbruster April 14, 2011, 6:41 a.m. UTC | #30
Blue Swirl <blauwirbel@gmail.com> writes:

> On Wed, Apr 13, 2011 at 4:08 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> On Tue, 12 Apr 2011 21:31:18 +0300
>> Blue Swirl <blauwirbel@gmail.com> wrote:
>>
>>> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
>>> > On 04/11/2011 08:15 PM, Blue Swirl wrote:
>>> >>
>>> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
>>> >>  wrote:
>>> >> >  Avi Kivity<avi@redhat.com>  writes:
>>> >> >
>>> >> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>>> >> >>>
>>> >> >>>  And it's a good thing to have, but exposing this as the only API to
>>> >> >>>  do something as simple as generating a guest crash dump is not the
>>> >> >>>  friendliest thing in the world to do to users.
>>> >> >>
>>> >> >>  nmi is a fine name for something that corresponds to a real-life nmi
>>> >> >>  button (often labeled "NMI").
>>> >> >
>>> >> >  Agree.
>>> >>
>>> >> We could also introduce an alias mechanism for user friendly names, so
>>> >> nmi could be used in addition of full path. Aliases could be useful
>>> >> for device paths as well.
>>> >
>>> > Yes.  Perhaps limited to the human monitor.
>>>
>>> I'd limit all debugging commands (including NMI) to the human monitor.
>>
>> Why?
>
> Do they have any real use in production environment? Also, we should
> have the freedom to change the debugging facilities (for example, to
> improve some internal implementation) as we want without regard to
> compatibility to previous versions.

For what it's worth, Lai (original poster) has been trying for many
months to get inject-nmi into QMP, and I suspect he has a really good
reason for his super-human persistence.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel P. Berrangé April 14, 2011, 9:55 a.m. UTC | #31
On Wed, Apr 13, 2011 at 10:56:21PM +0300, Blue Swirl wrote:
> On Wed, Apr 13, 2011 at 4:08 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Tue, 12 Apr 2011 21:31:18 +0300
> > Blue Swirl <blauwirbel@gmail.com> wrote:
> >
> >> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
> >> > On 04/11/2011 08:15 PM, Blue Swirl wrote:
> >> >>
> >> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
> >> >>  wrote:
> >> >> >  Avi Kivity<avi@redhat.com>  writes:
> >> >> >
> >> >> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
> >> >> >>>
> >> >> >>>  And it's a good thing to have, but exposing this as the only API to
> >> >> >>>  do something as simple as generating a guest crash dump is not the
> >> >> >>>  friendliest thing in the world to do to users.
> >> >> >>
> >> >> >>  nmi is a fine name for something that corresponds to a real-life nmi
> >> >> >>  button (often labeled "NMI").
> >> >> >
> >> >> >  Agree.
> >> >>
> >> >> We could also introduce an alias mechanism for user friendly names, so
> >> >> nmi could be used in addition of full path. Aliases could be useful
> >> >> for device paths as well.
> >> >
> >> > Yes.  Perhaps limited to the human monitor.
> >>
> >> I'd limit all debugging commands (including NMI) to the human monitor.
> >
> > Why?
> 
> Do they have any real use in production environment? Also, we should
> have the freedom to change the debugging facilities (for example, to
> improve some internal implementation) as we want without regard to
> compatibility to previous versions.

We have users of libvirt requesting that we add an API for triggering
a NMI. They want this for support in a production environment, to be
able to initiate Windows crash dumps.  We really don't want to have to
use HMP passthrough for this, instead of a proper QMP command.

More generally I don't want to see stuff in HMP, that isn't in the QMP.
We already have far too much that we have to do via HMP passthrough in
libvirt due to lack of QMP commands, to the extent that we might as
well have just ignored QMP and continued with HMP for everything.

If we want the flexibility to change the debugging commands between
releases then we should come up with a plan to do this within the
scope of QMP, not restrict them to HMP only.

Regards,
Daniel
Blue Swirl April 15, 2011, 4:37 p.m. UTC | #32
On Thu, Apr 14, 2011 at 12:55 PM, Daniel P. Berrange
<berrange@redhat.com> wrote:
> On Wed, Apr 13, 2011 at 10:56:21PM +0300, Blue Swirl wrote:
>> On Wed, Apr 13, 2011 at 4:08 PM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > On Tue, 12 Apr 2011 21:31:18 +0300
>> > Blue Swirl <blauwirbel@gmail.com> wrote:
>> >
>> >> On Tue, Apr 12, 2011 at 10:52 AM, Avi Kivity <avi@redhat.com> wrote:
>> >> > On 04/11/2011 08:15 PM, Blue Swirl wrote:
>> >> >>
>> >> >> On Mon, Apr 11, 2011 at 10:01 AM, Markus Armbruster<armbru@redhat.com>
>> >> >>  wrote:
>> >> >> >  Avi Kivity<avi@redhat.com>  writes:
>> >> >> >
>> >> >> >>  On 04/08/2011 12:41 AM, Anthony Liguori wrote:
>> >> >> >>>
>> >> >> >>>  And it's a good thing to have, but exposing this as the only API to
>> >> >> >>>  do something as simple as generating a guest crash dump is not the
>> >> >> >>>  friendliest thing in the world to do to users.
>> >> >> >>
>> >> >> >>  nmi is a fine name for something that corresponds to a real-life nmi
>> >> >> >>  button (often labeled "NMI").
>> >> >> >
>> >> >> >  Agree.
>> >> >>
>> >> >> We could also introduce an alias mechanism for user friendly names, so
>> >> >> nmi could be used in addition of full path. Aliases could be useful
>> >> >> for device paths as well.
>> >> >
>> >> > Yes.  Perhaps limited to the human monitor.
>> >>
>> >> I'd limit all debugging commands (including NMI) to the human monitor.
>> >
>> > Why?
>>
>> Do they have any real use in production environment? Also, we should
>> have the freedom to change the debugging facilities (for example, to
>> improve some internal implementation) as we want without regard to
>> compatibility to previous versions.
>
> We have users of libvirt requesting that we add an API for triggering
> a NMI. They want this for support in a production environment, to be
> able to initiate Windows crash dumps.  We really don't want to have to
> use HMP passthrough for this, instead of a proper QMP command.

OK, maybe I shouldn't be very proud of my imagination.

> More generally I don't want to see stuff in HMP, that isn't in the QMP.
> We already have far too much that we have to do via HMP passthrough in
> libvirt due to lack of QMP commands, to the extent that we might as
> well have just ignored QMP and continued with HMP for everything.
>
> If we want the flexibility to change the debugging commands between
> releases then we should come up with a plan to do this within the
> scope of QMP, not restrict them to HMP only.

If there is a need for the debugging facilities, full QMP support and
some level of compatibility is fine.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Capitulino April 26, 2011, 1:26 p.m. UTC | #33
On Thu, 21 Apr 2011 11:23:54 +0800
Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> 
> Hi, Anthony Liguori
> 
> Any suggestion?
> 
> Although all command line interfaces will be converted to to use QMP interfaces in 0.16,
> I hope inject-nmi come into QAPI earlier, 0.15.

I don't know what Anthony thinks about adding new commands like this one that
early to the new QMP interface, but adding them to current QMP will certainly
cause less code churn on your side. That's what I'd recommend for now.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori April 26, 2011, 1:29 p.m. UTC | #34
On 04/26/2011 08:26 AM, Luiz Capitulino wrote:
> On Thu, 21 Apr 2011 11:23:54 +0800
> Lai Jiangshan<laijs@cn.fujitsu.com>  wrote:
>
>>
>> Hi, Anthony Liguori
>>
>> Any suggestion?
>>
>> Although all command line interfaces will be converted to to use QMP interfaces in 0.16,
>> I hope inject-nmi come into QAPI earlier, 0.15.
>
> I don't know what Anthony thinks about adding new commands like this one that
> early to the new QMP interface, but adding them to current QMP will certainly
> cause less code churn on your side. That's what I'd recommend for now.

Yeah, sorry, this whole series has been confused in the QAPI discussion.

I did not intend for QAPI to be disruptive to current development.

As far as I can tell, the last series that was posted (before the QAPI 
post) still had checkpatch.pl issues (scripts/checkpatch.pl btw) and we 
had agreed that once that was resolved, it would come in through Luiz's 
tree.

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Luiz Capitulino April 29, 2011, 10:24 p.m. UTC | #35
On Thu, 28 Apr 2011 11:35:20 +0800
Lai Jiangshan <laijs@cn.fujitsu.com> wrote:

> 
> 
> Adds new QERR_UNSUPPORTED, converts "nmi" to "inject-nmi" and
> make it supports qmp.

Lai, unfortunately this series still have some issues (like changing
the HMP command name). I think V7 was the best submission so far, so
I decided to do this: I've incorporated your v7 patches in a new series
and fixed a few issues. I'll submit it for review.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jamie Lokier May 3, 2011, 9:54 a.m. UTC | #36
Gleb Natapov wrote:
> On Thu, Apr 07, 2011 at 04:39:58PM -0500, Anthony Liguori wrote:
> > On 04/07/2011 01:51 PM, Gleb Natapov wrote:
> > >NMI does not have to generate crash dump on every guest we support.
> > >Actually even for windows guest it does not generate one without
> > >tweaking registry. For all I know there is a guest that checks mail when
> > >NMI arrives.
> > 
> > And for all we know, a guest can respond to an ACPI poweroff event
> > by tweeting the star spangled banner but we still call the
> > corresponding QMP command system_poweroff.
> > 
> Correct :) But at least system_poweroff implements ACPI poweroff as
> defined by ACPI spec. NMI is not designed as core dump event and is not
> used as such by majority of the guests.

Imho acpi_poweroff or poweroff_button would have been a clearer name.
Or even 'sendkey poweroff' - it's just a button someone on the
keyboard on a lot of systems anyway.  Next to the email button and what
looks, on my laptop, like the play-a-tune button :-)

I put system_poweroff into some QEMU-controlling scripts once, and was
disappointed when several guests ignored it.

But it's done now.

-- Jamie

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jamie Lokier May 3, 2011, 10:01 a.m. UTC | #37
Blue Swirl wrote:
> On Fri, Apr 8, 2011 at 9:04 AM, Gleb Natapov <gleb@redhat.com> wrote:
> > On Thu, Apr 07, 2011 at 04:41:03PM -0500, Anthony Liguori wrote:
> >> On 04/07/2011 02:17 PM, Gleb Natapov wrote:
> >> >On Thu, Apr 07, 2011 at 10:04:00PM +0300, Blue Swirl wrote:
> >> >>On Thu, Apr 7, 2011 at 9:51 PM, Gleb Natapov<gleb@redhat.com>  wrote:
> >> >>
> >> >>I'd prefer something more generic like these:
> >> >>raise /apic@fee00000:l1int
> >> >>lower /i44FX-pcihost/e1000@03.0/pinD
> >> >>
> >> >>The clumsier syntax shouldn't be a problem, since this would be a
> >> >>system developer tool.
> >> >>
> >> >>Some kind of IRQ registration would be needed for this to work without
> >> >>lots of changes.
> >> >True. The ability to trigger any interrupt line is very useful for
> >> >debugging. I often re-implement it during debug.
> >>
> >> And it's a good thing to have, but exposing this as the only API to
> >> do something as simple as generating a guest crash dump is not the
> >> friendliest thing in the world to do to users.
> >>
> > Well, this is not intended to be used by regular users directly and
> > management can provide nicer interface for issuing NMI. But really,
> > my point is that NMI actually generates guest core dump in such rare
> > cases (only preconfigured Windows guests) that it doesn't warrant to
> > name command as such. Management is in much better position to implement
> > functionality with such name since it knows what type of guest it runs
> > and can tell agent to configure guest accordingly.
> 
> Does the management need to know about each and every debugging
> oriented interface? For example, "info regs",  "info mem", "info irq"
> and tracepoints?

Linux uses NMI for performance tracing, profiling, watchdog etc. so in
practice, NMI is very similar to the other IRQs.  I.e. highly guest
specific and depending on what's wired up to it.  Injecting NMI to all
CPUs at once does not make any sense for those Linux guests.

For Windows crash dumps, I think it makes sense to have a "button
wired to NMI" device, rather than inject-nmi directly, but I can see
that inject-nmi solves the intended problem quite neatly.

For Linux crash dumps, for example, there are other key combinations,
as well as watchdog devices, that can be used to trigger them.  A
virtual "button wired to GPIO/PCI-IRQ/etc." device might be quite
handy for debugging Linux guests, and would fit comfortably in a
management interface.

-- Jamie
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 372bef4..8aea56c 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -743,7 +743,7 @@  ETEXI
         .args_type  = "cpu_index:i",
         .params     = "cpu",
         .help       = "inject an NMI on the given CPU",
-        .mhandler.cmd = do_inject_nmi,
+        .mhandler.cmd = do_inject_nmi_cpu,
     },
 #endif
 STEXI
diff --git a/monitor.c b/monitor.c
index 22ae3bb..aebcc0c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2555,7 +2555,7 @@  static void do_wav_capture(Monitor *mon, const QDict *qdict)
 #endif
 
 #if defined(TARGET_I386)
-static void do_inject_nmi(Monitor *mon, const QDict *qdict)
+static void do_inject_nmi_cpu(Monitor *mon, const QDict *qdict)
 {
     CPUState *env;
     int cpu_index = qdict_get_int(qdict, "cpu_index");
@@ -2566,6 +2566,22 @@  static void do_inject_nmi(Monitor *mon, const QDict *qdict)
             break;
         }
 }
+
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    CPUState *env;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu)
+        cpu_interrupt(env, CPU_INTERRUPT_NMI);
+
+    return 0;
+}
+#else
+static int do_inject_nmi(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    qerror_report(QERR_UNSUPPORTED);
+    return -1;
+}
 #endif
 
 static void do_info_status_print(Monitor *mon, const QObject *data)
diff --git a/qmp-commands.hx b/qmp-commands.hx
index df40a3d..51f479e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -430,6 +430,35 @@  Example:
 EQMP
 
     {
+        .name       = "inject-nmi",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Inject an NMI on guest.\n"
+                      "Returns \"Unsupported\" error when the guest does"
+                      "not support NMI injection",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_inject_nmi,
+    },
+
+SQMP
+inject-nmi
+----------
+
+Inject an NMI on guest.
+
+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.
+
+EQMP
+
+    {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
         .params     = "[-d] [-b] [-i] uri",