From: "Anthony PERARD" <anthony.perard@citrix.com>
To: <devel@edk2.groups.io>
Cc: Liming Gao <gaoliming@byosoft.com.cn>,
Zhiguang Liu <zhiguang.liu@intel.com>,
Michael D Kinney <michael.d.kinney@intel.com>,
Anthony PERARD <anthony.perard@citrix.com>
Subject: [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Better support for dynamic PcdFSBClock
Date: Tue, 8 Nov 2022 14:31:48 +0000 [thread overview]
Message-ID: <20221108143148.1552888-1-anthony.perard@citrix.com> (raw)
From: Anthony PERARD <anthony.perard@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.perard@citrix.com>
---
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
next reply other threads:[~2022-11-08 14:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-08 14:31 Anthony PERARD [this message]
2023-02-10 1:27 ` [edk2-devel] [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Better support for dynamic PcdFSBClock joeyli
2023-02-10 2:23 ` Michael D Kinney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221108143148.1552888-1-anthony.perard@citrix.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox