On Tue, Nov 08, 2022 at 02:31:48PM +0000, Anthony PERARD via groups.io wrote: > From: Anthony PERARD <anthony.per...@citrix.com> > > The PcdFSBClock can be a dynamic PCD. This can be an issue when > InternalX86GetTimerFrequency() tries to get the value while been > called with TPL been set to TPL_HIGH_LEVEL. > > When the PCD is dynamic, PcdGet*() calls a function that try to grab a > lock which set TPL to TPL_NOTIFY. If TPL is already at TPL_HIGH_LEVEL, > then an assert() in RaiseTPL() fails (in debug build). > > To try to avoid the issue, we'll store the current PcdFSBClock value > in a local variable the first time we need it. > > The issue was discovered while attempting to boot a Windows guest with > OvmfXen platform. The issue appear while executing the Windows's boot > loader EFI application. > > The down side of this change is that when the PCD is FixedAtBuild, the > value is loaded from a variable rather than been a constant. > > Signed-off-by: Anthony PERARD <anthony.per...@citrix.com>
I have tested this patch with edk2-stable202202 and edk2-stable202211. It works to me to fix problem on bto#4340: Bug 4340 - Running windows 2k22 on OvmfXen got ASSERT /root/source-git/edk2/MdeModulePkg/Core/Dxe/Event/Tpl.c(66): ((BOOLEAN)(0==1)) https://bugzilla.tianocore.org/show_bug.cgi?id=4340 Thanks a lot! Joey Lee > --- > > InternalX86GetTimerFrequency() is called by > - MicroSecondDelay() > - NanoSecondDelay() > - GetPerformanceCounterProperties() > > If one of those functions is called for the first time with a TPL too > high, the work around in this patch would not work. But it seems to > workaround the issue in my test case. So maybe that's enough, unless > someone have a better idea? > --- > MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > b/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > index c60589607fde..28ff77ad0c1f 100644 > --- a/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > +++ b/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > @@ -90,8 +90,16 @@ InternalX86GetTimerFrequency ( > IN UINTN ApicBase > ) > { > + STATIC UINT32 mFSBClock; > + > + if (mFSBClock == 0) { > + // > + // Cache current value of PcdFSBClock in case it's a dynamic PCD. > + // > + mFSBClock = PcdGet32 (PcdFSBClock); > + } > return > - PcdGet32 (PcdFSBClock) / > + mFSBClock / > mTimerLibLocalApicDivisor[MmioBitFieldRead32 (ApicBase + APIC_TDCR, 0, > 3)]; > } > > -- > Anthony PERARD > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#99941): https://edk2.groups.io/g/devel/message/99941 Mute This Topic: https://groups.io/mt/94891128/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-