Re: [Qemu-devel] [PATCH] rtc: introduce nmi disable bit handler for cmos

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/12/2015 08:17, Gonglei (Arei) wrote:
>> On 16/12/2015 11:28, Gonglei (Arei) wrote:
>>> I'll move the global nmi_disabled into RTCState, then I have to add a global
>> RTCState
>>> Variable so that other C files can use the rtc_state->external_nmi_disabled.
>>
>> Hmm, I think it should be done differently.  This is a layering
>> violation, the NMI_EN is essentially a pin (qemu_irq) between the ISA
>> bridges and the RTC.  The NMI "button" is also a component of the ISA
> 
> So, you mean the NMI_EN can only control NMI injection came from ISA bridge?
> What's this NMI "button" mean? 

The NMI command in the monitor is a "virtual NMI button".

>> bridge; you should not need to touch anything except the RTC and the ISA
>> bridges, in particular not the APICs.
>>
> Currently, the qmp command "inject-nmi" doesn't pass ISA bridge. How
> do we address this situation?

That's step two below: make the ISA bridges implement NMIState.

>> First, you need to add a qemu_irq argument to rtc_init. The RTC can
>> raise/lower the IRQ on writes to port 0x70.
>>
>> Second, make the ISA bridges implement NMIState, where the
>> implementation of NMIState is similar to inject_nmi in hw/core/nmi.c:
>>
>>     CPU_FOREACH(cs) {
>> 	X86CPU *cpu = X86_CPU(cs);
>>
>> 	if (!cpu->apic_state) {
>> 	    cpu_interrupt(cs, CPU_INTERRUPT_NMI);
>> 	} else {
>> 	    apic_deliver_nmi(cpu->apic_state);
>> 	}
>>     }
>>
>> Third, the ISA bridges (hw/isa/piix4.c and hw/isa/lpc_ich9.c) need to
> 
> We don't use hw/isa/piix4.c but hw/pci-host/piix.c in x86 target. Right?

Right, I said I had certainly messed up something. :)

>> export a qemu_irq for nmi_en IRQ (e.g. using qdev_init_gpio_in_named),
>> and you should modify the ISA bridge's implementation of NMIState to
>> latch the NMI if you send one while NMIs are disabled.  The nmi_en IRQ
>> can also trigger an NMI when nmi_en is enabled and an NMI was latched.
> 
> Sorry, I'm a bit confused. The nmi_en can trigger an NMI? Isn't a flag bit which
> can enable/disable the NMI switch?

Suppose an NMI was injected with the monitor while nmi_en was disabled.
 The NMI is then latched, and triggered when you enable NMIs again with
nmi_en.

Thanks,

Paolo