From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.3114.1592469431336699789 for ; Thu, 18 Jun 2020 01:37:11 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=hm4F4SUn; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1592469430; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EouhV3m9pEJqd8bRq6yMQ3RQ/gzSAEeFWH5g8HpbWc4=; b=hm4F4SUnXsobFeNZ4c1y88ntfrlj72UVhbENVuzNjxRmX1/vqQ3kFF6XHtsEbveZt0oB+5 6zg//S8VAeZte9qfPZoddCnTd1prTvgHWkZ+/HToYWy64eoUwZ8VVNtDFqhU1gmJaF90LS 8Qt22p0bn79F6Xle9v+1PG4VjE9LmnE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-185--o3r-t8lPoeLUU5XJNtBpQ-1; Thu, 18 Jun 2020 04:37:06 -0400 X-MC-Unique: -o3r-t8lPoeLUU5XJNtBpQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2F66010CE788; Thu, 18 Jun 2020 08:37:05 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-60.ams2.redhat.com [10.36.113.60]) by smtp.corp.redhat.com (Postfix) with ESMTP id B53525D9D3; Thu, 18 Jun 2020 08:36:59 +0000 (UTC) Subject: Re: [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load To: Igor Druzhinin , Paolo Bonzini , devel@edk2.groups.io Cc: jordan.l.justen@intel.com, ard.biesheuvel@arm.com, anthony.perard@citrix.com, julien@xen.org, 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> <5f9198c8-b89d-a313-8237-5178af9cf484@citrix.com> From: "Laszlo Ersek" Message-ID: <524c006e-9c0a-33f0-70bc-6ededbd71d0b@redhat.com> Date: Thu, 18 Jun 2020 10:36:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5f9198c8-b89d-a313-8237-5178af9cf484@citrix.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 06/17/20 19:23, Igor Druzhinin wrote: > 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 Thank you, Paolo! > 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. Thanks -- please follow up with the URL, and then I'll pick up your current patch, with Paolo's R-b and the BZ URL added to the commit message. Acked-by: Laszlo Ersek Thanks Laszlo