This library is intended to be compatible with XIP SEC and XIP PEIMs that are not allowed to use writable global C variables.
I think storage for STATIC UINT32 mFSBClock is from the .data section and not the stack, so this will break XIP use cases. Mike > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of joeyli via > groups.io > Sent: Thursday, February 9, 2023 5:27 PM > To: devel@edk2.groups.io; anthony.per...@citrix.com > Cc: Gao, Liming <gaolim...@byosoft.com.cn>; Liu, Zhiguang > <zhiguang....@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com> > Subject: Re: [edk2-devel] [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Better support > for dynamic PcdFSBClock > > 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 (#99942): https://edk2.groups.io/g/devel/message/99942 Mute This Topic: https://groups.io/mt/94891128/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
