[Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Eduardo Habkost]

Hi,

I was looking at the monitor code handling the "current CPU", and
noticed that qmp_inject_nmi() looks suspicious: it is a QMP
command, but uses monitor_get_cpu_index().

In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is
used at:
* hmp_inject_nmi()
* ipmi_do_hw_op() (IPMI_SEND_NMI operation)

This confused me, so I would like to know:

1) What exactly "default CPU" is supposed to mean in the
   "inject-nmi" QMP command documentation?
2) To which CPU(s) are NMIs supposed to be sent when triggered by
   IPMI messages? I don't know how to test the IPMI code, but it
   looks like it will crash if QEMU runs without any monitor.

-- 
Eduardo

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Corey Minyard]

On 09/21/2016 03:20 PM, Eduardo Habkost wrote:
> Hi,
>
> I was looking at the monitor code handling the "current CPU", and
> noticed that qmp_inject_nmi() looks suspicious: it is a QMP
> command, but uses monitor_get_cpu_index().
>
> In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is
> used at:
> * hmp_inject_nmi()
> * ipmi_do_hw_op() (IPMI_SEND_NMI operation)
>
> This confused me, so I would like to know:
>
> 1) What exactly "default CPU" is supposed to mean in the
>     "inject-nmi" QMP command documentation?
> 2) To which CPU(s) are NMIs supposed to be sent when triggered by
>     IPMI messages? I don't know how to test the IPMI code, but it
>     looks like it will crash if QEMU runs without any monitor.
>
It doesn't matter which CPU it goes to.

I haven't tested without a monitor, so I'm not sure.  Does
another interface into the NMI code need to be added?

-corey

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Eduardo Habkost]

On Wed, Sep 21, 2016 at 03:38:25PM -0500, Corey Minyard wrote:
> On 09/21/2016 03:20 PM, Eduardo Habkost wrote:
> > Hi,
> > 
> > I was looking at the monitor code handling the "current CPU", and
> > noticed that qmp_inject_nmi() looks suspicious: it is a QMP
> > command, but uses monitor_get_cpu_index().
> > 
> > In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is
> > used at:
> > * hmp_inject_nmi()
> > * ipmi_do_hw_op() (IPMI_SEND_NMI operation)
> > 
> > This confused me, so I would like to know:
> > 
> > 1) What exactly "default CPU" is supposed to mean in the
> >     "inject-nmi" QMP command documentation?
> > 2) To which CPU(s) are NMIs supposed to be sent when triggered by
> >     IPMI messages? I don't know how to test the IPMI code, but it
> >     looks like it will crash if QEMU runs without any monitor.
> > 
> It doesn't matter which CPU it goes to.

OK, so in the case of IPMI we can make it send the NMI to the
first CPU.

> I haven't tested without a monitor, so I'm not sure.  Does
> another interface into the NMI code need to be added?

There's another interface, already: nmi_monitor_handle() already
gets a cpu_index argument and doesn't depend on the monitor code.
We could change the IPMI code to call
nmi_monitor_handle(first_cpu->cpu_index) directly.

In the case of the inject-nmi QMP command, I need to understand
what "default CPU" is supposed to mean in the inject-nmi
documentation. Maybe it can be changed to use the first CPU, too
(that's probably the existing behavior because there's no way to
change cur_mon->mon_cpu in a QMP monitor).

-- 
Eduardo

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Corey Minyard]

On 09/22/2016 01:42 PM, Eduardo Habkost wrote:
> On Wed, Sep 21, 2016 at 03:38:25PM -0500, Corey Minyard wrote:
>> On 09/21/2016 03:20 PM, Eduardo Habkost wrote:
>>> Hi,
>>>
>>> I was looking at the monitor code handling the "current CPU", and
>>> noticed that qmp_inject_nmi() looks suspicious: it is a QMP
>>> command, but uses monitor_get_cpu_index().
>>>
>>> In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is
>>> used at:
>>> * hmp_inject_nmi()
>>> * ipmi_do_hw_op() (IPMI_SEND_NMI operation)
>>>
>>> This confused me, so I would like to know:
>>>
>>> 1) What exactly "default CPU" is supposed to mean in the
>>>      "inject-nmi" QMP command documentation?
>>> 2) To which CPU(s) are NMIs supposed to be sent when triggered by
>>>      IPMI messages? I don't know how to test the IPMI code, but it
>>>      looks like it will crash if QEMU runs without any monitor.
>>>
>> It doesn't matter which CPU it goes to.
> OK, so in the case of IPMI we can make it send the NMI to the
> first CPU.
>
>> I haven't tested without a monitor, so I'm not sure.  Does
>> another interface into the NMI code need to be added?
> There's another interface, already: nmi_monitor_handle() already
> gets a cpu_index argument and doesn't depend on the monitor code.
> We could change the IPMI code to call
> nmi_monitor_handle(first_cpu->cpu_index) directly.

Ok, I'll make a change for this, unless we decide to fix it
another way.

> In the case of the inject-nmi QMP command, I need to understand
> what "default CPU" is supposed to mean in the inject-nmi
> documentation. Maybe it can be changed to use the first CPU, too
> (that's probably the existing behavior because there's no way to
> change cur_mon->mon_cpu in a QMP monitor).
>
I looked through is a bit, and the only place I found it was used was
the x390 code.

If we remove the CPU index from this, then the IPMI device can
keep the same interface.

Thanks,

-corey

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Eduardo Habkost]

On Thu, Sep 22, 2016 at 02:49:35PM -0500, Corey Minyard wrote:
> On 09/22/2016 01:42 PM, Eduardo Habkost wrote:
[...]
> > In the case of the inject-nmi QMP command, I need to understand
> > what "default CPU" is supposed to mean in the inject-nmi
> > documentation. Maybe it can be changed to use the first CPU, too
> > (that's probably the existing behavior because there's no way to
> > change cur_mon->mon_cpu in a QMP monitor).
> > 
> I looked through is a bit, and the only place I found it was used was
> the x390 code.
> 
> If we remove the CPU index from this, then the IPMI device can
> keep the same interface.

Well, the existing behavior is to use the first CPU, so I will
just keep the existing behavior but remove the
monitor_get_cpu_index() call from qmp_inject_nmi(). Then the IPMI
code can stay the same.

-- 
Eduardo

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Eduardo Habkost]

On Thu, Sep 22, 2016 at 02:49:35PM -0500, Corey Minyard wrote:
> On 09/22/2016 01:42 PM, Eduardo Habkost wrote:
[...]
> > In the case of the inject-nmi QMP command, I need to understand
> > what "default CPU" is supposed to mean in the inject-nmi
> > documentation. Maybe it can be changed to use the first CPU, too
> > (that's probably the existing behavior because there's no way to
> > change cur_mon->mon_cpu in a QMP monitor).
> > 
> I looked through is a bit, and the only place I found it was used was
> the x390 code.

s390 maintainers: is the ability to send a NMI to a specific CPU
really used in s390? The only way to use it today is to use HMP,
so I guess no management interface really supports it.

s390-ccw is the only machine that uses the cpu_index parameter.
If we remove it, it will allow us to simplify the code.
Otherwise, we will need to extend the inject-nmi command to
accept a "cpu" parameter.

-- 
Eduardo

Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)

[Christian Borntraeger]

On 09/27/2016 10:51 PM, Eduardo Habkost wrote:
> On Thu, Sep 22, 2016 at 02:49:35PM -0500, Corey Minyard wrote:
>> On 09/22/2016 01:42 PM, Eduardo Habkost wrote:
> [...]
>>> In the case of the inject-nmi QMP command, I need to understand
>>> what "default CPU" is supposed to mean in the inject-nmi
>>> documentation. Maybe it can be changed to use the first CPU, too
>>> (that's probably the existing behavior because there's no way to
>>> change cur_mon->mon_cpu in a QMP monitor).
>>>
>> I looked through is a bit, and the only place I found it was used was
>> the x390 code.
> 
> s390 maintainers: is the ability to send a NMI to a specific CPU
> really used in s390? The only way to use it today is to use HMP,
> so I guess no management interface really supports it.
> 
> s390-ccw is the only machine that uses the cpu_index parameter.
> If we remove it, it will allow us to simplify the code.
> Otherwise, we will need to extend the inject-nmi command to
> accept a "cpu" parameter.
> 

Doing it always on the first CPU should be ok for Linux guests.
This is wired up to a PSW restart interrupt, which has the same
handler on all CPUs. Originally it was wired up on the real 
old irons as a button, so I assume from a historical perspective
it is ok to not be able to specify the cpu number. Under z/VM it is 
possible to specify the CPU though. I will try to find out
if there is a real use case for specifying the CPU.

Christian