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 CA1F874003D for ; Thu, 29 Feb 2024 17:39:53 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Mw/xjWPG/4xfTabHqFLn1DZhoJSue4u56Xpd+GZCwEY=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1709228392; v=1; b=pA4+C/JrJYLH2TwMDhyEFK4ETAchnzMkXYA7/8GCReuTZ0vGlfmHxDTNz4hQdiE6G/PBRNV/ qx1dpQYtxBWcTDaZeIi95LhmoGfMzF8U46Vgk3IPrtqp3mJiKHlpUj7bNDp6bUDUIXYLKzQ4up4 rC4xgZRyJkaAlzRS4kyLe7D8= X-Received: by 127.0.0.2 with SMTP id UHJRYY7687511xMXjGTVrir4; Thu, 29 Feb 2024 09:39:52 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.1458.1709228391709677479 for ; Thu, 29 Feb 2024 09:39:51 -0800 X-Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-244-mkTsjSaBP0Wy0v15PCgqSA-1; Thu, 29 Feb 2024 12:39:48 -0500 X-MC-Unique: mkTsjSaBP0Wy0v15PCgqSA-1 X-Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2d31172c0b6so6113711fa.0 for ; Thu, 29 Feb 2024 09:39:48 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCVmQYVhGctrWqRl5DHc73ETVq38zy+g+P5JQA3cYjcabASMwj0Iu9SE834cXYAYugRZO1KV2ejE4QlurGb7po+EN35/Ig== X-Gm-Message-State: KSeCBY793UDEzE7foI63zk3Sx7686176AA= X-Received: by 2002:a05:651c:106f:b0:2d2:d6a0:6ee5 with SMTP id y15-20020a05651c106f00b002d2d6a06ee5mr1541531ljm.12.1709228387378; Thu, 29 Feb 2024 09:39:47 -0800 (PST) X-Google-Smtp-Source: AGHT+IHlGHxgZBCLthhxNeF8X4QbWXPybCN7x8D1jo32wq7aw5Ld6qtPQVig7b8d0dwtu/fd6DTnJdDYDodGABmmhAc= X-Received: by 2002:a05:651c:106f:b0:2d2:d6a0:6ee5 with SMTP id y15-20020a05651c106f00b002d2d6a06ee5mr1541521ljm.12.1709228387035; Thu, 29 Feb 2024 09:39:47 -0800 (PST) MIME-Version: 1.0 References: <20240229130246.3-1-ray.ni@intel.com> <20240229130246.3-3-ray.ni@intel.com> <0102018df508a238-cb6f0ada-c808-4b77-8555-2315d3377c96-000000@eu-west-1.amazonses.com> In-Reply-To: From: "Paolo Bonzini" Date: Thu, 29 Feb 2024 18:39:33 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack overflow issue due to nested interrupts To: "Kinney, Michael D" Cc: Michael Brown , edk2-devel-groups-io , "Ni, Ray" , Liming Gao , Laszlo Ersek X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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,pbonzini@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="000000000000d98a52061288beb0" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="pA4+C/Jr"; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --000000000000d98a52061288beb0 Content-Type: text/plain; charset="UTF-8" Il gio 29 feb 2024, 17:45 Kinney, Michael D ha scritto: > Hi Michael, > > Can you provide a pointer to the UEFI Spec statement this breaks? > The spec does say that interrupts are disabled for TPL_HIGH_LEVEL, but indeed it doesn't say they are always enabled at lower levels. However, if the interrupts aren't always enabled whenever you're below TPL_HIGH_LEVEL, you get priority inversions (and deadlocks). For example, if you end up running with interrupts disabled at TPL_CALLBACK, you are disabling the dispatching of timers at TPL_NOTIFY. I guess this can be deduced from these two passages: - "The functions in these queues are invoked in FIFO order, starting with the highest priority level queue and proceeding to the lowest priority queue that is unmasked by the current TPL" - "If Type is TimerRelative and TriggerTime is 0, then the timer event will be signaled on the next timer tick" (in the description of gBS->SetTimer) Paolo > Thanks, > > Mike > > > -----Original Message----- > > From: Michael Brown > > Sent: Thursday, February 29, 2024 5:23 AM > > To: devel@edk2.groups.io; Ni, Ray > > Cc: Kinney, Michael D ; Liming Gao > > ; Laszlo Ersek ; Paolo > > Bonzini > > Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack > > overflow issue due to nested interrupts > > > > On 29/02/2024 13:02, Ni, Ray wrote: > > > A ideal solution is to not keep the interrupt disabled when > > > RestoreTPL(TPL_HIGH -> not TPL_HIGH) is executed in the timer > > interrupt > > > context because the interrupt handler will re-enable the interrupt > > with > > > arch specific instructions (e.g.: IRET for x86). > > > > > > The patch introduces mInterruptedTplMask which tells RestoreTPL() if > > > it's called in the interrupt context and whether it should defer > > enabling > > > the interrupt. > > > > NACK. This breaks the specification-defined behaviour for > > RestoreTPL(). > > > > What guarantees do we have that there is no code anywhere in the world > > that relies upon RestoreTPL() unconditionally re-enabling interrupts. > > > > I also find this code substantially harder to follow than > > NestedInterruptTplLib (which does not break any specified behaviour). > > > > Thanks, > > > > Michael > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116179): https://edk2.groups.io/g/devel/message/116179 Mute This Topic: https://groups.io/mt/104642317/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- --000000000000d98a52061288beb0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Il gio 29 feb 2024, 17:45 Kinney, Michael D <michael.d.kinney@intel.com> ha scritto:
Hi Michael,

Can you provide a pointer to the UEFI Spec statement this breaks?

The spec d= oes say that interrupts are disabled for TPL_HIGH_LEVEL, but indeed it does= n't say they are always enabled at lower levels. However, if the interr= upts aren't always enabled whenever you're below TPL_HIGH_LEVEL, yo= u get priority inversions (and deadlocks).

For example, if you end up running with interrupts disab= led at TPL_CALLBACK, you are disabling the dispatching of timers at TPL_NOT= IFY.

I guess this can be= deduced from these two passages:

- "The functions in these queues are invoked in FIFO order, = starting with the highest priority level queue and proceeding to the lowest= priority queue that is unmasked by the current TPL"

- "If Type is TimerRelative and Trig= gerTime is 0, then the timer event will be signaled on the next timer tick&= quot; (in the description of gBS->SetTimer)

<= /div>
Paolo=C2=A0


Thanks,

Mike

> -----Original Message-----
> From: Michael Brown <mcb30@ipxe.org>
> Sent: Thursday, February 29, 2024 5:23 AM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni= @intel.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.co= m>; Liming Gao
> <gaoliming@byosoft.com.cn>; Laszlo Ersek &l= t;lersek@redhat.com>; Paolo
> Bonzini <pbonzini@redhat.com>
> Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack<= br> > overflow issue due to nested interrupts
>
> On 29/02/2024 13:02, Ni, Ray wrote:
> > A ideal solution is to not keep the interrupt disabled when
> > RestoreTPL(TPL_HIGH -> not TPL_HIGH) is executed in the timer<= br> > interrupt
> > context because the interrupt handler will re-enable the interrup= t
> with
> > arch specific instructions (e.g.: IRET for x86).
> >
> > The patch introduces mInterruptedTplMask which tells RestoreTPL()= if
> > it's called in the interrupt context and whether it should de= fer
> enabling
> > the interrupt.
>
> NACK.=C2=A0 This breaks the specification-defined behaviour for
> RestoreTPL().
>
> What guarantees do we have that there is no code anywhere in the world=
> that relies upon RestoreTPL() unconditionally re-enabling interrupts.<= br> >
> I also find this code substantially harder to follow than
> NestedInterruptTplLib (which does not break any specified behaviour).<= br> >
> Thanks,
>
> Michael

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#116179) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--000000000000d98a52061288beb0--