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

[Alexander Graf <agraf@suse.de>]

On 06/11/2014 12:47 PM, Paolo Bonzini wrote:
> Il 11/06/2014 10:42, Alexander Graf ha scritto:
>>
>> On 11.06.14 10:37, Paolo Bonzini wrote:
>>> Il 11/06/2014 10:29, Alexander Graf ha scritto:
>>>>>
>>>>> But right now inject-nmi was a CPU-specific interface and whoever
>>>>> needs something different will have to find a way.
>>>>>
>>>>> You could argue that Alexey does need something different thanks to
>>>>> his need to inject the NMI on all CPUs.
>>>>>
>>>>> What about an NMIMonitorHandler interface that takes a Monitor*, and
>>>>> then you iterate on all of /machine looking for implementors of the
>>>>> interface?  Then -M mac99 can add it to the MPIC, -M spapr can just
>>>>> ignore the Monitor*, and i386/s390 can look at the current CPU.
>>>>
>>>> I think modeling a button is easier and closer to what you'd actually
>>>> get on real hardware.
>>>
>>> What is a button?  It certainly doesn't have any evdev support in 
>>> Linux.
>>
>> It's a device that links a qemu_irq line and implements an interface to
>> call it.
>
> That seems awfully overengineered.

On PPC every input line to a CPU is a qemu_irq. I don't see how a 
linking piece between this qemu_irq object and a command line interface 
is overengineered. Whether the linking piece is "a device you look for 
and call a function in" or "a machine callback you call" is an 
implementation detail IMHO.

>
>>> The NMI button is just something that closes a circuit.  Whoever is
>>> attached to that circuit reacts, in QEMU that's whoever implements an
>>> NMIMonitorHandler.  The Monitor* is just for backwards HMP 
>>> compatibility.
>>
>> I think the only major difference in thinking we have here is that I
>> don't think it's necessary to maintain HMP compatibility for this debug
>> feature that maybe 2 people in the world ever used ;).
>
> Ok, I can buy removing the support for CPU != 0.  But still, the 
> overengineering remains.
>
> If Alexey needs to trigger the NMI on all CPUs, simply moving the new 
> method from CPU to Machine, and blindly using first_cpu in s390 and 
> x86 makes the most sense.

Yes :).


Alex