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 D2177740035 for ; Fri, 1 Mar 2024 09:33:45 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=oyqOWlPlQsZ7VFasDq6+2pqfHKielEK66Oe9fErbgiY=; 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:Content-Transfer-Encoding; s=20140610; t=1709285624; v=1; b=FXDwZqazRHQEsPodbPyhsqQA9lxcSLTLwiae842ONUo+SazIPqxwytjPw10lxRfapMxi9rHK Z1a8yOnkGA/tAJ4EYG3tJeEZUoLHnV51Vrqb0dZfK6ni4PqE0Cs8JV63xFgptp33gcdRuCWy2Ys vP1ITjC3ngzgqxhb/wdu713E= X-Received: by 127.0.0.2 with SMTP id 9WOQYY7687511xzMhrq2nKEd; Fri, 01 Mar 2024 01:33:44 -0800 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web11.18997.1709285623736408658 for ; Fri, 01 Mar 2024 01:33:43 -0800 X-Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-615-0zaZyNWjN-6bDXq2v9Nd8A-1; Fri, 01 Mar 2024 04:33:41 -0500 X-MC-Unique: 0zaZyNWjN-6bDXq2v9Nd8A-1 X-Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-412c629af3dso3120065e9.1 for ; Fri, 01 Mar 2024 01:33:41 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCWvJCUOH4g1qaSegZWSsQvPsk250roTGZ0n04X9EYx3sDt4zLGgg2ubkd60TYlHWf915R/G22m3GBSuDrwFxpfgRfWpoQ== X-Gm-Message-State: OcN04gw0DjCAJSKke6I7O02jx7686176AA= X-Received: by 2002:a05:600c:a01c:b0:412:b623:bbcc with SMTP id jg28-20020a05600ca01c00b00412b623bbccmr993280wmb.10.1709285620513; Fri, 01 Mar 2024 01:33:40 -0800 (PST) X-Google-Smtp-Source: AGHT+IHYVmYJ0pH5RZ20ia8qv/DNqckSCGBOe/NTkndZDltRsIhHoPE1uSuYQfqgY8sOFl061OykzBqCR5CvjAasSrc= X-Received: by 2002:a05:600c:a01c:b0:412:b623:bbcc with SMTP id jg28-20020a05600ca01c00b00412b623bbccmr993270wmb.10.1709285620174; Fri, 01 Mar 2024 01:33:40 -0800 (PST) MIME-Version: 1.0 References: <20240229130246.3-1-ray.ni@intel.com> <20240229130246.3-3-ray.ni@intel.com> <0102018df956cc94-e49702ac-a077-4b44-923c-3f33a7142f10-000000@eu-west-1.amazonses.com> In-Reply-To: <0102018df956cc94-e49702ac-a077-4b44-923c-3f33a7142f10-000000@eu-west-1.amazonses.com> From: "Paolo Bonzini" Date: Fri, 1 Mar 2024 10:33:27 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack overflow issue due to nested interrupts To: Michael Brown Cc: "Ni, Ray" , "devel@edk2.groups.io" , "Kinney, Michael D" , 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: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=FXDwZqaz; 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 On Fri, Mar 1, 2024 at 10:27=E2=80=AFAM Michael Brown wrot= e: > > My version is attached, feel free to reuse it (either entirely or > > partially) for a hypothetical v2. Apologies to you and Mike K for the > > confusion! > > I much prefer this version of the patch, because the explanations are > easier to follow and to reason about. Thanks! I find the new logic to be quite appealing... now that I understand it. Hopefully this provides a way to find the best of both worlds. > There is an implicit assumption that if interrupts are disabled when > RaiseTPL() is called then we must have been called from an interrupt > handler. How sure are we that this assumption is correct? > > It's possible that it doesn't matter. The new logic will effectively > mean that RestoreTPL() will restore not only the TPL but also the > interrupts-enabled state to whatever existed at the time of the > corresponding RaiseTPL(). Right: that's what my comment says + // However, when the handler calls RestoreTPL + // before returning, we want to keep interrupts disabled. This + // restores the exact state at the beginning of the handler, + // before the call to RaiseTPL(): low TPL and interrupts disabled. but indeed it applies beyond interrupt handlers. It might even be a bugfix. > Maybe this is what we want? If so, then we > should probably phrase the comments in these terms instead of in terms > of being called from an interrupt handler. I think phrasing the comments with reference to interrupt handlers is fine, but it may be worth adding a comment to either mInterruptedTplMask or CoreRestoreTpl(), like +/// NOTE: Strictly speaking, this applies to anything that +/// calls RaiseTPL() with interrupts disabled, not just +/// interrupt handlers. Interrupt handlers are just the case +/// that we care the most about, because of the potential +/// for stack overflow. Paolo -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116228): https://edk2.groups.io/g/devel/message/116228 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-