From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id E1E58D801B0 for ; Tue, 16 Jan 2024 15:16:54 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=fx/Ke9bVehsoxniiYd0++GEjIUYeOsm++7oW5aQizR4=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:Autocrypt:In-Reply-To:Feedback-ID:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1705418213; v=1; b=NRhjow3It0Jh/bUBKIh7fEi/Jtjh6uYj3NLU2sdavHbGJCq8pBul95BdDIskSDUlyMpwKS+8 yCbV8JlSDjDJLyrSZitI8gZZr/iDbrkhZwoE7IEWjyHLpsyBuuUvsYDB0yhUAIFz4+TVQY+d674 m/0cGj3vndhdauM6o5lM9CGA= X-Received: by 127.0.0.2 with SMTP id OrcdYY7687511xqJllaJpevw; Tue, 16 Jan 2024 07:16:53 -0800 X-Received: from a7-10.smtp-out.eu-west-1.amazonses.com (a7-10.smtp-out.eu-west-1.amazonses.com [54.240.7.10]) by mx.groups.io with SMTP id smtpd.web10.17328.1705418212591918515 for ; Tue, 16 Jan 2024 07:16:53 -0800 Message-ID: <0102018d12d8bd9f-d209332f-f501-498e-b43c-3b0cc4f7ef7b-000000@eu-west-1.amazonses.com> Date: Tue, 16 Jan 2024 15:16:50 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH 1/6] UefiCpuPkg/LocalApicTimerDxe: Duplicate OvmfPkg/LocalApicTimerDxe driver To: devel@edk2.groups.io, lersek@redhat.com, kraxel@redhat.com Cc: Pedro Falcato , ray.ni@intel.com, Michael D Kinney , Nate DeSimone , Rahul Kumar References: <20240115080325.147-1-ray.ni@intel.com> <20240115080325.147-2-ray.ni@intel.com> <0102018d11ac8fe8-1ce1e102-af57-426f-bafc-7297bec4799a-000000@eu-west-1.amazonses.com> From: "Michael Brown" Autocrypt: addr=mcb30@ipxe.org; keydata= xsFNBGPmfF4BEAC3vcU4aLC/9Uy/rTpmYujbqxQNZje9E34jGvLxO3uYwj4BeHj1Nn5T2TDM Gkc4ngk+mGPsJsIn69YU5cfVN+ch9O7FVfsn6egZsCNeLy6Qz0o//gBaWJodFBeawuBjXXyV HnQZa1p7bA/Lws8minW7NrZ7XZgEBaiVm1v1dNbLEoWR8UL2AMtph5loCQ5jPYQNqp/wH9El /R30GjXvAd1riWyJR2TWSN23J9rnuH2Ue+N4yEnWxAsBQ6M/NFQ5z42w4mYdsnzy1w3PulrL icpSixXHkm3lQcKGtKKX41HvJukSpxCgbHfuHGEJZ7bdhgRic1DHKav0JR8kQhx3gnPh06z8 1Teu2NKkSsTR3Iv6E2x6Yy6H34lKWzBzd8TLNSevesDD/L6NU/HxT9AxrTBuypk9PZGe2VH1 W03XnR/0Mnr0QqQBXcIAERdgNzRJY4VKF75vedf8IooZFUQ4RUlqH+x3aZB9nJ9ET77mPaNi SQVQBxE68uzb7eh2Kf6z7ftOYpWPw1v5HyB3oMmafEDG36SIvNF2wnmNaLQDRnAbTcy4ERgy tpJ3wtQDJeXOePLv8hJ3q7DSuePl7cwz4xy0ZHglW/EXRXLnyRRACfDGowyENoStg06qF+qm edGu1wNtmDZ/lypWm/CkzzpUDFeGP5BLZlqwVX4hn88llfvVzwARAQABzR5NaWNoYWVsIEJy b3duIDxtY2IzMEBpcHhlLm9yZz7CwZEEEwEIADsWIQTgD69MBpjBm2slMvwCNbEKAOtEUAUC Y+Z8nwIbAwULCQgHAgIiAgYVCgkICwIEFgIDAQIeBwIXgAAKCRACNbEKAOtEUFlhD/9ElIUg JxBXpIbF8s7u79OdXLld2Z1DfVmhP5Q+GilPvEeAWHhp689S9B88aNvpwW5zJfxlxcJZO0ay jc7E/vtdNrkXGWNEEXBgdve6m+uL+pW/i5E2htqxbLyfgTJKmsvJ8graHbwrrBS/PA8KuwVJ eAGbBNi3f1gyQQWrLqfTkUpLtuj7A76iVVk0G0a78L69Al84qhK2imqpFJoZt1F8h0Z5ddGv mvf2M/DZp87UXvXjy7X6r7msbMZa6S/Jv0dtWHeZGl3Xu3qzbtjlqFyz2Q7TibHiirsgg/CV BsbH/LLbi/aNCCQ/85C6jAMB0lNzcVZ7ZiKKo+vBNMTycDFk70LA9yjlNf7exHejoXmPkLmH ddapYZ4dzwdOiJlaTu8NZgzXUCt3RDDA1qmZrAOBF/F+tPILAEhenl9kj3blD3mPV2SrWLWY dbahY9BsylUhj/qE1ik5CJXrPotmJhok9Vpg07xKDpVnZXuWLGNIE8018UumO7phLrWQwLb1 wJdN7PG165w4UWf4aQphfwaMKOVU3WDghz3aVSP9rgtm3RsUcYHPKx8IaPcDh2yf0bgG386i Axx3U3UQeyz2Pb9Vigo6DmPwXjLkFr/dukvVLVJLVkUab9ZhhERzWTEEMifUVEK2rGNvA87L VKJ2zOyxWx1e0CPj6fcGbkJ0D10XLs7BTQRj5nxeARAAz18zv2ksRiM6eEKG0qzpiKHVYlVy wtjla+m9wuAIwm314tffY5hjQN46uwTstdhQirjywF1EmcS6KNGiIjmoLim+dqyFP5d/UF5A VjLt0TYq7HjadIxbm2/CvcRnNJ01FkD99xLxV0hFTUAWAUX1mNqQ3MmWIjV89wiT06uuAUog m+jG3RRDyWbUnVELR60mhzccKsaEsjO/HqIERvBwL7tlOJewlPrVyz9Zed9Nhhv0KDAYmdEm kIEEbOfsjRu5I6nIY3NrX+QP9+nmgxADlsjvLXTSU0fT/g7IPEl3gpsQZAbgmrlGcPtvXod8 P4iOmL8GJDU1RdBE9TBOLEbu9UlDRD4zr6tdzRpB9wvXdtSUcNCdHVqJTfq2qjIlBk7x+zQD ayhxzDvTMxD/93K6txKXmVVtfMBsmt9KuD2JBUEAExjsLHqzg48nQg8wF9JYWCWGBb36qpd0 yC6VPzhSLe2Ov3/GyV5ZshO046+OiGxEeaHCwMnDTZF9xrQ5paCwWedlWKvGM2zB64AHuk+M v2ABK/gbDO7eS6p+xz11oD1NHr1HQLRtknfClIqj9AmjgX9maD+4GUrmHaxmkNilIukahotd Un9Up2gX05Wy/S3H/v8RB0kxwWg2Wh065dnyCF4Doe18bcYZvM+iMJmUBag6aDfQlryM04K7 z4ITYDkAEQEAAcLBdgQYAQgAIBYhBOAPr0wGmMGbayUy/AI1sQoA60RQBQJj5nxeAhsMAAoJ EAI1sQoA60RQZj4QAIkiRDVNWynZ4kEdpqmf6hpD++Zycz+LMne4iGRsiyyTf/rPNgskNLrU JD555yDvFiEAhOI27R8YNCJj5byXRDa/Bm6ueClFia+POibt28UEdyOFU9PVcgFaU+VxaBIP rHacHL6A7UKFjmBN7o8VkVF2xXlmFge795mP4/Y3t6qfWUTodrpw1w1t5/bZxZdWqX4pUCpY fEx87jm60+Mj0Tb4VPWXz0UD1q1BDcdYxNa2ISLaJhGJmjjks9eqdFOhPo1fTINMNWF2Alxi jA6WNT8nn9lm1kav75EMYMc8WIR9tb03i+IuKNp2IWwTGBqIUyQj00BhHkZQFl4HxZhV0gXE AWu34Q/Z7hOUXGXq2tvYCxDeaQb2wks93e62lrrUm1JGhPWkVoCI8Md8N2mkonqIfMK8lQ0W WbkYHdKBkgDqhDypNNhkjWNX3JL1kL0c3rqGL381iBAZaGQPygyCx2xH9PDNp59W6u8sXb13 +UX+kXdWU+KYbMTVoO/t4MxUJg6nXPJHz9NCkyluI820l+2OtXZZy0u196evIlUdD6RoTrNK z5OgFxNctVi9BPsQea9du+JlYJ460vZNPz180oczj7iqffd+p9DmAkeK25njWhg3qPeXiNZN 45J9eMChSOaJ0GMGUQndIIxz7PO8IzjbkSHLG5CKrR3MaphMB/0L In-Reply-To: X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on blyat.fensystems.co.uk Feedback-ID: 1.eu-west-1.fspj4M/5bzJ9NLRzJP0PaxRwxrpZqiDQJ1IF94CF2TA=:AmazonSES X-SES-Outgoing: 2024.01.16-54.240.7.10 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mcb30@ipxe.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: dfiaKnJEV5pc8JLiYzYBhJwRx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=NRhjow3I; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=none On 16/01/2024 14:34, Laszlo Ersek wrote: > On 1/16/24 10:48, Michael Brown wrote: > IOW, my impression is that NestedInterruptTplLib can certainly handle > all scenarios thrown at it, but where it really matters is in the face > of an interrupt storm (not just "normal nesting"), and a storm is > unlikely (or even impossible?) on physical hardware. > > ... Oh, scratch that. "Interrupt storm" simply means that interrupts are > being delivered at a rate higher than the handler routine can service > them. IOW, the "storm" is not that interrupts are delivered *very > rapidly* in an absoulte sense. If interrupts are delivered at normal > frequency, but the handler is too slow to service *even that rate*, then > that also qualifies as "storm", because the nesting depth will *keep > growing*. It's not really the growth rate that matters; what matter is > the *trend*, i.e., the fact that there *is* growth (the stack gets > deeper and deeper). The stack might not overflow immediately, and if the > handler speeds up (for whatever reason), the stack might recover, but > there is nothing to prevent an overflow. > > So, in the end, I think you've convinced me. :) >> I'm happy to send a patch to migrate NestedInterruptTplLib to >> MdeModulePkg, so that it can be consumed outside of OvmfPkg.  Shall I do >> this? > > Sounds like a valid idea to me. > > Could be greatly supported by a test case (to be run on the bare metal) > installing a slow handler that *eventually* exhausted the stack, when > not using NestedInterruptTplLib. > > (FWIW, IIRC, the UEFI spec warns about this -- it says something like, > "return from TPL_HIGH as soon as you can, otherwise the system will > become unstable".) > > Sorry for the wall of text, I find this very difficult to reason about. I also find it very difficult to reason about, which is why NestedInterruptRestoreTpl() has 126 lines of comments providing a semi-formal proof of correctness for a mere 15 statements of C code! In particular, I find it difficult to reason about when it would be safe for a platform to *not* use NestedInterruptTplLib. It's clearly empirically difficult to trigger stack underflow via an interrupt "storm" on physical hardware, but I'm not convinced it's impossible. I find it mentally easier to rely on the hard guarantee that NestedInterruptTplLib provides: that nested interrupts will continue to be delivered but that the number of interrupt-induced stack frames is bounded by the (small, finite) number of distinct TPL levels in existence. While developing NestedInterruptTplLib, I did hack together a test case for a slow handler that would deliberately induce an interrupt storm, since I needed this to test that my code was working. When triggered, this test would cause the machine to effectively hang due to servicing an endless storm of timer interrupts. Before NestedInterruptTplLib, the stack would soon underflow and would typically cause a reboot (or other crash). With NestedInterruptTplLib the machine would continue to service interrupts indefinitely. How might such a test case be included in upstream EDK2? I'm peripherally aware of EDK2 test infrastructure such as UEFI SCT, but I've never interacted with it yet. Thanks, Michael -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113908): https://edk2.groups.io/g/devel/message/113908 Mute This Topic: https://groups.io/mt/103734961/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-