On 6 November 2015 at 17:02, Leif Lindholm <leif.lindh...@linaro.org> wrote: > On Fri, Nov 06, 2015 at 03:03:53PM +0100, Ard Biesheuvel wrote: >> The stride used by the cache maintenance by MVA instructions should >> be retrieved from CTR_EL0.DminLine and CTR_EL0.IminLine, whose values >> reflect the actual geometry of the caches. Using CCSIDR for this purpose >> violates the architecture. > > Does it? > Sure, you'd need to iterate over all levels of I/D/U cache and pick > the smallest, but... If the current code doesn't, then it's broken (as > opposed to violating the architecture). >
Yes, it does. The ARM ARM states quite clearly that the contents of CCSIDR should not be used to make any inferences about the actual cache geometry. >> Also, move the line length accessors to common code, since there is no >> need to keep them separate between ARMv7 and AArch64. > > There is for Arm9 though - the CTR format changed for ARMv7. > So either we turn this back into an Arm9 purge as part of the cache > maintenance fixes (which I would like to avoid) or I think we need to > keep the per-arch line length accessors for now. > Actually, you can't build the ARM9 version anymore, since ArmLib now depends on <Chipset/ArmV7.h> whereas the ARM9 specific code includes <Chipset/ARM926EJ-S.h> as well, resulting in a conflict. I don't think this has been buildable since bd6b97994ab6 ("ArmPkg/ArmLib: Clean ArmV7Lib") dated somewhere back in 2011 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >> Reviewed-by: Mark Rutland <mark.rutl...@arm.com> >> --- >> ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c | 25 ------------------ >> ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.c | 27 -------------------- >> ArmPkg/Library/ArmLib/Common/ArmLib.c | 18 +++++++++++++ >> 3 files changed, 18 insertions(+), 52 deletions(-) >> >> diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c >> b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c >> index 4bea20356fa3..dec125f248cd 100644 >> --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c >> +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.c >> @@ -21,31 +21,6 @@ >> #include "AArch64Lib.h" >> #include "ArmLibPrivate.h" >> >> -UINTN >> -EFIAPI >> -ArmDataCacheLineLength ( >> - VOID >> - ) >> -{ >> - UINT32 CCSIDR = ReadCCSIDR (0) & 7; >> - >> - // * 4 converts to bytes >> - return (1 << (CCSIDR + 2)) * 4; >> -} >> - >> -UINTN >> -EFIAPI >> -ArmInstructionCacheLineLength ( >> - VOID >> - ) >> -{ >> - UINT32 CCSIDR = ReadCCSIDR (1) & 7; >> - >> - // * 4 converts to bytes >> - return (1 << (CCSIDR + 2)) * 4; >> -} >> - >> - >> VOID >> AArch64DataCacheOperation ( >> IN AARCH64_CACHE_OPERATION DataCacheOperation >> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.c >> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.c >> index 44edff869eae..b53f455bfad2 100644 >> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.c >> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Lib.c >> @@ -20,33 +20,6 @@ >> #include "ArmV7Lib.h" >> #include "ArmLibPrivate.h" >> >> -UINTN >> -EFIAPI >> -ArmDataCacheLineLength ( >> - VOID >> - ) >> -{ >> - UINT32 CCSIDR = ReadCCSIDR (0) & 7; >> - >> - // * 4 converts to bytes >> - return (1 << (CCSIDR + 2)) * 4; >> -} >> - >> -UINTN >> -EFIAPI >> -ArmInstructionCacheLineLength ( >> - VOID >> - ) >> -{ >> - UINT32 CCSIDR = ReadCCSIDR (1) & 7; >> - >> - // * 4 converts to bytes >> - return (1 << (CCSIDR + 2)) * 4; >> - >> -// return 64; >> -} >> - >> - >> VOID >> ArmV7DataCacheOperation ( >> IN ARM_V7_CACHE_OPERATION DataCacheOperation >> diff --git a/ArmPkg/Library/ArmLib/Common/ArmLib.c >> b/ArmPkg/Library/ArmLib/Common/ArmLib.c >> index 4febc45220a3..ad0a265e9f59 100644 >> --- a/ArmPkg/Library/ArmLib/Common/ArmLib.c >> +++ b/ArmPkg/Library/ArmLib/Common/ArmLib.c >> @@ -70,3 +70,21 @@ ArmUnsetCpuActlrBit ( >> Value &= ~Bits; >> ArmWriteCpuActlr (Value); >> } >> + >> +UINTN >> +EFIAPI >> +ArmDataCacheLineLength ( >> + VOID >> + ) >> +{ >> + return 4 << ((ArmCacheInfo () >> 16) & 0xf); // CTR_EL0.DminLine >> +} >> + >> +UINTN >> +EFIAPI >> +ArmInstructionCacheLineLength ( >> + VOID >> + ) >> +{ >> + return 4 << (ArmCacheInfo () & 0xf); // CTR_EL0.IminLine >> +} >> -- >> 1.9.1 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel