From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9Ayh-0006in-BT for qemu-devel@nongnu.org; Wed, 16 Dec 2015 07:14:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9Ayb-0004rn-AU for qemu-devel@nongnu.org; Wed, 16 Dec 2015 07:14:43 -0500 Received: from mail-wm0-x22a.google.com ([2a00:1450:400c:c09::22a]:38384) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9Ayb-0004r4-2U for qemu-devel@nongnu.org; Wed, 16 Dec 2015 07:14:37 -0500 Received: by mail-wm0-x22a.google.com with SMTP id l126so35330491wml.1 for ; Wed, 16 Dec 2015 04:14:37 -0800 (PST) Sender: Paolo Bonzini References: <1449926146-14828-1-git-send-email-arei.gonglei@huawei.com> <566E9259.2010404@redhat.com> <33183CC9F5247A488A2544077AF19020B02B4533@SZXEMA503-MBS.china.huawei.com> <566EBB34.5030205@redhat.com> <33183CC9F5247A488A2544077AF19020B02B45BB@SZXEMA503-MBS.china.huawei.com> <566EC61A.7090407@redhat.com> <33183CC9F5247A488A2544077AF19020B02B4BE3@SZXEMA503-MBS.china.huawei.com> <145192308.40660604.1450176186593.JavaMail.zimbra@redhat.com> <20151215185335.GF17283@potion.brq.redhat.com> <5671261F.4010305@redhat.com> <33183CC9F5247A488A2544077AF19020B02B5191@SZXEMA503-MBS.china.huawei.com> From: Paolo Bonzini Message-ID: <567155AA.9040906@redhat.com> Date: Wed, 16 Dec 2015 13:14:34 +0100 MIME-Version: 1.0 In-Reply-To: <33183CC9F5247A488A2544077AF19020B02B5191@SZXEMA503-MBS.china.huawei.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] rtc: introduce nmi disable bit handler for cmos List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" , Radim Krcmar Cc: "qemu-devel@nongnu.org" , "kevin@koconnor.net" , "Huangpeng (Peter)" , "ehabkost@redhat.com" , "rth@twiddle.net" 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 bridge; you should not need to touch anything except the RTC and the ISA bridges, in particular not the APICs. 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 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. The nmi_en status and NMI latch status must be migrated in a new subsection of the ISA bridges. Fourth, the PC machines should use qdev_get_gpio_in_named to retrieve the qemu_irq from the ISA bridges, and pass it to pc_basic_device_init. I may have messed up some steps, but this is the basic idea. Paolo