From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from esa5.hc3370-68.iphmx.com (esa5.hc3370-68.iphmx.com [216.71.155.168]) by mx.groups.io with SMTP id smtpd.web12.843.1592414642331495461 for ; Wed, 17 Jun 2020 10:24:02 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: citrix.com, ip: 216.71.155.168, mailfrom: igor.druzhinin@citrix.com) Authentication-Results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: JLDRlMpFPRsJ4NqFP7w2Ou8hWoSjd8hqVIm+LBLBC82Q6bQTlyVU8ZBm8u5UucmrxjNtslTM3A diEo7Fudw9Yj4c0DyRGucGHjlPvg75hRUP06K8Mq38O7kubFuJ03NFM9T3RNDxh7VK5pr5P9ve dS6RVrb2mU07EmQEWHIi23P87H6E7eHpNFqpZKLA51VamXXg5QDeeXygdO5H4TAyy4T9wFqgJK TOr9/LutXN6yLAEKdA74JDOziu8WTu/wOiHKt1DyPMzSuvjaUler+rm6Lp7Bjfuv9tFqHW3h/G muQ= X-SBRS: 2.7 X-MesageID: 20525749 X-Ironport-Server: esa5.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.73,523,1583211600"; d="scan'208";a="20525749" Subject: Re: [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load To: Paolo Bonzini , Laszlo Ersek , CC: , , , , Ray Ni References: <1592275782-9369-1-git-send-email-igor.druzhinin@citrix.com> <1d4cd50f-9479-0361-2d23-edda83037243@redhat.com> <8f885a46-f72b-bf13-db55-28e6db8b5bff@redhat.com> <291ce7e8-c1ce-df38-de48-e39671c9c894@redhat.com> From: Igor Druzhinin Message-ID: <5f9198c8-b89d-a313-8237-5178af9cf484@citrix.com> Date: Wed, 17 Jun 2020 18:23:45 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <291ce7e8-c1ce-df38-de48-e39671c9c894@redhat.com> Return-Path: igor.druzhinin@citrix.com Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit On 17/06/2020 17:59, Paolo Bonzini wrote: > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > > On 17/06/20 17:46, Laszlo Ersek wrote: >>> That said, Igor's patch seems correct to me. In fact, I'd even move >>> DisableInterrupts before gBS->RestoreTPL unless there's a good reason >>> not to do so. >> OK, thank you! >> >> Igor, please confirm if you'd like to submit v2 with the update >> suggested by Paolo, or if you prefer the current version. We're at the >> beginning of the current development cycle, so I guess we can apply the >> patch and see how it works in practice. If it ends up wreaking havoc on >> some platforms, we can always revert the patch in time for the next >> stable tag (edk2-stable202008). > > For what it's worth "correct" means that I don't see anything that could > break and in fact I find it good policy hygienic not to allow recursive > interrupts. > > v1 is okay for me too, so: > > Reviewed-by: Paolo Bonzini Thanks, unfortunately it's not possible to move DisableInterrupts inside TPL block as RestoreTPL would immediately enable them according to current logic. IMO RaiseTPL could technically save interrupt state inside it (in that case it was disabled) and then honor it in RestoreTPL but that might have more surprise consequences than the proposed change I reckon. I will create a BZ ticket as requested. Igor