From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id CE72A81C91 for ; Mon, 14 Nov 2016 17:19:15 -0800 (PST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP; 14 Nov 2016 17:19:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,640,1473145200"; d="scan'208";a="901401657" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 14 Nov 2016 17:19:19 -0800 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 17:19:19 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 17:19:19 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.142]) with mapi id 14.03.0248.002; Tue, 15 Nov 2016 09:19:17 +0800 From: "Fan, Jeff" To: Laszlo Ersek , Paolo Bonzini CC: "edk2-devel@ml01.01.org" , "Yao, Jiewen" Thread-Topic: [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path Thread-Index: AQHSO97k043lSFBUy0mbX04flay71KDTq3eAgAMz0ICAAMHjAIAACSgAgAAeoYCAAAhqAIAABPsAgAAJRwCAAGZ0AIAAAaeAgABf0gCAAJmVcA== Date: Tue, 15 Nov 2016 01:19:16 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2DD0A3@shsmsx102.ccr.corp.intel.com> References: <20161111054545.19616-1-jeff.fan@intel.com> <542CF652F8836A4AB8DBFAAD40ED192A4A2DB4F5@shsmsx102.ccr.corp.intel.com> <00b6828b-78c5-af4f-ab98-de4460b1b8ec@redhat.com> <4dc14e5c-9b43-4338-c7a5-9750e8a9547a@redhat.com> <3e61ffc4-9eaf-0015-11a7-e2d698624acb@redhat.com> <648314a4-6c17-7c88-7e47-98c4de95fe2d@redhat.com> <7bca5070-be51-aa99-bdb6-9fcc03086430@redhat.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTM4OGI2YmQtNGNmYi00MWIxLTlhMzUtYTBlMjAxOGI2MzExIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkF1NHl1eE1JQUgrMUg2ZzF6eW8rUFlTTWNSeEFDdnR1dVFiV3BKbVQ0OTQ9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Nov 2016 01:19:16 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, Have you tried to decrease PcdCpuInitIpiDelayInMicroSeconds? For PCD PcdCpuInitIpiDelayInMicroSeconds before two SIPIs, we introduced th= is PCD is just to do customization. MicroSecondDelay (PcdGet32(PcdCpuInitIpiDelayInMicroSeconds)); Per our experience, we could decrease this PCD value to 10us (microsecond)= on some platforms/processors. You may try it. Thanks! Jeff -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Tuesday, November 15, 2016 7:56 AM To: Paolo Bonzini; Fan, Jeff Cc: edk2-devel@ml01.01.org; Yao, Jiewen Subject: Re: [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 pat= h On 11/14/16 19:13, Paolo Bonzini wrote: >=20 >=20 > On 14/11/2016 19:07, Laszlo Ersek wrote: >> On 11/14/16 13:00, Paolo Bonzini wrote: >>> >>> >>> On 14/11/2016 12:27, Laszlo Ersek wrote: >>>> Well... >>>> >>>> http://lists.nongnu.org/archive/html/qemu-devel/2015-10/msg05658.ht >>>> ml=20 >>>> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg00125.ht >>>> ml=20 >>>> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg00563.ht >>>> ml >>>> >>>> Are you suggesting that I resurrect this patch? That would be my=20 >>>> pleasure. Please say yes. >>> >>> It's hard to say no when someone has written the code already. :) >> >> Thanks. I refreshed both patches (OVMF and QEMU -- no code changes=20 >> just more precise commit messages). Unfortunately, quite a few things=20 >> seem broken, although these patches worked a year ago. >> >> My QEMU base commit is current master 83c83f9a5266. My host kernel is=20 >> 3.10.0-514.el7.x86_64. >> >> *** So, when I test these two patches, based on edk2 master (no=20 >> on-list patches), Ia32 target, my boot hangs (spins) with the log ending= in: >> >>> SmmInstallProtocolInterface: [EdkiiSmmExitBootServicesProtocol] 0 >> >> That is, MpInitChangeApLoopCallback() is entered, but it never finishes. >> "info cpus" prints: >> >> * CPU #0: pc=3D0x000000007f1f7763 thread_id=3D17395 >> CPU #1: pc=3D0x000000007f2ce01e (halted) thread_id=3D17396 >> CPU #2: pc=3D0x000000007f2ce01e (halted) thread_id=3D17397 >> CPU #3: pc=3D0x00000000fffffff0 thread_id=3D17398 >> >> and I've also seen a case where all the APs were stuck at the reset=20 >> vector (0x00000000fffffff0), *not* halted, like VCPU#3 above. They=20 >> don't spin, they're just stuck. The spinning comes from CPU#0,=20 >> apparently in MpInitChangeApLoopCallback. >> >> *** I flipped the AP sync mode to traditional (considering the=20 >> relaxed mode shouldn't be required with the broadcast SMIs). This=20 >> time the log ends with: >> >>> SmmInstallProtocolInterface: [EdkiiSmmExitBootServicesProtocol] 0 >>> MpInitChangeApLoopCallback() done! >> >> but then QEMU abort()s: >> >>> kvm_io_ioeventfd_add: error adding ioeventfd: File exists >>> 2016-11-14 17:00:41.405+0000: shutting down, reason=3Dcrashed >> >> I see some ioeventfd stuff in the recent QEMU history; do you think=20 >> it's related? >=20 > Yes, just try 2.7 for now or disable vhost. (1) I think I have some new results. I used the gdbserver built into QEMU, and I (sort of) single-stepped the MpInitChangeApLoopCallback() function in "UefiCpuPkg/Library/MpInitLib/DxeM= pLib.c", and whatever else it called. For this I used the Ia32 1x2x2 configuration. Also, the broadcast SMI patch= es were applied to both QEMU and OVMF. I set a breakpoint on RelocateApLoop(), so that when an AP would start up, = gdb would switch to it automatically (and it did in fact). I liberally used "info cpus" from a separate terminal, and also "info threa= d" from within gdb (which gives an incredibly cool insight into the VCPU st= ates!) (2) Here's a few interesting results (strictly empirically): * when I was stepping through the SendInitSipiSipiAllExcludingSelf() function *real slow*, manually, suddenly things worked * I noticed that, while stepping through the INIT-SIPI-SIPI sequence in the above-mentioned BSP function, the APs switched from "halted" to "running" after the *first* SIPI. Not the second SIPI, the first one. Then I went to the KVM code, and looked at arch/x86/kvm/lapic.c. My explana= tion is terribly inexact, but: * __apic_accept_irq() translates the LAPIC writes (APIC_DM_INIT and APIC_DM_STARTUP) into pending events (KVM_APIC_INIT and KVM_APIC_SIPI) * this function also calls kvm_vcpu_kick() * the kvm_apic_accept_events() function processes the pending events. The KVM_APIC_INIT event, if pending, causes kvm_vcpu_reset() to be called, and the AP to be moved to KVM_MP_STATE_INIT_RECEIVED state. And, in that state, a pending KVM_APIC_SIPI event moves the AP to KVM_MP_STATE_RUNNABLE. This confirms my surprising empirical result that only one SIPI is awaited after the INIT before launching the APs. * the kvm_vcpu_kick() function in "virt/kvm/kvm_main.c" boils down to a smp_send_reschedule() call, if the kicker and the "kickee" are different processors (for example, when the BSP kicks the AP) This seemed important because it suggested that host kernel scheduling jitter could delay the delivery (reception) of the INIT on the AP after the BSP sent it. If the BSP sent the first (and second) SIPI really fast after the INIT, then those SIPIs could be missed, and the AP would remain stuck in KVM_MP_STATE_INIT_RECEIVED state. And this state (remember kvm_vcpu_reset() from above!) is consistent with the RIP being stuck at the reset vector address, with the AP neither running nor being halted. (3) So, I looked at the Intel SDM. It says in the sysprog book (yes, yes, I= should be using the humongous fused edition), 8.4.4.1 Typical BSP Initialization Sequence 15. Broadcasts an INIT-SIPI-SIPI IPI sequence to the APs to wake them up and initialize them: MOV ESI, ICR_LOW; Load address of ICR low dword into ESI. MOV EAX, 000C4500H; Load ICR encoding for broadcast INIT IPI ; to all APs into EAX. MOV [ESI], EAX; Broadcast INIT IPI to all APs ; 10-millisecond delay loop. MOV EAX, 000C46XXH; Load ICR encoding for broadcast SIPI IP ; to all APs into EAX, where xx is the vector ; computed in step 10. MOV [ESI], EAX; Broadcast SIPI IPI to all APs ; 200-microsecond delay loop MOV [ESI], EAX; Broadcast second SIPI IPI to all APs ; 200-microsecond delay loop In the SendInitSipiSipiAllExcludingSelf(), we have a MicroSecondDelay (PcdGet32(PcdCpuInitIpiDelayInMicroSeconds)); between the INIT and the first SIPI, and a MicroSecondDelay (200); between both SIPIs. PcdCpuInitIpiDelayInMicroSeconds is set (in UefiCpuPkg.dec) to 10000, which matches the above 10ms recommendation. I think this could be too low for KVM. (It's telling that the value is a PC= D in the first place.) (4) The circumstances where the "AP is lost" -- due to the missed SIPI(s) I believe -- vary, interestingly. These results are from my testing= with the Ia32 SMM build of OVMF, CPU topology 1x2x2, no XD support. Jeff's= v2 (=3D=3D this series) is applied invariably as a basis (because we agree= that it fixes bugs). * The "AP lost" issue persists at S3 resume even if I raise PcdCpuInitIpiDelayInMicroSeconds to 1/10th of a second. (Reproduced at the 43th resume.) If I set the kernel's "cpu_init_udelay" parameter similarly in addition, then that seems to make the symptom more frequent, not less, which is completely counter-intuitive :( * If I apply the broadcast SMI patch to OVMF (on top of Jeff's v2), then I can't even boot; the AP (or some APs) regularly lose the SIPI and remain stuck in "INIT received" (I think) when started from MpInitChangeApLoopCallback(). Hence the boot never completes. Raising PcdCpuInitIpiDelayInMicroSeconds as described above fixes the boot. (And, the original goal of the broadcast SMI patches is achieved, "efibootmgr" is pretty fast even when bound to VCPU#1.) However, an AP still gets stuck during S3 occasionally :( And, cpu_init_udelay=3D100000 for the guest kernel makes it only worse. Ultimately, I couldn't find a way to make S3 work reliably in the Ia32 SMM = build. This v2 series doesn't help, broadcast SMIs don't help, raising the = INIT<->SIPI delay in the firmware doesn't help (it just mitigates the bad e= ffects of the broadcast SMIs :/), and raising the same delay in the guest k= ernel only makes things worse. Jeff, I think you should go ahead and commit this series. Paolo reviewed pa= tch #3 and I hope Mike or Jiewen can review the first two patches. For OVMF= they improve things (no more emulation failures), and I guess we can figur= e out the lost AP issue later. Thanks Laszlo