* [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
2019-04-30 11:47 ` [edk2-devel] " Laszlo Ersek
2019-04-30 1:30 ` [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType Michael D Kinney
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Liming Gao
Add gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType that
uses the PCD type FixedAtBuild. This performs a build time
selection for the type of speculation barrier to use in the
BaseLib function SpeculationBarrier(). The recommended
speculation barrier for x86 is LFENCE and this is the default
value for this PCD. x86 CPUs that do not support LFENCE must
select one of the other supported values which includes CPUID
and nothing.
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
MdePkg/MdePkg.dec | 9 +++++++++
MdePkg/MdePkg.uni | 8 ++++++++
2 files changed, 17 insertions(+)
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index e2ea8fff66..28d4a966c2 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2062,6 +2062,15 @@ [PcdsFixedAtBuild]
# @Prompt Enable control flow enforcement.
gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask|0x0|UINT32|0x30001017
+ ## Indicates the type of instruction sequence to use for a speculation
+ # barrier. The default instruction sequence is LFENCE.<BR><BR>
+ # 0x00 - No operation.<BR>
+ # 0x01 - LFENCE (IA32/X64).<BR>
+ # 0x02 - CPUID (IA32/X64).<BR>
+ # Other - reserved
+ # @Prompt Speculation Barrier Type.
+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01|UINT8|0x30001018
+
[PcdsFixedAtBuild,PcdsPatchableInModule]
## Indicates the maximum length of unicode string used in the following
# BaseLib functions: StrLen(), StrSize(), StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
index c359bb4b5b..5c1fa24065 100644
--- a/MdePkg/MdePkg.uni
+++ b/MdePkg/MdePkg.uni
@@ -149,6 +149,14 @@
" BIT0 - SMM CET Shadow Stack is enabled.<BR>\n"
" Other - reserved"
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_PROMPT #language en-US "Speculation Barrier Type."
+
+#string STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_HELP #language en-US "Indicates the type of instruction sequence to use for a speculation.barrier. The default instruction sequence is LFENCE.<BR><BR>\n"
+ "0x00 - No operation.<BR>\n"
+ "0x01 - LFENCE (IA32/X64).<BR>\n"
+ "0x02 - CPUID (IA32/X64).<BR>\n"
+ "Other - reserved"
+
#string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_PROMPT #language en-US "Maximum Length of Ascii String"
#string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_HELP #language en-US "Sets the maximum number of ASCII characters used for string functions. This affects the following BaseLib functions: AsciiStrLen(), AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(), AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
2019-04-30 1:30 ` [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType Michael D Kinney
@ 2019-04-30 11:47 ` Laszlo Ersek
2019-04-30 15:16 ` Michael D Kinney
0 siblings, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2019-04-30 11:47 UTC (permalink / raw)
To: devel, michael.d.kinney; +Cc: Liming Gao
On 04/30/19 03:30, Michael D Kinney wrote:
> Add gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType that
> uses the PCD type FixedAtBuild. This performs a build time
> selection for the type of speculation barrier to use in the
> BaseLib function SpeculationBarrier(). The recommended
> speculation barrier for x86 is LFENCE and this is the default
> value for this PCD. x86 CPUs that do not support LFENCE must
> select one of the other supported values which includes CPUID
> and nothing.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> MdePkg/MdePkg.dec | 9 +++++++++
> MdePkg/MdePkg.uni | 8 ++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index e2ea8fff66..28d4a966c2 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -2062,6 +2062,15 @@ [PcdsFixedAtBuild]
> # @Prompt Enable control flow enforcement.
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask|0x0|UINT32|0x30001017
>
> + ## Indicates the type of instruction sequence to use for a speculation
> + # barrier. The default instruction sequence is LFENCE.<BR><BR>
> + # 0x00 - No operation.<BR>
> + # 0x01 - LFENCE (IA32/X64).<BR>
> + # 0x02 - CPUID (IA32/X64).<BR>
> + # Other - reserved
> + # @Prompt Speculation Barrier Type.
> + gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01|UINT8|0x30001018
> +
> [PcdsFixedAtBuild,PcdsPatchableInModule]
> ## Indicates the maximum length of unicode string used in the following
> # BaseLib functions: StrLen(), StrSize(), StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
In MdePkg.dec, we have:
- [Includes.X64]
- [LibraryClasses.X64]
- [Guids.X64]
but no PCD declarations that are architecture-specific. Is that
intentional? Because, this PCD could be a good candidate for "IA32/X64
only". (Looking at the next patch too.)
But, that's just my curiosity.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
> index c359bb4b5b..5c1fa24065 100644
> --- a/MdePkg/MdePkg.uni
> +++ b/MdePkg/MdePkg.uni
> @@ -149,6 +149,14 @@
> " BIT0 - SMM CET Shadow Stack is enabled.<BR>\n"
> " Other - reserved"
>
> +#string STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_PROMPT #language en-US "Speculation Barrier Type."
> +
> +#string STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_HELP #language en-US "Indicates the type of instruction sequence to use for a speculation.barrier. The default instruction sequence is LFENCE.<BR><BR>\n"
> + "0x00 - No operation.<BR>\n"
> + "0x01 - LFENCE (IA32/X64).<BR>\n"
> + "0x02 - CPUID (IA32/X64).<BR>\n"
> + "Other - reserved"
> +
> #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_PROMPT #language en-US "Maximum Length of Ascii String"
>
> #string STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLength_HELP #language en-US "Sets the maximum number of ASCII characters used for string functions. This affects the following BaseLib functions: AsciiStrLen(), AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(), AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
2019-04-30 11:47 ` [edk2-devel] " Laszlo Ersek
@ 2019-04-30 15:16 ` Michael D Kinney
2019-04-30 15:43 ` Laszlo Ersek
0 siblings, 1 reply; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 15:16 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Kinney, Michael D; +Cc: Gao, Liming
Laszlo,
I tried to design this PCD so it could be used for other
architectures as needed in the future by expanding the enum.
I marked enum values 0x01(LFENCE) and 0x02(CPUID) for
IA32/X64. Value 0x00 (NOP) is for all archs.
Mike
> -----Original Message-----
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Tuesday, April 30, 2019 4:47 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add
> PcdSpeculationBarrierType
>
> On 04/30/19 03:30, Michael D Kinney wrote:
> > Add
> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType that
> > uses the PCD type FixedAtBuild. This performs a
> build time
> > selection for the type of speculation barrier to use
> in the
> > BaseLib function SpeculationBarrier(). The
> recommended
> > speculation barrier for x86 is LFENCE and this is the
> default
> > value for this PCD. x86 CPUs that do not support
> LFENCE must
> > select one of the other supported values which
> includes CPUID
> > and nothing.
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> > MdePkg/MdePkg.dec | 9 +++++++++
> > MdePkg/MdePkg.uni | 8 ++++++++
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index e2ea8fff66..28d4a966c2 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -2062,6 +2062,15 @@ [PcdsFixedAtBuild]
> > # @Prompt Enable control flow enforcement.
> >
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPrope
> rtyMask|0x0|UINT32|0x30001017
> >
> > + ## Indicates the type of instruction sequence to
> use for a speculation
> > + # barrier. The default instruction sequence is
> LFENCE.<BR><BR>
> > + # 0x00 - No operation.<BR>
> > + # 0x01 - LFENCE (IA32/X64).<BR>
> > + # 0x02 - CPUID (IA32/X64).<BR>
> > + # Other - reserved
> > + # @Prompt Speculation Barrier Type.
> > +
> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01
> |UINT8|0x30001018
> > +
> > [PcdsFixedAtBuild,PcdsPatchableInModule]
> > ## Indicates the maximum length of unicode string
> used in the following
> > # BaseLib functions: StrLen(), StrSize(),
> StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
>
> In MdePkg.dec, we have:
> - [Includes.X64]
> - [LibraryClasses.X64]
> - [Guids.X64]
>
> but no PCD declarations that are architecture-specific.
> Is that
> intentional? Because, this PCD could be a good
> candidate for "IA32/X64
> only". (Looking at the next patch too.)
>
> But, that's just my curiosity.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
>
>
> > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
> > index c359bb4b5b..5c1fa24065 100644
> > --- a/MdePkg/MdePkg.uni
> > +++ b/MdePkg/MdePkg.uni
> > @@ -149,6 +149,14 @@
> >
> " BIT0 - SMM CET Shadow Stack is enabled.<BR>\n"
> >
> " Other - reserved"
> >
> > +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
> PROMPT #language en-US "Speculation Barrier Type."
> > +
> > +#string
> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
> HELP #language en-US "Indicates the type of
> instruction sequence to use for a speculation.barrier.
> The default instruction sequence is LFENCE.<BR><BR>\n"
> > +
> "0x00 - No operation.<BR>\n"
> > +
> "0x01 - LFENCE (IA32/X64).<BR>\n"
> > +
> "0x02 - CPUID (IA32/X64).<BR>\n"
> > +
> "Other - reserved"
> > +
> > #string
> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
> h_PROMPT #language en-US "Maximum Length of Ascii
> String"
> >
> > #string
> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
> h_HELP #language en-US "Sets the maximum number of
> ASCII characters used for string functions. This
> affects the following BaseLib functions: AsciiStrLen(),
> AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(),
> AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
> >
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType
2019-04-30 15:16 ` Michael D Kinney
@ 2019-04-30 15:43 ` Laszlo Ersek
0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2019-04-30 15:43 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Gao, Liming
On 04/30/19 17:16, Kinney, Michael D wrote:
> Laszlo,
>
> I tried to design this PCD so it could be used for other
> architectures as needed in the future by expanding the enum.
> I marked enum values 0x01(LFENCE) and 0x02(CPUID) for
> IA32/X64. Value 0x00 (NOP) is for all archs.
Ah, good point. In fact, this has more or less crossed my mind, but I
ruled out the idea, as (I thought) a multi-arch PCD would have to be a
bitmap, not a simple enum.
Of course, I was wrong about that -- in any given platform build, the
PCD doesn't have to contain the right setting for every possible
architecture supported by edk2. It only need contain the right setting
for the arch of the current platform build.
So yes, this design is great; please apply my R-b.
Thanks
Laszlo
>> -----Original Message-----
>> From: devel@edk2.groups.io
>> [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
>> Sent: Tuesday, April 30, 2019 4:47 AM
>> To: devel@edk2.groups.io; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Cc: Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2-devel] [Patch V2 1/6] MdePkg: Add
>> PcdSpeculationBarrierType
>>
>> On 04/30/19 03:30, Michael D Kinney wrote:
>>> Add
>> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType that
>>> uses the PCD type FixedAtBuild. This performs a
>> build time
>>> selection for the type of speculation barrier to use
>> in the
>>> BaseLib function SpeculationBarrier(). The
>> recommended
>>> speculation barrier for x86 is LFENCE and this is the
>> default
>>> value for this PCD. x86 CPUs that do not support
>> LFENCE must
>>> select one of the other supported values which
>> includes CPUID
>>> and nothing.
>>>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Signed-off-by: Michael D Kinney
>> <michael.d.kinney@intel.com>
>>> ---
>>> MdePkg/MdePkg.dec | 9 +++++++++
>>> MdePkg/MdePkg.uni | 8 ++++++++
>>> 2 files changed, 17 insertions(+)
>>>
>>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
>>> index e2ea8fff66..28d4a966c2 100644
>>> --- a/MdePkg/MdePkg.dec
>>> +++ b/MdePkg/MdePkg.dec
>>> @@ -2062,6 +2062,15 @@ [PcdsFixedAtBuild]
>>> # @Prompt Enable control flow enforcement.
>>>
>> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPrope
>> rtyMask|0x0|UINT32|0x30001017
>>>
>>> + ## Indicates the type of instruction sequence to
>> use for a speculation
>>> + # barrier. The default instruction sequence is
>> LFENCE.<BR><BR>
>>> + # 0x00 - No operation.<BR>
>>> + # 0x01 - LFENCE (IA32/X64).<BR>
>>> + # 0x02 - CPUID (IA32/X64).<BR>
>>> + # Other - reserved
>>> + # @Prompt Speculation Barrier Type.
>>> +
>> gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01
>> |UINT8|0x30001018
>>> +
>>> [PcdsFixedAtBuild,PcdsPatchableInModule]
>>> ## Indicates the maximum length of unicode string
>> used in the following
>>> # BaseLib functions: StrLen(), StrSize(),
>> StrCmp(), StrnCmp(), StrCpy(), StrnCpy()<BR><BR>
>>
>> In MdePkg.dec, we have:
>> - [Includes.X64]
>> - [LibraryClasses.X64]
>> - [Guids.X64]
>>
>> but no PCD declarations that are architecture-specific.
>> Is that
>> intentional? Because, this PCD could be a good
>> candidate for "IA32/X64
>> only". (Looking at the next patch too.)
>>
>> But, that's just my curiosity.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Thanks
>> Laszlo
>>
>>
>>> diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni
>>> index c359bb4b5b..5c1fa24065 100644
>>> --- a/MdePkg/MdePkg.uni
>>> +++ b/MdePkg/MdePkg.uni
>>> @@ -149,6 +149,14 @@
>>>
>> " BIT0 - SMM CET Shadow Stack is enabled.<BR>\n"
>>>
>> " Other - reserved"
>>>
>>> +#string
>> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
>> PROMPT #language en-US "Speculation Barrier Type."
>>> +
>>> +#string
>> STR_gEfiMdePkgTokenSpaceGuid_PcdSpeculationBarrierType_
>> HELP #language en-US "Indicates the type of
>> instruction sequence to use for a speculation.barrier.
>> The default instruction sequence is LFENCE.<BR><BR>\n"
>>> +
>> "0x00 - No operation.<BR>\n"
>>> +
>> "0x01 - LFENCE (IA32/X64).<BR>\n"
>>> +
>> "0x02 - CPUID (IA32/X64).<BR>\n"
>>> +
>> "Other - reserved"
>>> +
>>> #string
>> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
>> h_PROMPT #language en-US "Maximum Length of Ascii
>> String"
>>>
>>> #string
>> STR_gEfiMdePkgTokenSpaceGuid_PcdMaximumAsciiStringLengt
>> h_HELP #language en-US "Sets the maximum number of
>> ASCII characters used for string functions. This
>> affects the following BaseLib functions: AsciiStrLen(),
>> AsciiStrSize(), AsciiStrCmp(), AsciiStrnCmp(),
>> AsciiStrCpy(), AsciiStrnCpy(). <BR><BR>\n"
>>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
2019-04-30 1:30 ` [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
2019-04-30 11:49 ` [edk2-devel] " Laszlo Ersek
2019-04-30 22:24 ` Brian J. Johnson
2019-04-30 1:30 ` [Patch V2 3/6] QuarkPlatformPkg: Set PcdSpeculationBarrierType to CPUID Michael D Kinney
` (3 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Liming Gao
Use PcdSpeculationBarrierType in the x86 implementation
of SpeculationBarrier() to select between AsmLfence(),
AsmCpuid(), and no operation.
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
MdePkg/Library/BaseLib/BaseLib.inf | 1 +
MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index 533e83e0b2..3586beb0ab 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -394,6 +394,7 @@ [Pcd]
gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## SOMETIMES_CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## SOMETIMES_CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## SOMETIMES_CONSUMES
+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES
diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
index 8e5f983bb8..b28fd8de9b 100644
--- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
+++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
@@ -1,7 +1,7 @@
/** @file
SpeculationBarrier() function for IA32 and x64.
- Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -22,5 +22,9 @@ SpeculationBarrier (
VOID
)
{
- AsmLfence ();
+ if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
+ AsmLfence ();
+ } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
+ AsmCpuid (0x01, NULL, NULL, NULL, NULL);
+ }
}
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
2019-04-30 1:30 ` [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType Michael D Kinney
@ 2019-04-30 11:49 ` Laszlo Ersek
2019-04-30 15:17 ` Michael D Kinney
2019-04-30 22:24 ` Brian J. Johnson
1 sibling, 1 reply; 14+ messages in thread
From: Laszlo Ersek @ 2019-04-30 11:49 UTC (permalink / raw)
To: devel, michael.d.kinney; +Cc: Liming Gao
On 04/30/19 03:30, Michael D Kinney wrote:
> Use PcdSpeculationBarrierType in the x86 implementation
> of SpeculationBarrier() to select between AsmLfence(),
> AsmCpuid(), and no operation.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> MdePkg/Library/BaseLib/BaseLib.inf | 1 +
> MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 533e83e0b2..3586beb0ab 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -394,6 +394,7 @@ [Pcd]
> gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## SOMETIMES_CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## SOMETIMES_CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## SOMETIMES_CONSUMES
> + gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## SOMETIMES_CONSUMES
>
> [FeaturePcd]
> gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES
So, based on the example of
MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf
and a few other INF files, I think we could use
[Pcd.IA32, Pcd.X64]
here as well.
Just an idea.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> index 8e5f983bb8..b28fd8de9b 100644
> --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> @@ -1,7 +1,7 @@
> /** @file
> SpeculationBarrier() function for IA32 and x64.
>
> - Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -22,5 +22,9 @@ SpeculationBarrier (
> VOID
> )
> {
> - AsmLfence ();
> + if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> + AsmLfence ();
> + } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
> + AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> + }
> }
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
2019-04-30 11:49 ` [edk2-devel] " Laszlo Ersek
@ 2019-04-30 15:17 ` Michael D Kinney
0 siblings, 0 replies; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 15:17 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, Kinney, Michael D; +Cc: Gao, Liming
Laszlo,
I agree the current usage is only for IA32/X64 source
files so your suggestion make sense. If there is
a use case for other archs in the future for this PCD,
then the INF would have to be updated.
Mike
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, April 30, 2019 4:50 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [Patch V2 2/6]
> MdePkg/BaseLib: Use PcdSpeculationBarrierType
>
> On 04/30/19 03:30, Michael D Kinney wrote:
> > Use PcdSpeculationBarrierType in the x86
> implementation
> > of SpeculationBarrier() to select between
> AsmLfence(),
> > AsmCpuid(), and no operation.
> >
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> > MdePkg/Library/BaseLib/BaseLib.inf | 1 +
> > MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8
> ++++++--
> > 2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index 533e83e0b2..3586beb0ab 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -394,6 +394,7 @@ [Pcd]
> >
> gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength
> ## SOMETIMES_CONSUMES
> >
> gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength
> ## SOMETIMES_CONSUMES
> >
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPrope
> rtyMask ## SOMETIMES_CONSUMES
> > + gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType
> ## SOMETIMES_CONSUMES
> >
> > [FeaturePcd]
> > gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ##
> CONSUMES
>
> So, based on the example of
>
>
> MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCp
> u.inf
>
> and a few other INF files, I think we could use
>
> [Pcd.IA32, Pcd.X64]
>
> here as well.
>
> Just an idea.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
>
> > diff --git
> a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > index 8e5f983bb8..b28fd8de9b 100644
> > --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> > @@ -1,7 +1,7 @@
> > /** @file
> > SpeculationBarrier() function for IA32 and x64.
> >
> > - Copyright (C) 2018, Intel Corporation. All rights
> reserved.<BR>
> > + Copyright (C) 2018 - 2019, Intel Corporation. All
> rights reserved.<BR>
> >
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -22,5 +22,9 @@ SpeculationBarrier (
> > VOID
> > )
> > {
> > - AsmLfence ();
> > + if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> > + AsmLfence ();
> > + } else if (PcdGet8 (PcdSpeculationBarrierType) ==
> 0x02) {
> > + AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> > + }
> > }
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType
2019-04-30 1:30 ` [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType Michael D Kinney
2019-04-30 11:49 ` [edk2-devel] " Laszlo Ersek
@ 2019-04-30 22:24 ` Brian J. Johnson
1 sibling, 0 replies; 14+ messages in thread
From: Brian J. Johnson @ 2019-04-30 22:24 UTC (permalink / raw)
To: devel, michael.d.kinney; +Cc: Liming Gao
On 4/29/19 8:30 PM, Michael D Kinney wrote:
> Use PcdSpeculationBarrierType in the x86 implementation
> of SpeculationBarrier() to select between AsmLfence(),
> AsmCpuid(), and no operation.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> MdePkg/Library/BaseLib/BaseLib.inf | 1 +
> MdePkg/Library/BaseLib/X86SpeculationBarrier.c | 8 ++++++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
> index 533e83e0b2..3586beb0ab 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -394,6 +394,7 @@ [Pcd]
> gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength ## SOMETIMES_CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength ## SOMETIMES_CONSUMES
> gEfiMdePkgTokenSpaceGuid.PcdControlFlowEnforcementPropertyMask ## SOMETIMES_CONSUMES
> + gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType ## SOMETIMES_CONSUMES
>
> [FeaturePcd]
> gEfiMdePkgTokenSpaceGuid.PcdVerifyNodeInList ## CONSUMES
> diff --git a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> index 8e5f983bb8..b28fd8de9b 100644
> --- a/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> +++ b/MdePkg/Library/BaseLib/X86SpeculationBarrier.c
> @@ -1,7 +1,7 @@
> /** @file
> SpeculationBarrier() function for IA32 and x64.
>
> - Copyright (C) 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (C) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -22,5 +22,9 @@ SpeculationBarrier (
> VOID
> )
> {
> - AsmLfence ();
> + if (PcdGet8 (PcdSpeculationBarrierType) == 0x01) {
> + AsmLfence ();
> + } else if (PcdGet8 (PcdSpeculationBarrierType) == 0x02) {
> + AsmCpuid (0x01, NULL, NULL, NULL, NULL);
> + }
> }
>
Looks good. I'm not a maintainer, but FWIW:
Reviewed-by: Brian J. Johnson <brian.johnson@hpe.com>
--
Brian J. Johnson
Enterprise X86 Lab
Hewlett Packard Enterprise
brian.johnson@hpe.com
+1 651 683 7521 Office
Eagan, MN
hpe.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Patch V2 3/6] QuarkPlatformPkg: Set PcdSpeculationBarrierType to CPUID
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
2019-04-30 1:30 ` [Patch V2 1/6] MdePkg: Add PcdSpeculationBarrierType Michael D Kinney
2019-04-30 1:30 ` [Patch V2 2/6] MdePkg/BaseLib: Use PcdSpeculationBarrierType Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
2019-04-30 1:30 ` [Patch V2 4/6] UefiCpuPkg/MpInitLib: Avoid MSR_IA32_APIC_BASE for single core Michael D Kinney
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Kelly Steele
Set PcdSpeculationBarrierType to use CPUID instead of the
default LFENCE in the BaseLib function SpeculationBarrier().
LFENCE requires SSE2, and Quark platforms do not support
SSE2.
Cc: Kelly Steele <kelly.steele@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
QuarkPlatformPkg/Quark.dsc | 7 ++++++-
QuarkPlatformPkg/QuarkMin.dsc | 5 +++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/QuarkPlatformPkg/Quark.dsc b/QuarkPlatformPkg/Quark.dsc
index 422fd9cf8d..96ddc1565a 100644
--- a/QuarkPlatformPkg/Quark.dsc
+++ b/QuarkPlatformPkg/Quark.dsc
@@ -2,7 +2,7 @@
# Clanton Peak CRB platform with 32-bit DXE for 4MB/8MB flash devices.
#
# This package provides Clanton Peak CRB platform specific modules.
-# Copyright (c) 2013 - 2018 Intel Corporation.
+# Copyright (c) 2013 - 2019 Intel Corporation.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -448,6 +448,11 @@ [PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdRecoveryFileName|L"QUARKREC.Cap"
!endif
+ #
+ # Quark does not support LFENCE. Use CPUID as speculation barrier
+ #
+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x02
+
[PcdsPatchableInModule]
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x803000C7
gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
diff --git a/QuarkPlatformPkg/QuarkMin.dsc b/QuarkPlatformPkg/QuarkMin.dsc
index 00e2febb54..8ca75bc474 100644
--- a/QuarkPlatformPkg/QuarkMin.dsc
+++ b/QuarkPlatformPkg/QuarkMin.dsc
@@ -406,6 +406,11 @@ [PcdsFixedAtBuild]
gEfiMdeModulePkgTokenSpaceGuid.PcdConInConnectOnDemand|FALSE
+ #
+ # Quark does not support LFENCE. Use CPUID as speculation barrier
+ #
+ gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x02
+
[PcdsPatchableInModule]
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x803000C7
gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Patch V2 4/6] UefiCpuPkg/MpInitLib: Avoid MSR_IA32_APIC_BASE for single core
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
` (2 preceding siblings ...)
2019-04-30 1:30 ` [Patch V2 3/6] QuarkPlatformPkg: Set PcdSpeculationBarrierType to CPUID Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
2019-04-30 10:31 ` Laszlo Ersek
2019-04-30 1:30 ` [Patch V2 5/6] QuarkSocPkg/SmmAccessDxe: Set region to UC on SMRAM close Michael D Kinney
2019-04-30 1:30 ` [Patch V2 6/6] QuarkPlatformPkg/PlatformInit: Resolve ResetSystemLib name collision Michael D Kinney
5 siblings, 1 reply; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek
Avoid access to MSR_IA32_APIC_BASE that may not be supported
on single core CPUs. If PcdCpuMaxLogicalProcessorNumber is 1,
then there is only one CPU that must be the BSP.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 35dff91fd2..5488049c08 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -1,7 +1,7 @@
/** @file
MP initialize support functions for PEI phase.
- Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -101,6 +101,19 @@ GetCpuMpData (
MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
IA32_DESCRIPTOR Idtr;
+ //
+ // If there is only 1 CPU, then it must be the BSP. This avoids an access to
+ // MSR_IA32_APIC_BASE that may not be supported on single core CPUs.
+ //
+ if (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) == 1) {
+ CpuMpData = GetCpuMpDataFromGuidedHob ();
+ ASSERT (CpuMpData != NULL);
+ return CpuMpData;
+ }
+
+ //
+ // Otherwise use MSR_IA32_APIC_BASE to determine if the CPU is BSP or AP.
+ //
ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
if (ApicBaseMsr.Bits.BSP == 1) {
CpuMpData = GetCpuMpDataFromGuidedHob ();
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Patch V2 4/6] UefiCpuPkg/MpInitLib: Avoid MSR_IA32_APIC_BASE for single core
2019-04-30 1:30 ` [Patch V2 4/6] UefiCpuPkg/MpInitLib: Avoid MSR_IA32_APIC_BASE for single core Michael D Kinney
@ 2019-04-30 10:31 ` Laszlo Ersek
0 siblings, 0 replies; 14+ messages in thread
From: Laszlo Ersek @ 2019-04-30 10:31 UTC (permalink / raw)
To: Michael D Kinney, devel; +Cc: Eric Dong, Ray Ni
Hi Mike,
On 04/30/19 03:30, Michael D Kinney wrote:
> Avoid access to MSR_IA32_APIC_BASE that may not be supported
> on single core CPUs. If PcdCpuMaxLogicalProcessorNumber is 1,
> then there is only one CPU that must be the BSP.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 35dff91fd2..5488049c08 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -1,7 +1,7 @@
> /** @file
> MP initialize support functions for PEI phase.
>
> - Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -101,6 +101,19 @@ GetCpuMpData (
> MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr;
> IA32_DESCRIPTOR Idtr;
>
> + //
> + // If there is only 1 CPU, then it must be the BSP. This avoids an access to
> + // MSR_IA32_APIC_BASE that may not be supported on single core CPUs.
> + //
> + if (PcdGet32 (PcdCpuMaxLogicalProcessorNumber) == 1) {
this PcdGet32() call has the same issue as in v1. It is unsafe because:
- if the platform makes PcdCpuMaxLogicalProcessorNumber a dynamic or
dynamic-ex PCD, then the above call will result in a PPI member function
invocation,
- and said invocation may be executed on an AP, not necessarily on the BSP.
But, APs must not call PPI member functions, to my knowledge.
We can certainly use a PCD here, but both UefiCpuPkg.dec, and the code
added here, must prevent platforms from making that PCD dynamic or
dynamic-ex.
Thanks
Laszlo
> + CpuMpData = GetCpuMpDataFromGuidedHob ();
> + ASSERT (CpuMpData != NULL);
> + return CpuMpData;
> + }
> +
> + //
> + // Otherwise use MSR_IA32_APIC_BASE to determine if the CPU is BSP or AP.
> + //
> ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
> if (ApicBaseMsr.Bits.BSP == 1) {
> CpuMpData = GetCpuMpDataFromGuidedHob ();
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Patch V2 5/6] QuarkSocPkg/SmmAccessDxe: Set region to UC on SMRAM close
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
` (3 preceding siblings ...)
2019-04-30 1:30 ` [Patch V2 4/6] UefiCpuPkg/MpInitLib: Avoid MSR_IA32_APIC_BASE for single core Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
2019-04-30 1:30 ` [Patch V2 6/6] QuarkPlatformPkg/PlatformInit: Resolve ResetSystemLib name collision Michael D Kinney
5 siblings, 0 replies; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Kelly Steele
The following commit removed the unconditional UC setting
just prior to closing the SMRAM region. This is a correct
change for most platforms.
https://github.com/tianocore/edk2/commit/bfc87aa78e77ed15b09d1b4499c5eab63e8842bb
The Quark platforms still require this UC setting, so move
the UC setting into the Quark specific SMM Access Protocol
when the Close() service is called.
Cc: Kelly Steele <kelly.steele@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Kelly Steele <kelly.steele@intel.com>
---
.../Smm/Dxe/SmmAccessDxe/SmmAccess.inf | 3 ++-
.../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c | 18 +++++++++++++++++-
.../Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h | 3 ++-
3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
index db916f686a..405e9eb7fd 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccess.inf
@@ -1,7 +1,7 @@
## @file
# Component description file for SmmAccess module
#
-# Copyright (c) 2013-2015 Intel Corporation.
+# Copyright (c) 2013-2019 Intel Corporation.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -34,6 +34,7 @@ [LibraryClasses]
S3BootScriptLib
UefiDriverEntryPoint
UefiBootServicesTableLib
+ DxeServicesTableLib
PcdLib
SmmLib
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c
index 6148dea1b4..830f8b83c3 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.c
@@ -2,7 +2,7 @@
This is the driver that publishes the SMM Access Protocol
instance for the Tylersburg chipset.
-Copyright (c) 2013-2015 Intel Corporation.
+Copyright (c) 2013-2019 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -221,6 +221,7 @@ Returns:
--*/
{
+ EFI_STATUS Status;
SMM_ACCESS_PRIVATE_DATA *SmmAccess;
BOOLEAN OpenState;
UINTN Index;
@@ -239,6 +240,21 @@ Returns:
return EFI_DEVICE_ERROR;
}
+ //
+ // Reset SMRAM cacheability to UC
+ //
+ for (Index = 0; Index < mSmmAccess.NumberRegions; Index++) {
+ DEBUG ((DEBUG_INFO, "SmmAccess->Close: Set to UC Base=%016lx Size=%016lx\n", SmmAccess->SmramDesc[Index].CpuStart, SmmAccess->SmramDesc[Index].PhysicalSize));
+ Status = gDS->SetMemorySpaceAttributes(
+ SmmAccess->SmramDesc[Index].CpuStart,
+ SmmAccess->SmramDesc[Index].PhysicalSize,
+ EFI_MEMORY_UC
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "SmmAccess: Failed to reset SMRAM window to EFI_MEMORY_UC\n"));
+ }
+ }
+
//
// Close TSEG
//
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h
index 80f73ba0e3..aca169d3e2 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/Dxe/SmmAccessDxe/SmmAccessDriver.h
@@ -3,7 +3,7 @@ Header file for SMM Access Driver.
This file includes package header files, library classes and protocol, PPI & GUID definitions.
-Copyright (c) 2013-2015 Intel Corporation.
+Copyright (c) 2013-2019 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -21,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseMemoryLib.h>
#include <Library/UefiDriverEntryPoint.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesTableLib.h>
#include <Library/PcdLib.h>
//
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Patch V2 6/6] QuarkPlatformPkg/PlatformInit: Resolve ResetSystemLib name collision
2019-04-30 1:30 [Patch V2 0/6] Resolve Quark build and boot issues Michael D Kinney
` (4 preceding siblings ...)
2019-04-30 1:30 ` [Patch V2 5/6] QuarkSocPkg/SmmAccessDxe: Set region to UC on SMRAM close Michael D Kinney
@ 2019-04-30 1:30 ` Michael D Kinney
5 siblings, 0 replies; 14+ messages in thread
From: Michael D Kinney @ 2019-04-30 1:30 UTC (permalink / raw)
To: devel; +Cc: Kelly Steele
Change function name from ResetSystem() to PlatformResetSystem()
to resolve name collision with ResetSystemLib.
Cc: Kelly Steele <kelly.steele@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Kelly Steele <kelly.steele@intel.com>
---
QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c | 6 +++---
.../Platform/Pei/PlatformInit/PlatformEarlyInit.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c b/QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c
index cc2a6674d0..cfdcba8e02 100644
--- a/QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c
+++ b/QuarkPlatformPkg/Platform/Pei/PlatformInit/MemoryCallback.c
@@ -7,7 +7,7 @@ following action is performed in this file,
4. Set MTRR for PEI
5. Create FV HOB and Flash HOB
-Copyright (c) 2013 - 2016, Intel Corporation.
+Copyright (c) 2013 - 2019, Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -20,7 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
extern EFI_PEI_PPI_DESCRIPTOR mPpiStall[];
-EFI_PEI_RESET_PPI mResetPpi = { ResetSystem };
+EFI_PEI_RESET_PPI mResetPpi = { PlatformResetSystem };
EFI_PEI_PPI_DESCRIPTOR mPpiList[1] = {
{
@@ -40,7 +40,7 @@ EFI_PEI_PPI_DESCRIPTOR mPpiList[1] = {
**/
EFI_STATUS
EFIAPI
-ResetSystem (
+PlatformResetSystem (
IN CONST EFI_PEI_SERVICES **PeiServices
)
{
diff --git a/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarlyInit.h b/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarlyInit.h
index 6792538d42..84def44717 100644
--- a/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarlyInit.h
+++ b/QuarkPlatformPkg/Platform/Pei/PlatformInit/PlatformEarlyInit.h
@@ -1,7 +1,7 @@
/** @file
The header file of Platform PEIM.
-Copyright (c) 2013 Intel Corporation.
+Copyright (c) 2013 - 2019 Intel Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -59,7 +59,7 @@ UpdateBootMode (
**/
EFI_STATUS
EFIAPI
-ResetSystem (
+PlatformResetSystem (
IN CONST EFI_PEI_SERVICES **PeiServices
);
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 14+ messages in thread