On 6 November 2015 at 17:13, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
> 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.
>
And yes, it is indeed broken as well since it only looks at the line
length of the L1.
>>> 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
