Michael,
do you have any updated patch?
Thanks,
Ray
From: Michael Brown
Sent: Friday, March 1, 2024 19:10
To: Paolo Bonzini
Cc: Ni, Ray ; devel@edk2.groups.io ;
Kinney, Michael D ; Liming Gao
; Laszlo Ersek
Subject: Re: [PATCH 2/2] MdeModulePkg/DxeCore: Fix
Il ven 1 mar 2024, 12:10 Michael Brown ha scritto:
> It feels as though this should be able to be cleanly modelled with a
> single global state array
>
>BOOLEAN mSavedInterruptState[TPL_HIGH_LEVEL]
>
Pretty much, yes. But you only have to write it when the interrupts are
already disabled,
On 01/03/2024 09:33, Paolo Bonzini wrote:
On Fri, Mar 1, 2024 at 10:27 AM Michael Brown wrote:
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
On Fri, Mar 1, 2024 at 10:27 AM Michael Brown wrote:
> > 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
On 01/03/2024 08:37, Paolo Bonzini wrote:
So I am starting to see more and more similarities between the two
approaches. I went a step further with fresh mind, removing the while
loop... and basically reinvented your and Michael's patch. :) The only
difference in the logic is a slightly
Paolo,
Happy weekends!
Thanks! I will read it on my next Monday.
Thanks,
Ray
> -Original Message-
> From: Paolo Bonzini
> Sent: Friday, March 1, 2024 4:44 PM
> To: Ni, Ray
> Cc: devel@edk2.groups.io; Kinney, Michael D ;
> Liming Gao ; Laszlo Ersek ;
> Michael Brown
> Subject: Re:
One fix is needed in the code.
On Thu, Feb 29, 2024 at 2:04 PM Ray Ni wrote:
> + //
> + // Save the "Interrupted TPL" (TPL that was interrupted).
> + //
> + mInterruptedTplMask |= (UINTN)(1 << gEfiCurrentTpl);
> +}
>}
> + //
> + // Clear interrupted TPL
On Fri, Mar 1, 2024 at 4:08 AM Ni, Ray wrote:
> @@ -161,5 +191,46 @@ CoreRestoreTpl (
>IN EFI_TPL NewTpl
>)
> {
> + BOOLEAN InInterruptHandler = FALSE;
> +
> + //
> + // Unwind the nested interrupt handlers up to the required
> + // TPL, paying attention not to overflow the stack.
I think we are all aligned on the purpose. It's to avoid enabling the
interrupts in the end of RestoreTPL (HIGH->non-HIGH) in the interrupt context.
The discussion is about how to implement it.
Michael Brown's idea is to avoid changing DxeCore but add a customized
RaiseTpl/RestoreTpl
On Thu, Feb 29, 2024 at 2:04 PM Ray Ni wrote:
> @@ -134,9 +262,9 @@ CoreRestoreTpl (
>}
>
>//
> - // Set the new value
> + // Set the new TPL with interrupt disabled.
>//
> -
> + CoreSetInterruptState (FALSE);
>gEfiCurrentTpl = NewTpl;
>
>//
> @@ -144,7 +272,22 @@
On 2/29/24 20:22, Michael Brown wrote:
The design of NestedInterruptTplLib is that each nested interrupt must increase
the TPL, but if I understand correctly there is a hole here:
//
// Call RestoreTPL() to allow event notifications to be
// dispatched. This will implicitly re-enable
On 2/29/24 20:16, Kinney, Michael D wrote:
-Original Message-
From: Paolo Bonzini
Sent: Thursday, February 29, 2024 11:04 AM
To: Ni, Ray ; devel@edk2.groups.io
Cc: Kinney, Michael D ; Liming Gao
; Laszlo Ersek ; Michael
Brown
Subject: Re: [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack
On 29/02/2024 19:26, Michael D Kinney wrote:
I think one advantage of this new proposal is to prevent
an extra level of nesting and use of stack resources in
that extra level.
I think that's a negligible benefit. In the scenario as I outlined for
NestedInterruptTplLib, there is potentially
On 29/02/2024 19:09, Michael D Kinney wrote:
"When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has
been installed, then the full version of the Boot Service RestoreTPL()
can be made available. When an attempt is made to restore the TPL
level
to level below EFI_TPL_HIGH_LEVEL,
> Sent: Thursday, February 29, 2024 11:22 AM
> To: devel@edk2.groups.io; pbonz...@redhat.com; Ni, Ray
>
> Cc: Kinney, Michael D ; Liming Gao
> ; Laszlo Ersek
> Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack
> overflow issue due to nested interrupts
>
On 29/02/2024 19:04, Paolo Bonzini wrote:
On 2/29/24 14:02, Ray Ni wrote:
In the end, it will lower the TPL to TPL_APPLICATION with interrupt
enabled.
However, it's possible that another timer interrupt happens just in
the end
of RestoreTPL() function when TPL is TPL_APPLICATION.
How do
> -Original Message-
> From: Paolo Bonzini
> Sent: Thursday, February 29, 2024 11:04 AM
> To: Ni, Ray ; devel@edk2.groups.io
> Cc: Kinney, Michael D ; Liming Gao
> ; Laszlo Ersek ; Michael
> Brown
> Subject: Re: [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack overflow issue
> due to nested
> -Original Message-
> From: Michael Brown
> Sent: Thursday, February 29, 2024 9:39 AM
> To: Kinney, Michael D ;
> devel@edk2.groups.io; Ni, Ray
> Cc: Liming Gao ; Laszlo Ersek
> ; Paolo Bonzini
> Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix st
Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack overflow
issue due to nested interrupts
Il gio 29 feb 2024, 17:45 Kinney, Michael D
mailto:michael.d.kin...@intel.com>> ha scritto:
Hi Michael,
Can you provide a pointer to the UEFI Spec statement this breaks?
Th
On 2/29/24 14:02, Ray Ni wrote:
In the end, it will lower the TPL to TPL_APPLICATION with interrupt enabled.
However, it's possible that another timer interrupt happens just in the end
of RestoreTPL() function when TPL is TPL_APPLICATION.
How do non-OVMF platforms solve the issue? Do they
gt; 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 ide
On 29/02/2024 16:43, Kinney, Michael D wrote:
Hi Michael,
Can you provide a pointer to the UEFI Spec statement this breaks?
II-9.7.1.3 RestoreTPL():
"When the DXE Foundation is notified that the EFI_CPU_ARCH_PROTOCOL has
been installed, then the full version of the Boot Service RestoreTPL()
szlo 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 -> no
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
24 matches
Mail list logo
