On 6 November 2015 at 17:02, Leif Lindholm <[email protected]> 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 <[email protected]>
>> Reviewed-by: Mark Rutland <[email protected]>
>> ---
>> 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
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
