* [edk2-devel] [PATCH v2] MdeModulePkg/Bus: Fix XhciDxe Linker Issues
@ 2023-12-04 18:47 Nate DeSimone
2023-12-04 18:47 ` [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
[not found] ` <179DB5683F38928F.24600@groups.io>
0 siblings, 2 replies; 6+ messages in thread
From: Nate DeSimone @ 2023-12-04 18:47 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Michael D Kinney
The DXE & MM standalone variant of AcpiTimerLib defines a global
named mPerformanceCounterFrequency. A global with an identical
name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
Since XhciDxe has a dependency on TimerLib, this can cause link
errors due to the same symbol being defined twice if the platform
DSC chooses to use AcpiTimerLib as the TimerLib implementation for
any given platform.
To resolve this, I noted that some of the globals in Xhci.c are not
used outside of the Xhci.c compilation unit:
- mPerformanceCounterStartValue
- mPerformanceCounterEndValue
- mPerformanceCounterFrequency
- mPerformanceCounterValuesCached
I have changed the definition for all of these to static and added
an Xhci prefix. Since they are not used outside of the Xhci.c
compilation unit, there is no reason to have them exported as
globals.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 32 ++++++++++++++---------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 7a2e32a9dd..f4e61d223c 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -2,7 +2,7 @@
The XHCI controller driver.
(C) Copyright 2023 Hewlett Packard Enterprise Development LP<BR>
-Copyright (c) 2011 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -86,10 +86,10 @@ EFI_USB2_HC_PROTOCOL gXhciUsb2HcTemplate = {
0x0
};
-UINT64 mPerformanceCounterStartValue;
-UINT64 mPerformanceCounterEndValue;
-UINT64 mPerformanceCounterFrequency;
-BOOLEAN mPerformanceCounterValuesCached = FALSE;
+static UINT64 mXhciPerformanceCounterStartValue;
+static UINT64 mXhciPerformanceCounterEndValue;
+static UINT64 mXhciPerformanceCounterFrequency;
+static BOOLEAN mXhciPerformanceCounterValuesCached = FALSE;
/**
Retrieves the capability of root hub ports.
@@ -2318,17 +2318,17 @@ XhcConvertTimeToTicks (
UINTN Shift;
// Cache the return values to avoid repeated calls to GetPerformanceCounterProperties ()
- if (!mPerformanceCounterValuesCached) {
- mPerformanceCounterFrequency = GetPerformanceCounterProperties (
- &mPerformanceCounterStartValue,
- &mPerformanceCounterEndValue
- );
+ if (!mXhciPerformanceCounterValuesCached) {
+ mXhciPerformanceCounterFrequency = GetPerformanceCounterProperties (
+ &mXhciPerformanceCounterStartValue,
+ &mXhciPerformanceCounterEndValue
+ );
- mPerformanceCounterValuesCached = TRUE;
+ mXhciPerformanceCounterValuesCached = TRUE;
}
// Prevent returning a tick value of 0, unless Time is already 0
- if (0 == mPerformanceCounterFrequency) {
+ if (0 == mXhciPerformanceCounterFrequency) {
return Time;
}
@@ -2342,7 +2342,7 @@ XhcConvertTimeToTicks (
//
Ticks = MultU64x64 (
DivU64x64Remainder (
- mPerformanceCounterFrequency,
+ mXhciPerformanceCounterFrequency,
Divisor,
&Remainder
),
@@ -2384,12 +2384,12 @@ XhcGetElapsedTicks (
//
// Determine if the counter is counting up or down
//
- if (mPerformanceCounterStartValue < mPerformanceCounterEndValue) {
+ if (mXhciPerformanceCounterStartValue < mXhciPerformanceCounterEndValue) {
//
// Counter counts upwards, check for an overflow condition
//
if (*PreviousTick > CurrentTick) {
- Delta = (mPerformanceCounterEndValue - *PreviousTick) + CurrentTick;
+ Delta = (mXhciPerformanceCounterEndValue - *PreviousTick) + CurrentTick;
} else {
Delta = CurrentTick - *PreviousTick;
}
@@ -2398,7 +2398,7 @@ XhcGetElapsedTicks (
// Counter counts downwards, check for an underflow condition
//
if (*PreviousTick < CurrentTick) {
- Delta = (mPerformanceCounterStartValue - CurrentTick) + *PreviousTick;
+ Delta = (mXhciPerformanceCounterStartValue - CurrentTick) + *PreviousTick;
} else {
Delta = *PreviousTick - CurrentTick;
}
--
2.39.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112049): https://edk2.groups.io/g/devel/message/112049
Mute This Topic: https://groups.io/mt/102976787/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-04 18:47 [edk2-devel] [PATCH v2] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
@ 2023-12-04 18:47 ` Nate DeSimone
2023-12-04 19:59 ` Pedro Falcato
[not found] ` <179DB5683F38928F.24600@groups.io>
1 sibling, 1 reply; 6+ messages in thread
From: Nate DeSimone @ 2023-12-04 18:47 UTC (permalink / raw)
To: devel; +Cc: Ray Ni, Michael D Kinney
The DXE & MM standalone variant of AcpiTimerLib defines a global
named mPerformanceCounterFrequency. A global with an identical
name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
Since XhciDxe has a dependency on TimerLib, this can cause link
errors due to the same symbol being defined twice if the platform
DSC chooses to use AcpiTimerLib as the TimerLib implementation for
any given platform.
To resolve this, I have changed made the definition of
mPerformanceCounterFrequency to static and renamed it to
mAcpiTimerLibTscFrequency. Since this variable is not used outside
of the DxeStandaloneMmAcpiTimerLib.c compilation unit, there is no
reason to have it exported as a global.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
.../AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
index 16ac48938f..ccceb8a649 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
@@ -1,7 +1,7 @@
/** @file
ACPI Timer implements one instance of Timer Library.
- Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -11,6 +11,11 @@
#include <Library/BaseLib.h>
#include <Library/HobLib.h>
+//
+// Cached performance counter frequency
+//
+static UINT64 mAcpiTimerLibTscFrequency = 0;
+
extern GUID mFrequencyHobGuid;
/**
@@ -48,11 +53,6 @@ InternalCalculateTscFrequency (
VOID
);
-//
-// Cached performance counter frequency
-//
-UINT64 mPerformanceCounterFrequency = 0;
-
/**
Internal function to retrieves the 64-bit frequency in Hz.
@@ -66,7 +66,7 @@ InternalGetPerformanceCounterFrequency (
VOID
)
{
- return mPerformanceCounterFrequency;
+ return mAcpiTimerLibTscFrequency;
}
/**
@@ -92,9 +92,9 @@ CommonAcpiTimerLibConstructor (
//
GuidHob = GetFirstGuidHob (&mFrequencyHobGuid);
if (GuidHob != NULL) {
- mPerformanceCounterFrequency = *(UINT64 *)GET_GUID_HOB_DATA (GuidHob);
+ mAcpiTimerLibTscFrequency = *(UINT64 *)GET_GUID_HOB_DATA (GuidHob);
} else {
- mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
+ mAcpiTimerLibTscFrequency = InternalCalculateTscFrequency ();
}
return EFI_SUCCESS;
--
2.39.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112050): https://edk2.groups.io/g/devel/message/112050
Mute This Topic: https://groups.io/mt/102976788/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-04 18:47 ` [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
@ 2023-12-04 19:59 ` Pedro Falcato
2023-12-04 22:13 ` Nate DeSimone
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Falcato @ 2023-12-04 19:59 UTC (permalink / raw)
To: devel, nathaniel.l.desimone; +Cc: Ray Ni, Michael D Kinney
On Mon, Dec 4, 2023 at 6:48 PM Nate DeSimone
<nathaniel.l.desimone@intel.com> wrote:
>
> The DXE & MM standalone variant of AcpiTimerLib defines a global
> named mPerformanceCounterFrequency. A global with an identical
> name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
>
> Since XhciDxe has a dependency on TimerLib, this can cause link
> errors due to the same symbol being defined twice if the platform
> DSC chooses to use AcpiTimerLib as the TimerLib implementation for
> any given platform.
>
> To resolve this, I have changed made the definition of
> mPerformanceCounterFrequency to static and renamed it to
> mAcpiTimerLibTscFrequency. Since this variable is not used outside
> of the DxeStandaloneMmAcpiTimerLib.c compilation unit, there is no
> reason to have it exported as a global.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
> .../AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> index 16ac48938f..ccceb8a649 100644
> --- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> +++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> @@ -1,7 +1,7 @@
> /** @file
> ACPI Timer implements one instance of Timer Library.
>
> - Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2013 - 2023, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -11,6 +11,11 @@
> #include <Library/BaseLib.h>
> #include <Library/HobLib.h>
>
> +//
> +// Cached performance counter frequency
> +//
> +static UINT64 mAcpiTimerLibTscFrequency = 0;
I'd say you don't need to rename it if it's a static variable. Now the
identifier is 2x longer with no additional relevant information.
Aren't we supposed to use STATIC vs static, CONST vs const, etc? Annoyingly :/
--
Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112051): https://edk2.groups.io/g/devel/message/112051
Mute This Topic: https://groups.io/mt/102976788/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-04 19:59 ` Pedro Falcato
@ 2023-12-04 22:13 ` Nate DeSimone
2023-12-05 8:18 ` Ni, Ray
0 siblings, 1 reply; 6+ messages in thread
From: Nate DeSimone @ 2023-12-04 22:13 UTC (permalink / raw)
To: Pedro Falcato, devel@edk2.groups.io; +Cc: Ni, Ray, Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 3584 bytes --]
> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Monday, December 4, 2023 11:59 AM
> To: devel@edk2.groups.io; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib
> incompatibility with XhciDxe
>
> On Mon, Dec 4, 2023 at 6:48 PM Nate DeSimone
> <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>> wrote:
> >
> > The DXE & MM standalone variant of AcpiTimerLib defines a global named
> > mPerformanceCounterFrequency. A global with an identical name is also
> > present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> >
> > Since XhciDxe has a dependency on TimerLib, this can cause link errors
> > due to the same symbol being defined twice if the platform DSC chooses
> > to use AcpiTimerLib as the TimerLib implementation for any given
> > platform.
> >
> > To resolve this, I have changed made the definition of
> > mPerformanceCounterFrequency to static and renamed it to
> > mAcpiTimerLibTscFrequency. Since this variable is not used outside of
> > the DxeStandaloneMmAcpiTimerLib.c compilation unit, there is no reason
> > to have it exported as a global.
> >
> > Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
> > ---
> > .../AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c | 18
> > +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git
> > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > index 16ac48938f..ccceb8a649 100644
> > ---
> > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > +++
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.
> > +++ c
> > @@ -1,7 +1,7 @@
> > /** @file
> > ACPI Timer implements one instance of Timer Library.
> >
> > - Copyright (c) 2013 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > + Copyright (c) 2013 - 2023, Intel Corporation. All rights
> > + reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -11,6 +11,11 @@
> > #include <Library/BaseLib.h>
> > #include <Library/HobLib.h>
> >
> > +//
> > +// Cached performance counter frequency // static UINT64
> > +mAcpiTimerLibTscFrequency = 0;
>
> I'd say you don't need to rename it if it's a static variable. Now the identifier is
> 2x longer with no additional relevant information.
This is in response to Mike K's feedback: https://edk2.groups.io/g/devel/message/111966
>
> Aren't we supposed to use STATIC vs static, CONST vs const, etc? Annoyingly
> :/
Apparently not. Honestly, I lose track of what the current coding style is sometimes too.
>
> --
> Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112052): https://edk2.groups.io/g/devel/message/112052
Mute This Topic: https://groups.io/mt/102976788/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 8802 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
2023-12-04 22:13 ` Nate DeSimone
@ 2023-12-05 8:18 ` Ni, Ray
0 siblings, 0 replies; 6+ messages in thread
From: Ni, Ray @ 2023-12-05 8:18 UTC (permalink / raw)
To: Desimone, Nathaniel L, Pedro Falcato, devel@edk2.groups.io
Cc: Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 4424 bytes --]
Reviewed-by: Ray Ni <ray.ni@intel.com>
I agree that with “static”, the “mAcpiTimerLib” prefix is not really needed. But having it does no harm😊
I remember that coding style doc says “static” instead of “STATIC”. “STATIC” was the old requirement.
Thanks,
Ray
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Sent: Tuesday, December 5, 2023 6:13 AM
To: Pedro Falcato <pedro.falcato@gmail.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com<mailto:pedro.falcato@gmail.com>>
> Sent: Monday, December 4, 2023 11:59 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
> Cc: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Subject: Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib
> incompatibility with XhciDxe
>
> On Mon, Dec 4, 2023 at 6:48 PM Nate DeSimone
> <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>> wrote:
> >
> > The DXE & MM standalone variant of AcpiTimerLib defines a global named
> > mPerformanceCounterFrequency. A global with an identical name is also
> > present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
> >
> > Since XhciDxe has a dependency on TimerLib, this can cause link errors
> > due to the same symbol being defined twice if the platform DSC chooses
> > to use AcpiTimerLib as the TimerLib implementation for any given
> > platform.
> >
> > To resolve this, I have changed made the definition of
> > mPerformanceCounterFrequency to static and renamed it to
> > mAcpiTimerLibTscFrequency. Since this variable is not used outside of
> > the DxeStandaloneMmAcpiTimerLib.c compilation unit, there is no reason
> > to have it exported as a global.
> >
> > Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> > Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com<mailto:nathaniel.l.desimone@intel.com>>
> > ---
> > .../AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c | 18
> > +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git
> > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > index 16ac48938f..ccceb8a649 100644
> > ---
> > a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > +++
> b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.
> > +++ c
> > @@ -1,7 +1,7 @@
> > /** @file
> > ACPI Timer implements one instance of Timer Library.
> >
> > - Copyright (c) 2013 - 2018, Intel Corporation. All rights
> > reserved.<BR>
> > + Copyright (c) 2013 - 2023, Intel Corporation. All rights
> > + reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -11,6 +11,11 @@
> > #include <Library/BaseLib.h>
> > #include <Library/HobLib.h>
> >
> > +//
> > +// Cached performance counter frequency // static UINT64
> > +mAcpiTimerLibTscFrequency = 0;
>
> I'd say you don't need to rename it if it's a static variable. Now the identifier is
> 2x longer with no additional relevant information.
This is in response to Mike K's feedback: https://edk2.groups.io/g/devel/message/111966
>
> Aren't we supposed to use STATIC vs static, CONST vs const, etc? Annoyingly
> :/
Apparently not. Honestly, I lose track of what the current coding style is sometimes too.
>
> --
> Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112070): https://edk2.groups.io/g/devel/message/112070
Mute This Topic: https://groups.io/mt/102976788/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 11824 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
[not found] ` <179DB5683F38928F.24600@groups.io>
@ 2023-12-06 22:50 ` Nate DeSimone
0 siblings, 0 replies; 6+ messages in thread
From: Nate DeSimone @ 2023-12-06 22:50 UTC (permalink / raw)
To: devel@edk2.groups.io, Desimone, Nathaniel L; +Cc: Ni, Ray, Kinney, Michael D
Pushed as df2ec2a
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Nate DeSimone
Sent: Monday, December 4, 2023 10:48 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe
The DXE & MM standalone variant of AcpiTimerLib defines a global named mPerformanceCounterFrequency. A global with an identical name is also present in MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
Since XhciDxe has a dependency on TimerLib, this can cause link errors due to the same symbol being defined twice if the platform DSC chooses to use AcpiTimerLib as the TimerLib implementation for any given platform.
To resolve this, I have changed made the definition of mPerformanceCounterFrequency to static and renamed it to mAcpiTimerLibTscFrequency. Since this variable is not used outside of the DxeStandaloneMmAcpiTimerLib.c compilation unit, there is no reason to have it exported as a global.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
.../AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
index 16ac48938f..ccceb8a649 100644
--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
@@ -1,7 +1,7 @@
/** @file
ACPI Timer implements one instance of Timer Library.
- Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2013 - 2023, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -11,6 +11,11 @@
#include <Library/BaseLib.h>
#include <Library/HobLib.h>
+//
+// Cached performance counter frequency // static UINT64
+mAcpiTimerLibTscFrequency = 0;
+
extern GUID mFrequencyHobGuid;
/**
@@ -48,11 +53,6 @@ InternalCalculateTscFrequency (
VOID
);
-//
-// Cached performance counter frequency -//
-UINT64 mPerformanceCounterFrequency = 0;
-
/**
Internal function to retrieves the 64-bit frequency in Hz.
@@ -66,7 +66,7 @@ InternalGetPerformanceCounterFrequency (
VOID
)
{
- return mPerformanceCounterFrequency;
+ return mAcpiTimerLibTscFrequency;
}
/**
@@ -92,9 +92,9 @@ CommonAcpiTimerLibConstructor (
//
GuidHob = GetFirstGuidHob (&mFrequencyHobGuid);
if (GuidHob != NULL) {
- mPerformanceCounterFrequency = *(UINT64 *)GET_GUID_HOB_DATA (GuidHob);
+ mAcpiTimerLibTscFrequency = *(UINT64 *)GET_GUID_HOB_DATA (GuidHob);
} else {
- mPerformanceCounterFrequency = InternalCalculateTscFrequency ();
+ mAcpiTimerLibTscFrequency = InternalCalculateTscFrequency ();
}
return EFI_SUCCESS;
--
2.39.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112141): https://edk2.groups.io/g/devel/message/112141
Mute This Topic: https://groups.io/mt/103024047/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-06 22:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 18:47 [edk2-devel] [PATCH v2] MdeModulePkg/Bus: Fix XhciDxe Linker Issues Nate DeSimone
2023-12-04 18:47 ` [edk2-devel] [PATCH v2] PcAtChipsetPkg: Fix AcpiTimerLib incompatibility with XhciDxe Nate DeSimone
2023-12-04 19:59 ` Pedro Falcato
2023-12-04 22:13 ` Nate DeSimone
2023-12-05 8:18 ` Ni, Ray
[not found] ` <179DB5683F38928F.24600@groups.io>
2023-12-06 22:50 ` Nate DeSimone
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox