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

[Eduardo Habkost]

On Sat, Dec 12, 2015 at 09:15:46PM +0800, 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.
> 
> Signed-off-by: Gonglei <address@hidden>
> ---
>  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!
> ---
>  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/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;

Please, not another global variable. Doesn't this belong to
struct RTCState or APICCommonState?

-- 
Eduardo