> On 20 Mar 2024, at 13:00, Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of
> any links or attachments, and do not enable macros.
>
> On Wed, 20 Mar 2024 at 11:20, Philippe Mathieu-Daudé <phi...@linaro.org>
> wrote:
>>
>> On 20/2/24 16:19, Thomas Huth wrote:
>>> On 20/02/2024 16.08, Philippe Mathieu-Daudé wrote:
>>>> Have s390x always deliver NMI to the first CPU,
>>>> remove the @cpu_index argument from handler,
>>>> rename API as nmi_trigger() (not monitor specific).
>>>
>>> Could you please add some rationale here why this is needed / desired?
>>
>> I'm not sure it is desired... I'm trying to get the NMI delivery
>> working in heterogeneous machine, but now I'm wondering whether
>> hw/core/nmi.c was designed with that in mind or likely not.
>>
>> I suppose in a complex machine you explicitly wire IRQ lines such
>> NMI, so they are delivered to a particular INTC or CPU core, and
>> there is no "broadcast this signal to all listeners registered
>> for NMI events".
>
> I think in a complex heterogenous machine you do want the
> monitor NMI command to do something sensible, but the
> definition of "sensible" is going to be machine-specific:
> probably it will be "raise NMI in some way on some core in
> the main application processor cluster", and it's the machine
> model that's going to know what "sensible" is for that machine.
>
> The current hw/core/nmi.c code is a bit odd because it's partly
> working with a cpu_index and partly not: the code passes cpu_index
> around, but in practice for the QMP command the user can't set
> which CPU to operate on, and for everything except s390 the
> implementation doesn't care anyway. My impression from the IRC
> discussion is that it's not really necessary for the S390 that
> the monitor user be able to specify which CPU to NMI (and in any
> case you can only do that from the HMP command, not the QMP
> command, AIUI), so getting rid of that weird inconsistency makes
> sense to me: and that's what this patchset is doing.
>
> What NMI probably ought to be is board-specific: so it's like
> having some notional front panel switch labeled "NMI", and the
Do the youngsters of today know what one of those is ?
:-)
Is there a reason for not using a GPIO interface for ’NMI’ - wiring it up like
any other wire?
Cheers
Mark.
> board gets to decide what that means (which is usually going to be
> "send some NMI like interrupt to the first CPU in the main cluster",
> but could be something else). It doesn't need to be like a
> front panel switch with a rotary-selector for 'pick a CPU'
> plus a button for "send NMI to that CPU". In fact we're quite
> close to "it's a board thing" already, because almost every
> implementation of the TYPE_NMI interface is actually a machine
> model. (The exceptions are hw/intc/m68k_irqc.c,
> hw/m68k/q800-glue.c and hw/misc/macio/gpio.c.)
>
> So I think that:
> * we should indeed drop the cpu_index stuff, per this patch:
> it's unnecessary cruft we don't really use
> * we should look at whether the three classes listed above
> which implement TYPE_NMI on a non-machine-model are really
> the right way to do that, i.e. whether it would be a lot of
> effort to effectively switch to having nmi_monitor_handler
> be a simple method on MachineClass. Not walking the QOM
> tree would make the NMI infrastructure rather simpler.
> (But I just looked at the macio case, and it's inside a
> PCI device, so at best that's a bunch of clunky plumbing.)
> * failing that, we should look at whether we should really
> continue to walk the whole QOM tree calling methods on every
> TYPE_NMI object, or whether we can say "once we've found one
> implementation we're done". This also depends on those three
> non-MachineClass implementations, because obviously there's
> only ever one MachineClass object in the system. This is
> kind of useful for heterogenous boards which use the m68k
> or ppc devices listed above (seems highly unlikely), but it
> would mean you can override the default "those objects handle
> NMI" by having your heterogenous board implement TYPE_NMI,
> and then since it's earlier in the QOM tree that will be
> the method called, not the ones on specific devices.
> (This one I think we can easily do -- my quick check suggests
> that TYPE_M68K_IRQ is only used in the m68k virt board,
> TYPE_GLUE is only used in the m68k q800 board, and
> TYPE_MACIO_GPIO is only used in the ppc mac99 board. So in
> fact in all cases there's only ever one TYPE_NMI interface
> present in the system.)
>
> The last two aren't blockers for heterogenous-system work,
> though: they just seem to me like nice cleanup of this interface.
>
> thanks
> -- PMM
