Re: [PATCH 2/2] ARM: at91: setup outer cache .write_sec() callback if needed

[Claudiu.Beznea]

On 04.07.2022 09:39, Claudiu Beznea - M18063 wrote:
> On 06.06.2022 17:57, Clément Léger wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> When running under OP-TEE, the L2 cache is configured by OP-TEE and the
>> sam platform code does not allow any modification yet. Setup a dummy
>> .write_sec() callback to avoid triggering exceptions when Linux tries
>> to modify the L2 cache configuration.
>>
>> Signed-off-by: Clément Léger <clement.leger@xxxxxxxxxxx>

Applied it to at91-soc with adjustments described in previous reply, thanks!

>> ---
>>  arch/arm/mach-at91/sama5.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
>> index de5dd28b392e..d1a9e940a785 100644
>> --- a/arch/arm/mach-at91/sama5.c
>> +++ b/arch/arm/mach-at91/sama5.c
>> @@ -9,13 +9,27 @@
>>  #include <linux/of.h>
>>  #include <linux/of_platform.h>
>>
>> +#include <asm/hardware/cache-l2x0.h>
>>  #include <asm/mach/arch.h>
>>  #include <asm/mach/map.h>
>> +#include <asm/outercache.h>
>>  #include <asm/system_misc.h>
>>
>>  #include "generic.h"
>>  #include "sam_secure.h"
>>
>> +static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
>> +{
>> +       /* OP-TEE configures the L2 cache and does not allow modifying it yet */
>> +}
>> +
>> +static void __init sama5_secure_cache_init(void)
>> +{
>> +       sam_secure_init();
> 
> With this, could the sam_secure_init() in sama5d2_init() (not listed in
> this diff) be removed?
> 
>> +       if (sam_linux_is_in_normal_world())
>> +               outer_cache.write_sec = sama5_l2c310_write_sec;
>> +}
>> +
>>  static void __init sama5_dt_device_init(void)
>>  {
>>         of_platform_default_populate(NULL, NULL, NULL);
>> @@ -30,6 +44,7 @@ static const char *const sama5_dt_board_compat[] __initconst = {
>>  DT_MACHINE_START(sama5_dt, "Atmel SAMA5")
>>         /* Maintainer: Atmel */
>>         .init_machine   = sama5_dt_device_init,
>> +       .init_early     = sama5_secure_cache_init,
> 
> This is for the generic "atmel,sama5" which can apply also to sama5d3 or
> sama5d4. I know this is harmless for functionality (except maybe when optee
> is in DT) but do we want it here?
> 
>>         .dt_compat      = sama5_dt_board_compat,
>>  MACHINE_END
>>
>> @@ -41,6 +56,7 @@ static const char *const sama5_alt_dt_board_compat[] __initconst = {
>>  DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
>>         /* Maintainer: Atmel */
>>         .init_machine   = sama5_dt_device_init,
>> +       .init_early     = sama5_secure_cache_init,
> 
> Same here except it applies to sama5d4 only.
> 
>>         .dt_compat      = sama5_alt_dt_board_compat,
>>         .l2c_aux_mask   = ~0UL,
>>  MACHINE_END
>> @@ -60,6 +76,7 @@ static const char *const sama5d2_compat[] __initconst = {
>>  DT_MACHINE_START(sama5d2, "Atmel SAMA5")
>>         /* Maintainer: Atmel */
>>         .init_machine   = sama5d2_init,
>> +       .init_early     = sama5_secure_cache_init,
>>         .dt_compat      = sama5d2_compat,
>>         .l2c_aux_mask   = ~0UL,
>>  MACHINE_END
>> --
>> 2.36.1
>>
>