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

[Paolo Bonzini]

On 15/12/2015 19:53, Radim Krcmar wrote:
> 2015-12-15 05:43-0500, Paolo Bonzini:
>>> Hi Paolo,
>>>
>>> /* for KVM_GET/SET_VCPU_EVENTS */
>>> struct kvm_vcpu_events {
>>>  ...
>>> struct {
>>>             __u8 injected;
>>>             __u8 pending;
>>>             __u8 masked;
>>>             __u8 pad;
>>>     } nmi;
>>>  ...
>>>
>>> I found that the nmi.masked property does these enable or disable NMI jobs.
>>> So, I think we don't need to add a new bit. Right?
>>
>> nmi.masked says whether the CPU is accepting the NMIs, and is cleared
>> by the next IRET instruction.  This is a different thing; it probably
>> shouldn't affect NMI IPIs, and it definitely should remain set until
>> cleared via the RTC.  So it should be something like
>>
>>     _u8 external_nmi_disabled;
>>
>> or similar.
>>
>> *However* I found this in the ICH9 datasheet:
>>
>>     The ICH9's I/O APIC can only send interrupts due to interrupts which
>>     do not include SMI, NMI or INIT. This means that in IA-32/Intel ® 64
>>     based platforms, Front Side Bus interrupt message format delivery modes
>>     010 (SMI/PMI), 100 (NMI), and 101 (INIT) as indicated in this section,
>>     must not be used and is not supported.
>>
>> In theory the PIIX4 could deliver such messages, but perhaps we could
>> disable them in the KVM IOAPIC.  If we do this, there is no need for a
>> change to struct kvm_vcpu_events, because all external NMI sources will
>> be in userspace.
>>
>> Radim, what do you think?
> 
> I looked at the 440fx, piix, and 82083aa(ioapic) datasheets and the
> NMI_EN bit doesn't seem to be propagated into the IOAPIC.
> The IOAPIC datasheet doesn't mention a thing about NMI masking and PIIX4
> generates NMI on SERR# or IOCHK# so it seems that the NMI_EN feature
> only changes the behavior of those two ...
> 
> I think it's best to do nothing in KVM.

Then Gonglei's patch (apart from the issues that Eduardo pointed out) is
fine.

Paolo