On 12/12/2015 14:15, Gonglei wrote: > The Non-Maskable Interrupt (NMI) Enable bit is 0x80 bit of > Port 0x70 (and its aliases). This bit must be 0b to enable > the hardware chipset to send a Non-Maskable Interrupt. When > set to a 1b, NMI's are disabled. This bit is commonly accessed > by applications, BIOS, and even the operating system since it is > used to block NMI assertions when sensitive code is executing. > > Currently, QEMU do no not handle the bit, means Qemu cannot > block NMI occur, sometimes maybe cause a race between the CMOS > read/write and the NMI handler. If you are setting the CMOS clock > or reading CMOS RAM and an NMI occurs, Bad values could be written > to or read from the CMOS RAM, or the NMI operation might not > occur correctly. > > This patch introduce nmi disable bit handler to fix the problem > and make the emulated CMOS like the real hardware.
I think that this only works with -machine kernel_irqchip=off, however. You would have to add a new bit to struct kvm_vcpu_events, which could for example replace nmi.pad. > Please refer to: > https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg00616.html > > Note: We can't reproduce the problem, what a pity :( > I holp the patch can fix it. Please review, thanks! The effect of the patch could be tested with kvm-unit-tests. Thanks, Paolo > hw/i386/kvm/apic.c | 4 +++- > hw/timer/mc146818rtc.c | 11 +++++++++++ > include/hw/timer/mc146818rtc_regs.h | 3 +++ > include/sysemu/sysemu.h | 1 + > target-i386/kvm.c | 4 ++-- > vl.c | 1 + > 6 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c > index 5b47056..deea49f 100644 > --- a/hw/i386/kvm/apic.c > +++ b/hw/i386/kvm/apic.c > @@ -12,6 +12,7 @@ > #include "hw/i386/apic_internal.h" > #include "hw/pci/msi.h" > #include "sysemu/kvm.h" > +#include "sysemu/sysemu.h" > > static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic, > int reg_id, uint32_t val) > @@ -132,7 +133,8 @@ static void do_inject_external_nmi(void *data) > cpu_synchronize_state(cpu); > > lvt = s->lvt[APIC_LVT_LINT1]; > - if (!(lvt & APIC_LVT_MASKED) && ((lvt >> 8) & 7) == APIC_DM_NMI) { > + if (!nmi_disabled && (lvt & APIC_LVT_MASKED) > + && ((lvt >> 8) & 7) == APIC_DM_NMI) { > ret = kvm_vcpu_ioctl(cpu, KVM_NMI); > if (ret < 0) { > fprintf(stderr, "KVM: injection failed, NMI lost (%s)\n", > diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c > index a9f0efd..aaa8b4e 100644 > --- a/hw/timer/mc146818rtc.c > +++ b/hw/timer/mc146818rtc.c > @@ -389,6 +389,17 @@ static void cmos_ioport_write(void *opaque, hwaddr addr, > RTCState *s = opaque; > > if ((addr & 1) == 0) { > + /* > + * The Non-Maskable Interrupt (NMI) Enable bit is 0x80 bit of > + * Port 0x70 (and its aliases). This bit must be 0b to enable > + * the hardware chipset to send a Non-Maskable Interrupt. When > + * set to a 1b, NMI's are disabled. This bit is commonly accessed > + * 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); > + CMOS_DPRINTF("cmos: nmi_disabled=%s\n", > + nmi_disabled ? "true" : "false"); > s->cmos_index = data & 0x7f; > } else { > CMOS_DPRINTF("cmos: write index=0x%02x val=0x%02" PRIx64 "\n", > diff --git a/include/hw/timer/mc146818rtc_regs.h > b/include/hw/timer/mc146818rtc_regs.h > index ccdee42..175249f 100644 > --- a/include/hw/timer/mc146818rtc_regs.h > +++ b/include/hw/timer/mc146818rtc_regs.h > @@ -64,4 +64,7 @@ > #define REG_C_AF 0x20 > #define REG_C_MASK 0x70 > > +/* PORT_CMOS_INDEX nmi disable bit */ > +#define NMI_DISABLE_BIT 0x80 > + > #endif > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 3bb8897..a5b2342 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -177,6 +177,7 @@ extern uint8_t qemu_extra_params_fw[2]; > extern QEMUClockType rtc_clock; > extern const char *mem_path; > extern int mem_prealloc; > +extern bool nmi_disabled; > > #define MAX_NODES 128 > #define NUMA_NODE_UNASSIGNED MAX_NODES > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 6dc9846..abbd65b 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -2456,9 +2456,9 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run > *run) > CPUX86State *env = &x86_cpu->env; > int ret; > > - /* Inject NMI */ > + /* Inject NMI Or SMI */ > if (cpu->interrupt_request & (CPU_INTERRUPT_NMI | CPU_INTERRUPT_SMI)) { > - if (cpu->interrupt_request & CPU_INTERRUPT_NMI) { > + if (!nmi_disabled && (cpu->interrupt_request & CPU_INTERRUPT_NMI)) { > qemu_mutex_lock_iothread(); > cpu->interrupt_request &= ~CPU_INTERRUPT_NMI; > qemu_mutex_unlock_iothread(); > diff --git a/vl.c b/vl.c > index 4211ff1..ff5b06f 100644 > --- a/vl.c > +++ b/vl.c > @@ -185,6 +185,7 @@ bool boot_strict; > uint8_t *boot_splash_filedata; > size_t boot_splash_filedata_size; > uint8_t qemu_extra_params_fw[2]; > +bool nmi_disabled; > > int icount_align_option; > >
