| Message ID | 1472475023-2996-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | d683a424f7f11cb46f94cfec6b153001bc9da2cc |
| Headers | show |
On Mon, Aug 29, 2016 at 01:50:23PM +0100, Ard Biesheuvel wrote: > The PCD PcdArmPrimaryCoreMask is of dubious value, not only because it is > unclear what the point is of masking out bits before comparing a given > MPIDR_EL1 value with a known value belonging to the primary core, but also > because the default value of 0xF03 masks out bits that belong to the actual > affinity levels. > > For Styx, simply ignore the mask altogether. Since its version of > ArmPlatformPeiBootAction () records the primary MPIDR_EL1 value upon > first entry, we know its exact value, and no masking is necessary. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Looks sane to me. Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > > Hello Alan, > > Any chance you could try this patch, and check whether it solves the > regression you have been experiencing? I'd try it myself it it weren't > for the fact that my SPI flash programmer is broken atm. > > Thanks, > Ard. > > > Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S | 3 --- > Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf | 1 - > Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf | 1 - > 3 files changed, 5 deletions(-) > > diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S > index a3ac60282706..b7ec02f0e69f 100644 > --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S > +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S > @@ -58,9 +58,6 @@ ASM_FUNC(ArmGetCpuCountPerCluster) > // IN UINTN MpId > // ); > ASM_FUNC(ArmPlatformIsPrimaryCore) > - MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask)) > - and x0, x0, x1 > - > ldr w1, PrimaryCoreMpid > > cmp w0, w1 > diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf > index 4a6469ee016c..8e9169161d16 100644 > --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf > +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf > @@ -67,7 +67,6 @@ > gAmdStyxTokenSpaceGuid.PcdTrustedFWMemorySize > > [FixedPcd] > - gArmTokenSpaceGuid.PcdArmPrimaryCoreMask > gArmTokenSpaceGuid.PcdArmPrimaryCore > > gArmPlatformTokenSpaceGuid.PcdCoreCount > diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf > index 0b9b6287168d..2a2d6ffb8c64 100644 > --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf > +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf > @@ -62,7 +62,6 @@ > gArmTokenSpaceGuid.PcdFvBaseAddress > > [FixedPcd] > - gArmTokenSpaceGuid.PcdArmPrimaryCoreMask > gArmTokenSpaceGuid.PcdArmPrimaryCore > > gArmPlatformTokenSpaceGuid.PcdCoreCount > -- > 2.7.4 >
diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S index a3ac60282706..b7ec02f0e69f 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S @@ -58,9 +58,6 @@ ASM_FUNC(ArmGetCpuCountPerCluster) // IN UINTN MpId // ); ASM_FUNC(ArmPlatformIsPrimaryCore) - MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask)) - and x0, x0, x1 - ldr w1, PrimaryCoreMpid cmp w0, w1 diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf index 4a6469ee016c..8e9169161d16 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf @@ -67,7 +67,6 @@ gAmdStyxTokenSpaceGuid.PcdTrustedFWMemorySize [FixedPcd] - gArmTokenSpaceGuid.PcdArmPrimaryCoreMask gArmTokenSpaceGuid.PcdArmPrimaryCore gArmPlatformTokenSpaceGuid.PcdCoreCount diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf index 0b9b6287168d..2a2d6ffb8c64 100644 --- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf +++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf @@ -62,7 +62,6 @@ gArmTokenSpaceGuid.PcdFvBaseAddress [FixedPcd] - gArmTokenSpaceGuid.PcdArmPrimaryCoreMask gArmTokenSpaceGuid.PcdArmPrimaryCore gArmPlatformTokenSpaceGuid.PcdCoreCount
The PCD PcdArmPrimaryCoreMask is of dubious value, not only because it is unclear what the point is of masking out bits before comparing a given MPIDR_EL1 value with a known value belonging to the primary core, but also because the default value of 0xF03 masks out bits that belong to the actual affinity levels. For Styx, simply ignore the mask altogether. Since its version of ArmPlatformPeiBootAction () records the primary MPIDR_EL1 value upon first entry, we know its exact value, and no masking is necessary. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Hello Alan, Any chance you could try this patch, and check whether it solves the regression you have been experiencing? I'd try it myself it it weren't for the fact that my SPI flash programmer is broken atm. Thanks, Ard. Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S | 3 --- Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf | 1 - Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf | 1 - 3 files changed, 5 deletions(-)