> > 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. > 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.
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c index e770e74..c019ec0 100644 --- a/hw/timer/mc146818rtc.c +++ b/hw/timer/mc146818rtc.c @@ -67,6 +67,7 @@ typedef struct RTCState { MemoryRegion io; uint8_t cmos_data[128]; uint8_t cmos_index; + bool external_nmi_disabled; int32_t base_year; uint64_t base_rtc; uint64_t last_update; @@ -89,6 +90,8 @@ typedef struct RTCState { QLIST_ENTRY(RTCState) link; } RTCState; +static RTCState *rtc_global_state; + static void rtc_set_time(RTCState *s); static void rtc_update_time(RTCState *s); static void rtc_set_cmos(RTCState *s, const struct tm *tm); @@ -383,6 +386,11 @@ static void rtc_update_timer(void *opaque) check_update_timer(s); } +bool external_nmi_is_disabled() +{ + return rtc_global_state->external_nmi_disabled; +} + static void cmos_ioport_write(void *opaque, hwaddr addr, uint64_t data, unsigned size) { @@ -397,9 +405,9 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, * by applications, BIOS, and even the operating system since it is * used to block NMI assertions when sensitive code is executing. */ - nmi_disabled = !!(data & NMI_DISABLE_BIT); + s->external_nmi_disabled = !!(data & NMI_DISABLE_BIT); CMOS_DPRINTF("cmos: nmi_disabled=%s\n", - nmi_disabled ? "true" : "false"); + s->external_nmi_disabled ? "true" : "false"); s->cmos_index = data & 0x7f; } else { CMOS_DPRINTF("cmos: write index=0x%02x val=0x%02" PRIx64 "\n", @@ -930,6 +938,8 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq) } QLIST_INSERT_HEAD(&rtc_devices, s, link); + rtc_global_state = s; + return isadev; } diff --git a/include/hw/timer/mc146818rtc.h b/include/hw/timer/mc146818rtc.h index eaf6497..761eba2 100644 --- a/include/hw/timer/mc146818rtc.h +++ b/include/hw/timer/mc146818rtc.h @@ -9,5 +9,6 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq); void rtc_set_memory(ISADevice *dev, int addr, int val); int rtc_get_memory(ISADevice *dev, int addr); +bool external_nmi_is_disabled(); #endif /* !MC146818RTC_H */ And one more thing is we need to keep the property in vmstate for supporting live Migration. Am I right? Thanks! Regards, -Gonglei