This patch adds RX-51 specific SMC support and sets IBE bit in ACTLR during
board init code for ARM errata 430973 workaround.
This is second version of patch which simplifing API and moving all board
functions to one file board-rx51.c. Wrapper around smc #1 instruction is in
omap-smc.S file because some other board may use it too.
Pali Rohár (2):
ARM: OMAP: Add secure function omap_smc3() which calling instruction
smc #1
RX-51: ARM errata 430973 workaround
arch/arm/mach-omap2/board-rx51.c | 78 +++++++++++++++++++++++++++++++++++++
arch/arm/mach-omap2/omap-secure.h | 1 +
arch/arm/mach-omap2/omap-smc.S | 22 ++++++++++-
3 files changed, 100 insertions(+), 1 deletion(-)
--
1.7.10.4
Closed and signed Nokia X-Loader bootloader stored in RX-51 nand does not set
IBE bit in ACTLR and starting kernel in non-secure mode. So direct write to
ACTLR by our kernel does not working and the code for ARM errata 430973 in
commit 7ce236fcd6fd45b0441a2d49acb2ceb2de2e8a47 that sets IBE bit is a noop.
In order to have workaround for ARM errata 430973 from non-secure world on
RX-51 we needs Secure Monitor Call to set IBE BIT in ACTLR.
This patch adds RX-51 specific SMC support and sets IBE bit in ACTLR during
board init code for ARM errata 430973 workaround.
ARM errata 430973 workaround is needed for thumb-2 ISA compiled userspace
binaries. Without this workaround thumb-2 binaries crashing. So with this
patch it is possible to recompile and run applications/binaries with thumb-2
ISA on RX-51.
Signed-off-by: Ivaylo Dimitrov <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/mach-omap2/board-rx51.c | 78 ++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 03663c2..f234dee 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -2,6 +2,8 @@
* Board support file for Nokia N900 (aka RX-51).
*
* Copyright (C) 2007, 2008 Nokia
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -21,6 +23,7 @@
#include <linux/usb/musb.h>
#include <linux/platform_data/spi-omap2-mcspi.h>
+#include <asm/cacheflush.h>
#include <asm/mach-types.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -31,7 +34,14 @@
#include "mux.h"
#include "gpmc.h"
#include "pm.h"
+#include "soc.h"
#include "sdram-nokia.h"
+#include "omap-secure.h"
+
+/* Secure PPA (Primary Protected Application) APIs */
+#define RX51_PPA_HWRNG 29
+#define RX51_PPA_L2_INVAL 40
+#define RX51_PPA_WRITE_ACR 42
#define RX51_GPIO_SLEEP_IND 162
@@ -90,6 +100,66 @@ static struct omap_musb_board_data musb_board_data = {
.power = 0,
};
+/**
+ * rx51_secure_dispatcher: Routine to dispatch secure PPA API calls
+ * @idx: The PPA API index
+ * @process: Process ID
+ * @flag: The flag indicating criticality of operation
+ * @nargs: Number of valid arguments out of four.
+ * @arg1, arg2, arg3 args4: Parameters passed to secure API
+ *
+ * Return the non-zero error value on failure.
+ */
+static u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
+ u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+{
+ u32 ret;
+ u32 param[5];
+
+ param[0] = nargs+1; /* RX-51 needs number of arguments + 1 */
+ param[1] = arg1;
+ param[2] = arg2;
+ param[3] = arg3;
+ param[4] = arg4;
+
+ /*
+ * Secure API needs physical address
+ * pointer for the parameters
+ */
+ local_irq_disable();
+ local_fiq_disable();
+ flush_cache_all();
+ outer_clean_range(__pa(param), __pa(param + 5));
+ ret = omap_smc3(idx, process, flag, __pa(param));
+ flush_cache_all();
+ local_fiq_enable();
+ local_irq_enable();
+
+ return ret;
+}
+
+/**
+ * rx51_secure_update_aux_cr: Routine to modify the contents of Auxiliary Control Register
+ * @set_bits: bits to set in ACR
+ * @clr_bits: bits to clear in ACR
+ *
+ * Return the non-zero error value on failure.
+*/
+static u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits)
+{
+ u32 acr;
+
+ /* Read ACR */
+ asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
+ acr &= ~clear_bits;
+ acr |= set_bits;
+
+ return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
+ 0,
+ FLAG_START_CRITICAL,
+ 1, acr, 0, 0, 0);
+}
+
static void __init rx51_init(void)
{
struct omap_sdrc_params *sdrc_params;
@@ -105,6 +175,14 @@ static void __init rx51_init(void)
rx51_peripherals_init();
rx51_camera_init();
+ if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
+#ifdef CONFIG_ARM_ERRATA_430973
+ pr_info("RX-51: Enabling ARM errata 430973 workaround\n");
+ /* set IBE to 1 */
+ rx51_secure_update_aux_cr(BIT(6), 0);
+#endif
+ }
+
/* Ensure SDRC pins are mux'd for self-refresh */
omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
--
1.7.10.4
Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
but Nokia RX-51 board needs to call smc #1 for PPA access.
Signed-off-by: Ivaylo Dimitrov <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
---
arch/arm/mach-omap2/omap-secure.h | 1 +
arch/arm/mach-omap2/omap-smc.S | 22 +++++++++++++++++++++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index 0e72917..c4586f4 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -51,6 +51,7 @@
extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
u32 arg1, u32 arg2, u32 arg3, u32 arg4);
extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
+extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
extern phys_addr_t omap_secure_ram_mempool_base(void);
extern int omap_secure_ram_reserve_memblock(void);
diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
index f6441c1..5c02b8d 100644
--- a/arch/arm/mach-omap2/omap-smc.S
+++ b/arch/arm/mach-omap2/omap-smc.S
@@ -1,9 +1,11 @@
/*
- * OMAP44xx secure APIs file.
+ * OMAP34xx and OMAP44xx secure APIs file.
*
* Copyright (C) 2010 Texas Instruments, Inc.
* Written by Santosh Shilimkar <[email protected]>
*
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software,you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -54,6 +56,24 @@ ENTRY(omap_smc2)
ldmfd sp!, {r4-r12, pc}
ENDPROC(omap_smc2)
+/**
+ * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
+ * Low level common routine for secure HAL and PPA APIs via smc #1
+ * r0 - @service_id: Secure Service ID
+ * r1 - @process_id: Process ID
+ * r2 - @flag: Flag to indicate the criticality of operation
+ * r3 - @pargs: Physical address of parameter list
+ */
+ENTRY(omap_smc3)
+ stmfd sp!, {r4-r12, lr}
+ mov r12, r0 @ Copy the secure service ID
+ mov r6, #0xff @ Indicate new Task call
+ dsb
+ dmb
+ smc #1 @ call PPA service
+ ldmfd sp!, {r4-r12, pc}
+ENDPROC(omap_smc3)
+
ENTRY(omap_modify_auxcoreboot0)
stmfd sp!, {r1-r12, lr}
ldr r12, =0x104
--
1.7.10.4
On Wed, Jul 10, 2013 at 02:59:04PM +0200, Pali Roh?r wrote:
> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> but Nokia RX-51 board needs to call smc #1 for PPA access.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> Signed-off-by: Pali Roh?r <[email protected]>
> ---
> arch/arm/mach-omap2/omap-secure.h | 1 +
> arch/arm/mach-omap2/omap-smc.S | 22 +++++++++++++++++++++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> index 0e72917..c4586f4 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -51,6 +51,7 @@
> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> extern phys_addr_t omap_secure_ram_mempool_base(void);
> extern int omap_secure_ram_reserve_memblock(void);
>
> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> index f6441c1..5c02b8d 100644
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -1,9 +1,11 @@
> /*
> - * OMAP44xx secure APIs file.
> + * OMAP34xx and OMAP44xx secure APIs file.
> *
> * Copyright (C) 2010 Texas Instruments, Inc.
> * Written by Santosh Shilimkar <[email protected]>
> *
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Roh?r <[email protected]>
> *
> * This program is free software,you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -54,6 +56,24 @@ ENTRY(omap_smc2)
> ldmfd sp!, {r4-r12, pc}
> ENDPROC(omap_smc2)
>
> +/**
> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> + * Low level common routine for secure HAL and PPA APIs via smc #1
> + * r0 - @service_id: Secure Service ID
> + * r1 - @process_id: Process ID
> + * r2 - @flag: Flag to indicate the criticality of operation
> + * r3 - @pargs: Physical address of parameter list
> + */
> +ENTRY(omap_smc3)
> + stmfd sp!, {r4-r12, lr}
You don't need to save/restore r12. The ABI allows it to be clobbered
across function calls.
> + mov r12, r0 @ Copy the secure service ID
> + mov r6, #0xff @ Indicate new Task call
> + dsb
> + dmb
dsb synchronises a superset of what dmb synchronises, so the dmb here is
not useful.
In any case, any code calling this must flush the region addressed by
r3 beforehand anyway, which will include a dsb as part of its semantics
-- this is how you call it from rx51_secure_dispatcher().
So I think the dsb may not be needed here (?)
Cheers
---Dave
>-------- Оригинално писмо --------
>От: Dave Martin
>Относно: Re: [PATCH v2 1/2] ARM: OMAP: Add secure function omap_smc3() which
calling instruction smc #1
>До: Pali Rohár
>Изпратено на: Сряда, 2013, Юли 10 20:45:26 EEST
>
>
>On Wed, Jul 10, 2013 at 02:59:04PM +0200, Pali Rohár wrote:
>> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
>> but Nokia RX-51 board needs to call smc #1 for PPA access.
>>
>> Signed-off-by: Ivaylo Dimitrov
>> Signed-off-by: Pali Rohár
>> ---
>> arch/arm/mach-omap2/omap-secure.h | 1 +
>> arch/arm/mach-omap2/omap-smc.S | 22 +++++++++++++++++++++-
>> 2 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
>> index 0e72917..c4586f4 100644
>> --- a/arch/arm/mach-omap2/omap-secure.h
>> +++ b/arch/arm/mach-omap2/omap-secure.h
>> @@ -51,6 +51,7 @@
>> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
>> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
>> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
>> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
>> extern phys_addr_t omap_secure_ram_mempool_base(void);
>> extern int omap_secure_ram_reserve_memblock(void);
>>
>> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
>> index f6441c1..5c02b8d 100644
>> --- a/arch/arm/mach-omap2/omap-smc.S
>> +++ b/arch/arm/mach-omap2/omap-smc.S
>> @@ -1,9 +1,11 @@
>> /*
>> - * OMAP44xx secure APIs file.
>> + * OMAP34xx and OMAP44xx secure APIs file.
>> *
>> * Copyright (C) 2010 Texas Instruments, Inc.
>> * Written by Santosh Shilimkar
>> *
>> + * Copyright (C) 2012 Ivaylo Dimitrov
>> + * Copyright (C) 2013 Pali Rohár
>> *
>> * This program is free software,you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -54,6 +56,24 @@ ENTRY(omap_smc2)
>> ldmfd sp!, {r4-r12, pc}
>> ENDPROC(omap_smc2)
>>
>> +/**
>> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
>> + * Low level common routine for secure HAL and PPA APIs via smc #1
>> + * r0 - @service_id: Secure Service ID
>> + * r1 - @process_id: Process ID
>> + * r2 - @flag: Flag to indicate the criticality of operation
>> + * r3 - @pargs: Physical address of parameter list
>> + */
>> +ENTRY(omap_smc3)
>> + stmfd sp!, {r4-r12, lr}
>
>You don't need to save/restore r12. The ABI allows it to be clobbered
>across function calls.
>
>> + mov r12, r0 @ Copy the secure service ID
>> + mov r6, #0xff @ Indicate new Task call
>> + dsb
>> + dmb
>
>dsb synchronises a superset of what dmb synchronises, so the dmb here is
>not useful.
>
>In any case, any code calling this must flush the region addressed by
>r3 beforehand anyway, which will include a dsb as part of its semantics
>-- this is how you call it from rx51_secure_dispatcher().
>
>So I think the dsb may not be needed here (?)
>
>Cheers
>---Dave
>
>
Could be, but I wonder why almost all the kernel code(I am aware of) that uses SMC and is written by TI, is storing r12 and is using both DSB and DMB. See arch/arm/mach-omap2/sleep34xx.S or arch/arm/mach-omap2/omap-smc.S for examples. I'd rather play it safe and leave it that way, unless someone confirms the other code using SMC has extra DSB/DMB instructions too. I wouldn't risk passing invalid/stale data to the Secure Monitor to just save 8 bytes and barriers in a performance non-critical code which will be called only a couple of times during the boot-up process. r12 save/restore is a legacy from omap_smc2 in omap-smc.S, so I guess it can go away without much of a trouble.
On Thursday 11 July 2013 03:43 PM, Ивайло Димитров wrote:
>
>
>
>
>
> >-------- Оригинално писмо --------
> >От: Dave Martin
> >Относно: Re: [PATCH v2 1/2] ARM: OMAP: Add secure function omap_smc3() which
> calling instruction smc #1
> >До: Pali Rohár
> >Изпратено на: Сряда, 2013, Юли 10 20:45:26 EEST
> >
> >
> >On Wed, Jul 10, 2013 at 02:59:04PM +0200, Pali Rohár wrote:
> >> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> >> but Nokia RX-51 board needs to call smc #1 for PPA access.
> >>
> >> Signed-off-by: Ivaylo Dimitrov
> >> Signed-off-by: Pali Rohár
> >> ---
> >> arch/arm/mach-omap2/omap-secure.h | 1 +
> >> arch/arm/mach-omap2/omap-smc.S | 22 +++++++++++++++++++++-
> >> 2 files changed, 22 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> >> index 0e72917..c4586f4 100644
> >> --- a/arch/arm/mach-omap2/omap-secure.h
> >> +++ b/arch/arm/mach-omap2/omap-secure.h
> >> @@ -51,6 +51,7 @@
> >> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> >> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> >> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> >> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> >> extern phys_addr_t omap_secure_ram_mempool_base(void);
> >> extern int omap_secure_ram_reserve_memblock(void);
> >>
> >> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> >> index f6441c1..5c02b8d 100644
> >> --- a/arch/arm/mach-omap2/omap-smc.S
> >> +++ b/arch/arm/mach-omap2/omap-smc.S
> >> @@ -1,9 +1,11 @@
> >> /*
> >> - * OMAP44xx secure APIs file.
> >> + * OMAP34xx and OMAP44xx secure APIs file.
> >> *
> >> * Copyright (C) 2010 Texas Instruments, Inc.
> >> * Written by Santosh Shilimkar
> >> *
> >> + * Copyright (C) 2012 Ivaylo Dimitrov
> >> + * Copyright (C) 2013 Pali Rohár
> >> *
> >> * This program is free software,you can redistribute it and/or modify
> >> * it under the terms of the GNU General Public License version 2 as
> >> @@ -54,6 +56,24 @@ ENTRY(omap_smc2)
> >> ldmfd sp!, {r4-r12, pc}
> >> ENDPROC(omap_smc2)
> >>
> >> +/**
> >> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> >> + * Low level common routine for secure HAL and PPA APIs via smc #1
> >> + * r0 - @service_id: Secure Service ID
> >> + * r1 - @process_id: Process ID
> >> + * r2 - @flag: Flag to indicate the criticality of operation
> >> + * r3 - @pargs: Physical address of parameter list
> >> + */
> >> +ENTRY(omap_smc3)
> >> + stmfd sp!, {r4-r12, lr}
> >
> >You don't need to save/restore r12. The ABI allows it to be clobbered
> >across function calls.
> >
> >> + mov r12, r0 @ Copy the secure service ID
> >> + mov r6, #0xff @ Indicate new Task call
> >> + dsb
> >> + dmb
> >
> >dsb synchronises a superset of what dmb synchronises, so the dmb here is
> >not useful.
> >
> >In any case, any code calling this must flush the region addressed by
> >r3 beforehand anyway, which will include a dsb as part of its semantics
> >-- this is how you call it from rx51_secure_dispatcher().
> >
> >So I think the dsb may not be needed here (?)
> >
> >Cheers
> >---Dave
> >
> >
>
> Could be, but I wonder why almost all the kernel code(I am aware of) that uses SMC and is written by TI, is storing r12 and is using both DSB and DMB. See arch/arm/mach-omap2/sleep34xx.S or arch/arm/mach-omap2/omap-smc.S for examples. I'd rather play it safe and leave it that way, unless someone confirms the other code using SMC has extra DSB/DMB instructions too. I wouldn't risk passing invalid/stale data to the Secure Monitor to just save 8 bytes and barriers in a performance non-critical code which will be called only a couple of times during the boot-up process. r12 save/restore is a legacy from omap_smc2 in omap-smc.S, so I guess it can go away without much of a trouble.
>
Dave pointed out about the dsb and r12 to me in another thread. R12 can be easily removed
but the DSB's were needed on OMAP for power sequencing. Without those, we have seen
many issues. So you can leave the dsb's to be consistent with rest of the code.
Regards
Santosh
P.S: Please sensibly wrap your message while replying. You reply apears like
one single line and I needed to keep scrolling to read it.
On Thu, Jul 11, 2013 at 03:54:34PM -0400, Santosh Shilimkar wrote:
> On Thursday 11 July 2013 03:43 PM, Ивайло Димитров wrote:
> >
> >
> >
> >
> >
> > >-------- Оригинално писмо --------
> > >От: Dave Martin
> > >Относно: Re: [PATCH v2 1/2] ARM: OMAP: Add secure function omap_smc3() which
> > calling instruction smc #1
> > >До: Pali Rohár
> > >Изпратено на: Сряда, 2013, Юли 10 20:45:26 EEST
> > >
> > >
> > >On Wed, Jul 10, 2013 at 02:59:04PM +0200, Pali Rohár wrote:
> > >> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> > >> but Nokia RX-51 board needs to call smc #1 for PPA access.
> > >>
> > >> Signed-off-by: Ivaylo Dimitrov
> > >> Signed-off-by: Pali Rohár
> > >> ---
> > >> arch/arm/mach-omap2/omap-secure.h | 1 +
> > >> arch/arm/mach-omap2/omap-smc.S | 22 +++++++++++++++++++++-
> > >> 2 files changed, 22 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> > >> index 0e72917..c4586f4 100644
> > >> --- a/arch/arm/mach-omap2/omap-secure.h
> > >> +++ b/arch/arm/mach-omap2/omap-secure.h
> > >> @@ -51,6 +51,7 @@
> > >> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> > >> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> > >> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> > >> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> > >> extern phys_addr_t omap_secure_ram_mempool_base(void);
> > >> extern int omap_secure_ram_reserve_memblock(void);
> > >>
> > >> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> > >> index f6441c1..5c02b8d 100644
> > >> --- a/arch/arm/mach-omap2/omap-smc.S
> > >> +++ b/arch/arm/mach-omap2/omap-smc.S
> > >> @@ -1,9 +1,11 @@
> > >> /*
> > >> - * OMAP44xx secure APIs file.
> > >> + * OMAP34xx and OMAP44xx secure APIs file.
> > >> *
> > >> * Copyright (C) 2010 Texas Instruments, Inc.
> > >> * Written by Santosh Shilimkar
> > >> *
> > >> + * Copyright (C) 2012 Ivaylo Dimitrov
> > >> + * Copyright (C) 2013 Pali Rohár
> > >> *
> > >> * This program is free software,you can redistribute it and/or modify
> > >> * it under the terms of the GNU General Public License version 2 as
> > >> @@ -54,6 +56,24 @@ ENTRY(omap_smc2)
> > >> ldmfd sp!, {r4-r12, pc}
> > >> ENDPROC(omap_smc2)
> > >>
> > >> +/**
> > >> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> > >> + * Low level common routine for secure HAL and PPA APIs via smc #1
> > >> + * r0 - @service_id: Secure Service ID
> > >> + * r1 - @process_id: Process ID
> > >> + * r2 - @flag: Flag to indicate the criticality of operation
> > >> + * r3 - @pargs: Physical address of parameter list
> > >> + */
> > >> +ENTRY(omap_smc3)
> > >> + stmfd sp!, {r4-r12, lr}
> > >
> > >You don't need to save/restore r12. The ABI allows it to be clobbered
> > >across function calls.
> > >
> > >> + mov r12, r0 @ Copy the secure service ID
> > >> + mov r6, #0xff @ Indicate new Task call
> > >> + dsb
> > >> + dmb
> > >
> > >dsb synchronises a superset of what dmb synchronises, so the dmb here is
> > >not useful.
> > >
> > >In any case, any code calling this must flush the region addressed by
> > >r3 beforehand anyway, which will include a dsb as part of its semantics
> > >-- this is how you call it from rx51_secure_dispatcher().
> > >
> > >So I think the dsb may not be needed here (?)
> > >
> > >Cheers
> > >---Dave
> > >
> > >
> >
> > Could be, but I wonder why almost all the kernel code(I am aware of) that uses SMC and is written by TI, is storing r12 and is using both DSB and DMB. See arch/arm/mach-omap2/sleep34xx.S or arch/arm/mach-omap2/omap-smc.S for examples. I'd rather play it safe and leave it that way, unless someone confirms the other code using SMC has extra DSB/DMB instructions too. I wouldn't risk passing invalid/stale data to the Secure Monitor to just save 8 bytes and barriers in a performance non-critical code which will be called only a couple of times during the boot-up process. r12 save/restore is a legacy from omap_smc2 in omap-smc.S, so I guess it can go away without much of a trouble.
> >
> Dave pointed out about the dsb and r12 to me in another thread. R12 can be easily removed
> but the DSB's were needed on OMAP for power sequencing. Without those, we have seen
> many issues. So you can leave the dsb's to be consistent with rest of the code.
Consistency is a perfectly good reason, especially in code like this
where a certain code sequence has been proven, but it's preferable to
include brief comments to explain nonetheless.
Difficulty in explaining precisely why something is needed should be a
warning flag that a comment is needed.
Cheers
---Dave
Here is new version (v3) of omap secure part patch:
Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
but Nokia RX-51 board needs to call smc #1 for PPA access.
Signed-off-by: Ivaylo Dimitrov <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
---
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index 0e72917..c4586f4 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -51,6 +51,7 @@
extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
u32 arg1, u32 arg2, u32 arg3, u32 arg4);
extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
+extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
extern phys_addr_t omap_secure_ram_mempool_base(void);
extern int omap_secure_ram_reserve_memblock(void);
diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
index f6441c1..7bbc043 100644
--- a/arch/arm/mach-omap2/omap-smc.S
+++ b/arch/arm/mach-omap2/omap-smc.S
@@ -1,9 +1,11 @@
/*
- * OMAP44xx secure APIs file.
+ * OMAP34xx and OMAP44xx secure APIs file.
*
* Copyright (C) 2010 Texas Instruments, Inc.
* Written by Santosh Shilimkar <[email protected]>
*
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software,you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -54,6 +56,23 @@ ENTRY(omap_smc2)
ldmfd sp!, {r4-r12, pc}
ENDPROC(omap_smc2)
+/**
+ * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
+ * Low level common routine for secure HAL and PPA APIs via smc #1
+ * r0 - @service_id: Secure Service ID
+ * r1 - @process_id: Process ID
+ * r2 - @flag: Flag to indicate the criticality of operation
+ * r3 - @pargs: Physical address of parameter list
+ */
+ENTRY(omap_smc3)
+ stmfd sp!, {r4-r11, lr}
+ mov r12, r0 @ Copy the secure service ID
+ mov r6, #0xff @ Indicate new Task call
+ dsb @ Memory Barrier
+ smc #1 @ Call PPA service
+ ldmfd sp!, {r4-r11, pc}
+ENDPROC(omap_smc3)
+
ENTRY(omap_modify_auxcoreboot0)
stmfd sp!, {r1-r12, lr}
ldr r12, =0x104
--
Pali Rohár
[email protected]
On Sun, Aug 04, 2013 at 10:45:00AM +0200, Pali Roh?r wrote:
> Here is new version (v3) of omap secure part patch:
>
> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> but Nokia RX-51 board needs to call smc #1 for PPA access.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> Signed-off-by: Pali Roh?r <[email protected]>
> ---
> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> index 0e72917..c4586f4 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -51,6 +51,7 @@
> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> extern phys_addr_t omap_secure_ram_mempool_base(void);
> extern int omap_secure_ram_reserve_memblock(void);
>
> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> index f6441c1..7bbc043 100644
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -1,9 +1,11 @@
> /*
> - * OMAP44xx secure APIs file.
> + * OMAP34xx and OMAP44xx secure APIs file.
> *
> * Copyright (C) 2010 Texas Instruments, Inc.
> * Written by Santosh Shilimkar <[email protected]>
> *
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Roh?r <[email protected]>
> *
> * This program is free software,you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> ldmfd sp!, {r4-r12, pc}
> ENDPROC(omap_smc2)
>
> +/**
> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> + * Low level common routine for secure HAL and PPA APIs via smc #1
> + * r0 - @service_id: Secure Service ID
> + * r1 - @process_id: Process ID
> + * r2 - @flag: Flag to indicate the criticality of operation
> + * r3 - @pargs: Physical address of parameter list
> + */
> +ENTRY(omap_smc3)
> + stmfd sp!, {r4-r11, lr}
> + mov r12, r0 @ Copy the secure service ID
> + mov r6, #0xff @ Indicate new Task call
> + dsb @ Memory Barrier
Can you explain _why_ the barrier is there? The reader doesn't need to
be told that a barrier instruction is a barrier instruction.
Cheers
---Dave
>-------- Оригинално писмо --------
>От: Dave Martin
>Относно: Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which
calling instruction smc #1
>До: Pali Rohár
>Изпратено на: Понеделник, 2013, Август 5 16:29:44 EEST
>
>
>On Sun, Aug 04, 2013 at 10:45:00AM +0200, Pali Rohár wrote:
>> Here is new version (v3) of omap secure part patch:
>>
>> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
>> but Nokia RX-51 board needs to call smc #1 for PPA access.
>>
>> Signed-off-by: Ivaylo Dimitrov
>> Signed-off-by: Pali Rohár
>> ---
>> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
>> index 0e72917..c4586f4 100644
>> --- a/arch/arm/mach-omap2/omap-secure.h
>> +++ b/arch/arm/mach-omap2/omap-secure.h
>> @@ -51,6 +51,7 @@
>> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
>> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
>> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
>> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
>> extern phys_addr_t omap_secure_ram_mempool_base(void);
>> extern int omap_secure_ram_reserve_memblock(void);
>>
>> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
>> index f6441c1..7bbc043 100644
>> --- a/arch/arm/mach-omap2/omap-smc.S
>> +++ b/arch/arm/mach-omap2/omap-smc.S
>> @@ -1,9 +1,11 @@
>> /*
>> - * OMAP44xx secure APIs file.
>> + * OMAP34xx and OMAP44xx secure APIs file.
>> *
>> * Copyright (C) 2010 Texas Instruments, Inc.
>> * Written by Santosh Shilimkar
>> *
>> + * Copyright (C) 2012 Ivaylo Dimitrov
>> + * Copyright (C) 2013 Pali Rohár
>> *
>> * This program is free software,you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
>> ldmfd sp!, {r4-r12, pc}
>> ENDPROC(omap_smc2)
>>
>> +/**
>> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
>> + * Low level common routine for secure HAL and PPA APIs via smc #1
>> + * r0 - @service_id: Secure Service ID
>> + * r1 - @process_id: Process ID
>> + * r2 - @flag: Flag to indicate the criticality of operation
>> + * r3 - @pargs: Physical address of parameter list
>> + */
>> +ENTRY(omap_smc3)
>> + stmfd sp!, {r4-r11, lr}
>> + mov r12, r0 @ Copy the secure service ID
>> + mov r6, #0xff @ Indicate new Task call
>> + dsb @ Memory Barrier
>
>Can you explain _why_ the barrier is there? The reader doesn't need to
>be told that a barrier instruction is a barrier instruction.
>
>Cheers
>---Dave
>
Hi Dave,
Would quoting Santosh's explanation "DSBs were needed on OMAP for power sequencing." do the job?
Something like "@ Needed on OMAP for power sequencing" instead of "@ Memory Barrier".
I want to be sure I correctly understand your requirement.
Regards,
Ivo
On Sunday 11 August 2013 20:36:40 Ивайло Димитров wrote:
> >-------- Оригинално писмо --------
> >
> >От: Dave Martin
> >Относно: Re: [PATCH v3 1/2] ARM: OMAP: Add secure function
> >omap_smc3() which
>
> calling instruction smc #1
>
> >До: Pali Rohár
> >Изпратено на: Понеделник, 2013, Август 5 16:29:44 EEST
> >
> >On Sun, Aug 04, 2013 at 10:45:00AM +0200, Pali Rohár wrote:
> >> Here is new version (v3) of omap secure part patch:
> >>
> >> Other secure functions omap_smc1() and omap_smc2() calling
> >> instruction smc #0 but Nokia RX-51 board needs to call
> >> smc #1 for PPA access.
> >>
> >> Signed-off-by: Ivaylo Dimitrov
> >> Signed-off-by: Pali Rohár
> >> ---
> >> diff --git a/arch/arm/mach-omap2/omap-secure.h
> >> b/arch/arm/mach-omap2/omap-secure.h index
> >> 0e72917..c4586f4 100644
> >> --- a/arch/arm/mach-omap2/omap-secure.h
> >> +++ b/arch/arm/mach-omap2/omap-secure.h
> >> @@ -51,6 +51,7 @@
> >>
> >> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32
> >> nargs,
> >>
> >> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> >>
> >> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> >>
> >> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32
> >> pargs);
> >>
> >> extern phys_addr_t omap_secure_ram_mempool_base(void);
> >> extern int omap_secure_ram_reserve_memblock(void);
> >>
> >> diff --git a/arch/arm/mach-omap2/omap-smc.S
> >> b/arch/arm/mach-omap2/omap-smc.S index f6441c1..7bbc043
> >> 100644
> >> --- a/arch/arm/mach-omap2/omap-smc.S
> >> +++ b/arch/arm/mach-omap2/omap-smc.S
> >> @@ -1,9 +1,11 @@
> >>
> >> /*
> >>
> >> - * OMAP44xx secure APIs file.
> >> + * OMAP34xx and OMAP44xx secure APIs file.
> >>
> >> *
> >> * Copyright (C) 2010 Texas Instruments, Inc.
> >> * Written by Santosh Shilimkar
> >> *
> >>
> >> + * Copyright (C) 2012 Ivaylo Dimitrov
> >> + * Copyright (C) 2013 Pali Rohár
> >>
> >> *
> >> * This program is free software,you can redistribute it
> >> and/or modify * it under the terms of the GNU General
> >> Public License version 2 as
> >>
> >> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> >>
> >> ldmfd sp!, {r4-r12, pc}
> >>
> >> ENDPROC(omap_smc2)
> >>
> >> +/**
> >> + * u32 omap_smc3(u32 service_id, u32 process_id, u32
> >> flag, u32 pargs) + * Low level common routine for secure
> >> HAL and PPA APIs via smc #1 + * r0 - @service_id: Secure
> >> Service ID
> >> + * r1 - @process_id: Process ID
> >> + * r2 - @flag: Flag to indicate the criticality of
> >> operation + * r3 - @pargs: Physical address of parameter
> >> list + */
> >> +ENTRY(omap_smc3)
> >> + stmfd sp!, {r4-r11, lr}
> >> + mov r12, r0 @ Copy the secure service ID
> >> + mov r6, #0xff @ Indicate new Task call
> >> + dsb @ Memory Barrier
> >
> >Can you explain _why_ the barrier is there? The reader
> >doesn't need to be told that a barrier instruction is a
> >barrier instruction.
> >
> >Cheers
> >---Dave
>
> Hi Dave,
>
> Would quoting Santosh's explanation "DSBs were needed on OMAP
> for power sequencing." do the job? Something like "@ Needed
> on OMAP for power sequencing" instead of "@ Memory Barrier".
>
> I want to be sure I correctly understand your requirement.
>
> Regards,
> Ivo
Hello,
I'd like to know what happened with this patch? What is needed
for including it into mainline? Note that without with this patch
series Thumb-2 user space binaries crashing.
--
Pali Rohár
[email protected]
On Wed, Sep 04, 2013 at 10:10:29AM +0200, Pali Rohár wrote:
> On Sunday 11 August 2013 20:36:40 Ивайло Димитров wrote:
> > >-------- Оригинално писмо --------
> > >
> > >От: Dave Martin
> > >Относно: Re: [PATCH v3 1/2] ARM: OMAP: Add secure function
> > >omap_smc3() which
> >
> > calling instruction smc #1
> >
> > >До: Pali Rohár
> > >Изпратено на: Понеделник, 2013, Август 5 16:29:44 EEST
> > >
> > >On Sun, Aug 04, 2013 at 10:45:00AM +0200, Pali Rohár wrote:
> > >> Here is new version (v3) of omap secure part patch:
> > >>
> > >> Other secure functions omap_smc1() and omap_smc2() calling
> > >> instruction smc #0 but Nokia RX-51 board needs to call
> > >> smc #1 for PPA access.
> > >>
> > >> Signed-off-by: Ivaylo Dimitrov
> > >> Signed-off-by: Pali Rohár
> > >> ---
> > >> diff --git a/arch/arm/mach-omap2/omap-secure.h
> > >> b/arch/arm/mach-omap2/omap-secure.h index
> > >> 0e72917..c4586f4 100644
> > >> --- a/arch/arm/mach-omap2/omap-secure.h
> > >> +++ b/arch/arm/mach-omap2/omap-secure.h
> > >> @@ -51,6 +51,7 @@
> > >>
> > >> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32
> > >> nargs,
> > >>
> > >> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> > >>
> > >> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> > >>
> > >> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32
> > >> pargs);
> > >>
> > >> extern phys_addr_t omap_secure_ram_mempool_base(void);
> > >> extern int omap_secure_ram_reserve_memblock(void);
> > >>
> > >> diff --git a/arch/arm/mach-omap2/omap-smc.S
> > >> b/arch/arm/mach-omap2/omap-smc.S index f6441c1..7bbc043
> > >> 100644
> > >> --- a/arch/arm/mach-omap2/omap-smc.S
> > >> +++ b/arch/arm/mach-omap2/omap-smc.S
> > >> @@ -1,9 +1,11 @@
> > >>
> > >> /*
> > >>
> > >> - * OMAP44xx secure APIs file.
> > >> + * OMAP34xx and OMAP44xx secure APIs file.
> > >>
> > >> *
> > >> * Copyright (C) 2010 Texas Instruments, Inc.
> > >> * Written by Santosh Shilimkar
> > >> *
> > >>
> > >> + * Copyright (C) 2012 Ivaylo Dimitrov
> > >> + * Copyright (C) 2013 Pali Rohár
> > >>
> > >> *
> > >> * This program is free software,you can redistribute it
> > >> and/or modify * it under the terms of the GNU General
> > >> Public License version 2 as
> > >>
> > >> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> > >>
> > >> ldmfd sp!, {r4-r12, pc}
> > >>
> > >> ENDPROC(omap_smc2)
> > >>
> > >> +/**
> > >> + * u32 omap_smc3(u32 service_id, u32 process_id, u32
> > >> flag, u32 pargs) + * Low level common routine for secure
> > >> HAL and PPA APIs via smc #1 + * r0 - @service_id: Secure
> > >> Service ID
> > >> + * r1 - @process_id: Process ID
> > >> + * r2 - @flag: Flag to indicate the criticality of
> > >> operation + * r3 - @pargs: Physical address of parameter
> > >> list + */
> > >> +ENTRY(omap_smc3)
> > >> + stmfd sp!, {r4-r11, lr}
> > >> + mov r12, r0 @ Copy the secure service ID
> > >> + mov r6, #0xff @ Indicate new Task call
> > >> + dsb @ Memory Barrier
> > >
> > >Can you explain _why_ the barrier is there? The reader
> > >doesn't need to be told that a barrier instruction is a
> > >barrier instruction.
> > >
> > >Cheers
> > >---Dave
> >
> > Hi Dave,
> >
> > Would quoting Santosh's explanation "DSBs were needed on OMAP
> > for power sequencing." do the job? Something like "@ Needed
> > on OMAP for power sequencing" instead of "@ Memory Barrier".
> >
> > I want to be sure I correctly understand your requirement.
> >
> > Regards,
> > Ivo
>
> Hello,
>
> I'd like to know what happened with this patch? What is needed
> for including it into mainline? Note that without with this patch
> series Thumb-2 user space binaries crashing.
Apologies, it looks like I missed the previous reply somehow.
"Needed for power sequencing" is not a good explanation, because
DSB has no direct role in "power sequencing".
I'm guessing that the reason for its being needed is either not well
understood (otherwise someone would have offered a real explanation by
now), or it is a secret (errata, whatever) or related to something so
obscure it's likely to be of any interest or use to anyone anyway.
In the former case, just document the uncertainty.
In the latter case, at least document that the reasons are specific to
that platform and probably not applicable to other situations: this
warns someone maintaining or copying the code later that it may not make
sense for them, and they should stop and think. It also avoids people
reading the code for educational purposes and learning incorrect lessons
from it.
I've seen a mail thread where precisely this bad education happened:
someone is trying to figure out whether they need a DSB before every SVC
after reading the mach-omap2 SMC code.
The same goes for the disabling of interrupts around the call in
rx51_secure_dispatcher(). Even if the Secure World might write into the
params struct during the SMC, making the params ____cacheline_aligned
should be sufficient to avoid the possibility of incoherent S/NS aliases
of the same cacheline clobbering each other. SMC disables all
interrupts on entry to the Secure World anyway, so I can't see what else
the interrupt disabling achieves unless there are platform-specific
errata reasons.
Cheers
---Dave
Here is new version (v4) of omap secure part patch:
Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
but Nokia RX-51 board needs to call smc #1 for PPA access.
Signed-off-by: Ivaylo Dimitrov <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
---
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index 0e72917..c4586f4 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -51,6 +51,7 @@
extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
u32 arg1, u32 arg2, u32 arg3, u32 arg4);
extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
+extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
extern phys_addr_t omap_secure_ram_mempool_base(void);
extern int omap_secure_ram_reserve_memblock(void);
diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
index f6441c1..fd90125 100644
--- a/arch/arm/mach-omap2/omap-smc.S
+++ b/arch/arm/mach-omap2/omap-smc.S
@@ -1,9 +1,11 @@
/*
- * OMAP44xx secure APIs file.
+ * OMAP34xx and OMAP44xx secure APIs file.
*
* Copyright (C) 2010 Texas Instruments, Inc.
* Written by Santosh Shilimkar <[email protected]>
*
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software,you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -54,6 +56,23 @@ ENTRY(omap_smc2)
ldmfd sp!, {r4-r12, pc}
ENDPROC(omap_smc2)
+/**
+ * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
+ * Low level common routine for secure HAL and PPA APIs via smc #1
+ * r0 - @service_id: Secure Service ID
+ * r1 - @process_id: Process ID
+ * r2 - @flag: Flag to indicate the criticality of operation
+ * r3 - @pargs: Physical address of parameter list
+ */
+ENTRY(omap_smc3)
+ stmfd sp!, {r4-r11, lr}
+ mov r12, r0 @ Copy the secure service ID
+ mov r6, #0xff @ Indicate new Task call
+ dsb @ Memory Barrier (not sure if needed, copied from omap_smc2)
+ smc #1 @ Call PPA service
+ ldmfd sp!, {r4-r11, pc}
+ENDPROC(omap_smc3)
+
ENTRY(omap_modify_auxcoreboot0)
stmfd sp!, {r1-r12, lr}
ldr r12, =0x104
--
Pali Rohár
[email protected]
On Sunday 08 September 2013 09:43:29 Pali Rohár wrote:
> Here is new version (v4) of omap secure part patch:
>
> Other secure functions omap_smc1() and omap_smc2() calling
> instruction smc #0 but Nokia RX-51 board needs to call smc #1
> for PPA access.
>
> Signed-off-by: Ivaylo Dimitrov <[email protected]>
> Signed-off-by: Pali Rohár <[email protected]>
> ---
> diff --git a/arch/arm/mach-omap2/omap-secure.h
> b/arch/arm/mach-omap2/omap-secure.h index 0e72917..c4586f4
> 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -51,6 +51,7 @@
> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32
> nargs, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32
> pargs); extern phys_addr_t
> omap_secure_ram_mempool_base(void); extern int
> omap_secure_ram_reserve_memblock(void);
>
> diff --git a/arch/arm/mach-omap2/omap-smc.S
> b/arch/arm/mach-omap2/omap-smc.S index f6441c1..fd90125
> 100644
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -1,9 +1,11 @@
> /*
> - * OMAP44xx secure APIs file.
> + * OMAP34xx and OMAP44xx secure APIs file.
> *
> * Copyright (C) 2010 Texas Instruments, Inc.
> * Written by Santosh Shilimkar <[email protected]>
> *
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Rohár <[email protected]>
> *
> * This program is free software,you can redistribute it
> and/or modify * it under the terms of the GNU General Public
> License version 2 as @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> ldmfd sp!, {r4-r12, pc}
> ENDPROC(omap_smc2)
>
> +/**
> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag,
> u32 pargs) + * Low level common routine for secure HAL and
> PPA APIs via smc #1 + * r0 - @service_id: Secure Service ID
> + * r1 - @process_id: Process ID
> + * r2 - @flag: Flag to indicate the criticality of operation
> + * r3 - @pargs: Physical address of parameter list
> + */
> +ENTRY(omap_smc3)
> + stmfd sp!, {r4-r11, lr}
> + mov r12, r0 @ Copy the secure service ID
> + mov r6, #0xff @ Indicate new Task call
> + dsb @ Memory Barrier (not sure if needed, copied from
> omap_smc2) + smc #1 @ Call PPA service
> + ldmfd sp!, {r4-r11, pc}
> +ENDPROC(omap_smc3)
> +
> ENTRY(omap_modify_auxcoreboot0)
> stmfd sp!, {r1-r12, lr}
> ldr r12, =0x104
Dave, it is ok now?
--
Pali Rohár
[email protected]
On Sat, Sep 14, 2013 at 10:37:12AM +0100, Pali Roh?r wrote:
> On Sunday 08 September 2013 09:43:29 Pali Roh?r wrote:
> > Here is new version (v4) of omap secure part patch:
> >
> > Other secure functions omap_smc1() and omap_smc2() calling
> > instruction smc #0 but Nokia RX-51 board needs to call smc #1
> > for PPA access.
> >
> > Signed-off-by: Ivaylo Dimitrov <[email protected]>
> > Signed-off-by: Pali Roh?r <[email protected]>
> > ---
> > diff --git a/arch/arm/mach-omap2/omap-secure.h
> > b/arch/arm/mach-omap2/omap-secure.h index 0e72917..c4586f4
> > 100644
> > --- a/arch/arm/mach-omap2/omap-secure.h
> > +++ b/arch/arm/mach-omap2/omap-secure.h
> > @@ -51,6 +51,7 @@
> > extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32
> > nargs, u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> > extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> > +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32
> > pargs); extern phys_addr_t
> > omap_secure_ram_mempool_base(void); extern int
> > omap_secure_ram_reserve_memblock(void);
> >
> > diff --git a/arch/arm/mach-omap2/omap-smc.S
> > b/arch/arm/mach-omap2/omap-smc.S index f6441c1..fd90125
> > 100644
> > --- a/arch/arm/mach-omap2/omap-smc.S
> > +++ b/arch/arm/mach-omap2/omap-smc.S
> > @@ -1,9 +1,11 @@
> > /*
> > - * OMAP44xx secure APIs file.
> > + * OMAP34xx and OMAP44xx secure APIs file.
> > *
> > * Copyright (C) 2010 Texas Instruments, Inc.
> > * Written by Santosh Shilimkar <[email protected]>
> > *
> > + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> > + * Copyright (C) 2013 Pali Roh?r <[email protected]>
> > *
> > * This program is free software,you can redistribute it
> > and/or modify * it under the terms of the GNU General Public
> > License version 2 as @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> > ldmfd sp!, {r4-r12, pc}
> > ENDPROC(omap_smc2)
> >
> > +/**
> > + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag,
> > u32 pargs) + * Low level common routine for secure HAL and
> > PPA APIs via smc #1 + * r0 - @service_id: Secure Service ID
> > + * r1 - @process_id: Process ID
> > + * r2 - @flag: Flag to indicate the criticality of operation
> > + * r3 - @pargs: Physical address of parameter list
> > + */
> > +ENTRY(omap_smc3)
> > + stmfd sp!, {r4-r11, lr}
> > + mov r12, r0 @ Copy the secure service ID
> > + mov r6, #0xff @ Indicate new Task call
> > + dsb @ Memory Barrier (not sure if needed, copied from
> > omap_smc2) + smc #1 @ Call PPA service
> > + ldmfd sp!, {r4-r11, pc}
> > +ENDPROC(omap_smc3)
> > +
> > ENTRY(omap_modify_auxcoreboot0)
> > stmfd sp!, {r1-r12, lr}
> > ldr r12, =0x104
>
> Dave, it is ok now?
Yes, that's sufficient to warn people to stop and think (at least, if
someone copy-pastes it, they will likely highlight the possible error by
copy-pasting the comment too). Thanks.
Acked-by: Dave Martin <[email protected]>
* Dave Martin <[email protected]> [130916 10:18]:
> On Sat, Sep 14, 2013 at 10:37:12AM +0100, Pali Rohár wrote:
> > On Sunday 08 September 2013 09:43:29 Pali Rohár wrote:
> > > + */
> > > +ENTRY(omap_smc3)
> > > + stmfd sp!, {r4-r11, lr}
> > > + mov r12, r0 @ Copy the secure service ID
> > > + mov r6, #0xff @ Indicate new Task call
> > > + dsb @ Memory Barrier (not sure if needed, copied from
> > > omap_smc2) + smc #1 @ Call PPA service
> > > + ldmfd sp!, {r4-r11, pc}
> > > +ENDPROC(omap_smc3)
> > > +
> > > ENTRY(omap_modify_auxcoreboot0)
> > > stmfd sp!, {r1-r12, lr}
> > > ldr r12, =0x104
> >
> > Dave, it is ok now?
>
> Yes, that's sufficient to warn people to stop and think (at least, if
> someone copy-pastes it, they will likely highlight the possible error by
> copy-pasting the comment too). Thanks.
>
> Acked-by: Dave Martin <[email protected]>
Have you guys checked how this works with the recently posted
"[PATCH v6 0/5] ARM: support for Trusted Foundations secure monitor"
series?
Regards,
Tony
On Tuesday 17 September 2013 17:43:31 Tony Lindgren wrote:
> * Dave Martin <[email protected]> [130916 10:18]:
> > On Sat, Sep 14, 2013 at 10:37:12AM +0100, Pali Rohár wrote:
> > > On Sunday 08 September 2013 09:43:29 Pali Rohár wrote:
> > > > + */
> > > > +ENTRY(omap_smc3)
> > > > + stmfd sp!, {r4-r11, lr}
> > > > + mov r12, r0 @ Copy the secure service ID
> > > > + mov r6, #0xff @ Indicate new Task call
> > > > + dsb @ Memory Barrier (not sure if needed, copied
> > > > from omap_smc2) + smc #1 @ Call PPA service
> > > > + ldmfd sp!, {r4-r11, pc}
> > > > +ENDPROC(omap_smc3)
> > > > +
> > > >
> > > > ENTRY(omap_modify_auxcoreboot0)
> > > >
> > > > stmfd sp!, {r1-r12, lr}
> > > > ldr r12, =0x104
> > >
> > > Dave, it is ok now?
> >
> > Yes, that's sufficient to warn people to stop and think (at
> > least, if someone copy-pastes it, they will likely
> > highlight the possible error by copy-pasting the comment
> > too). Thanks.
> >
> > Acked-by: Dave Martin <[email protected]>
>
> Have you guys checked how this works with the recently posted
> "[PATCH v6 0/5] ARM: support for Trusted Foundations secure
> monitor" series?
>
> Regards,
>
> Tony
Hello,
this code looks like some Tegra and "Trusted Foundations"
specific. There is Note: The API followed by Trusted Foundations
does *not* follow the SMC calling conventions. Also code calling
smc #0 instruction, so in my opinion for rx51 it is useless.
Tony, can you include this two rx51 secure patches (patch v4 1/2
and patch v2 2/2)? Or is there some any other problem?
--
Pali Rohár
[email protected]
* Pali Rohár <[email protected]> [130917 09:01]:
> On Tuesday 17 September 2013 17:43:31 Tony Lindgren wrote:
> > Have you guys checked how this works with the recently posted
> > "[PATCH v6 0/5] ARM: support for Trusted Foundations secure
> > monitor" series?
>
> this code looks like some Tegra and "Trusted Foundations"
> specific. There is Note: The API followed by Trusted Foundations
> does *not* follow the SMC calling conventions. Also code calling
> smc #0 instruction, so in my opinion for rx51 it is useless.
OK, so still no generic SMC code then :( This patch is fine
with me.
> Tony, can you include this two rx51 secure patches (patch v4 1/2
> and patch v2 2/2)? Or is there some any other problem?
No other comments on this patch, I'll post some comments on the
v2 2/2 patch considering we're moving to device tree based
booting.
Regards,
Tony
* Pali Rohár <[email protected]> [130710 06:06]:
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
This file will be gone as soon as we're moving to device
tree based booting. So let's do this in more future proof
way.
> +/**
> + * rx51_secure_dispatcher: Routine to dispatch secure PPA API calls
> + * @idx: The PPA API index
> + * @process: Process ID
> + * @flag: The flag indicating criticality of operation
> + * @nargs: Number of valid arguments out of four.
> + * @arg1, arg2, arg3 args4: Parameters passed to secure API
> + *
> + * Return the non-zero error value on failure.
> + */
> +static u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
> + u32 arg1, u32 arg2, u32 arg3, u32 arg4)
> +{
> + u32 ret;
> + u32 param[5];
> +
> + param[0] = nargs+1; /* RX-51 needs number of arguments + 1 */
> + param[1] = arg1;
> + param[2] = arg2;
> + param[3] = arg3;
> + param[4] = arg4;
> +
> + /*
> + * Secure API needs physical address
> + * pointer for the parameters
> + */
> + local_irq_disable();
> + local_fiq_disable();
> + flush_cache_all();
> + outer_clean_range(__pa(param), __pa(param + 5));
> + ret = omap_smc3(idx, process, flag, __pa(param));
> + flush_cache_all();
> + local_fiq_enable();
> + local_irq_enable();
> +
> + return ret;
> +}
I think this used to be in omap-secure.c, and then made rx51
specific.. But since board-rx51.c is going away, let's move
this function to omap-secure.c.
> + * rx51_secure_update_aux_cr: Routine to modify the contents of Auxiliary Control Register
> + * @set_bits: bits to set in ACR
> + * @clr_bits: bits to clear in ACR
> + *
> + * Return the non-zero error value on failure.
> +*/
> +static u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits)
> +{
> + u32 acr;
> +
> + /* Read ACR */
> + asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
> + acr &= ~clear_bits;
> + acr |= set_bits;
> +
> + return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
> + 0,
> + FLAG_START_CRITICAL,
> + 1, acr, 0, 0, 0);
> +}
This too.
> static void __init rx51_init(void)
> {
> struct omap_sdrc_params *sdrc_params;
> @@ -105,6 +175,14 @@ static void __init rx51_init(void)
> rx51_peripherals_init();
> rx51_camera_init();
>
> + if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
> +#ifdef CONFIG_ARM_ERRATA_430973
> + pr_info("RX-51: Enabling ARM errata 430973 workaround\n");
> + /* set IBE to 1 */
> + rx51_secure_update_aux_cr(BIT(6), 0);
> +#endif
> + }
> +
> /* Ensure SDRC pins are mux'd for self-refresh */
> omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
> omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
Then this can be called both from board-generic.c based on the
compatible flag, and board-rx51.c for now.
Regards,
Tony
On Wednesday 18 September 2013 01:24:17 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [130710 06:06]:
> > --- a/arch/arm/mach-omap2/board-rx51.c
> > +++ b/arch/arm/mach-omap2/board-rx51.c
>
> This file will be gone as soon as we're moving to device
> tree based booting. So let's do this in more future proof
> way.
>
> > +/**
> > + * rx51_secure_dispatcher: Routine to dispatch secure PPA
> > API calls + * @idx: The PPA API index
> > + * @process: Process ID
> > + * @flag: The flag indicating criticality of operation
> > + * @nargs: Number of valid arguments out of four.
> > + * @arg1, arg2, arg3 args4: Parameters passed to secure API
> > + *
> > + * Return the non-zero error value on failure.
> > + */
> > +static u32 rx51_secure_dispatcher(u32 idx, u32 process, u32
> > flag, u32 nargs, + u32 arg1, u32 arg2, u32 arg3, u32
> > arg4)
> > +{
> > + u32 ret;
> > + u32 param[5];
> > +
> > + param[0] = nargs+1; /* RX-51 needs number of arguments +
1
> > */ + param[1] = arg1;
> > + param[2] = arg2;
> > + param[3] = arg3;
> > + param[4] = arg4;
> > +
> > + /*
> > + * Secure API needs physical address
> > + * pointer for the parameters
> > + */
> > + local_irq_disable();
> > + local_fiq_disable();
> > + flush_cache_all();
> > + outer_clean_range(__pa(param), __pa(param + 5));
> > + ret = omap_smc3(idx, process, flag, __pa(param));
> > + flush_cache_all();
> > + local_fiq_enable();
> > + local_irq_enable();
> > +
> > + return ret;
> > +}
>
> I think this used to be in omap-secure.c, and then made rx51
> specific.. But since board-rx51.c is going away, let's move
> this function to omap-secure.c.
>
> > + * rx51_secure_update_aux_cr: Routine to modify the
> > contents of Auxiliary Control Register + * @set_bits: bits
> > to set in ACR
> > + * @clr_bits: bits to clear in ACR
> > + *
> > + * Return the non-zero error value on failure.
> > +*/
> > +static u32 rx51_secure_update_aux_cr(u32 set_bits, u32
> > clear_bits) +{
> > + u32 acr;
> > +
> > + /* Read ACR */
> > + asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
> > + acr &= ~clear_bits;
> > + acr |= set_bits;
> > +
> > + return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
> > + 0,
> > + FLAG_START_CRITICAL,
> > + 1, acr, 0, 0, 0);
> > +}
>
> This too.
>
> > static void __init rx51_init(void)
> > {
> >
> > struct omap_sdrc_params *sdrc_params;
> >
> > @@ -105,6 +175,14 @@ static void __init rx51_init(void)
> >
> > rx51_peripherals_init();
> > rx51_camera_init();
> >
> > + if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
> > +#ifdef CONFIG_ARM_ERRATA_430973
> > + pr_info("RX-51: Enabling ARM errata 430973
> > workaround\n"); + /* set IBE to 1 */
> > + rx51_secure_update_aux_cr(BIT(6), 0);
> > +#endif
> > + }
> > +
> >
> > /* Ensure SDRC pins are mux'd for self-refresh */
> > omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
> > omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
>
> Then this can be called both from board-generic.c based on the
> compatible flag, and board-rx51.c for now.
>
> Regards,
>
> Tony
Hi Tony,
I'm not very happy. I sent this patch 6 months ago and only now
you commented that needs rework again. This patch is needed
because all thumb-2 userspace binaries crashing. I want to have
working support for Nokia N900 and not always rebasing and
changing patches. Also DT still not working on N900 (file contains
only small subset of devices as in board files plus it is not in
stable kernel) so I do not want to switch to DT. I need something
which is working and not something new non-working. I belive that
you and other kernel guys do not remove all n900 board files until
every one line will be rewritten to DT and tested that everything
working. And from this and other conversation it looks for me
that you are going to do that. So please clarify what you want to
do (and when) with board-rx51-* files. Aftethat I can decide what
to do in future.
For now I see this situation something like: I wrote patches,
send them to ML and after half of year maintainer politely
rejected them becuase my patches not using new uber cool
technology with still not working and also was not available half
year ago. What happen if I find another time to rework this patch
and send it again in next 2 or 5 months?
Tony, if you did not have time for review this patch months ago
or you found it only today - no problem, I understand it. But
what I need to know is what will happen with board-rx51-* files
(and when?) You can see that DT does not have definitions of all
n900 hw parts (plus it is not in last 3.11 kernel!) which means
that DT is not usable for me and other n900 people. This also
means that I cannot rewrite my patches for DT and test if they
working.
--
Pali Rohár
[email protected]
* Pali Rohár <[email protected]> [130918 01:41]:
>
> I'm not very happy. I sent this patch 6 months ago and only now
> you commented that needs rework again. This patch is needed
> because all thumb-2 userspace binaries crashing. I want to have
> working support for Nokia N900 and not always rebasing and
> changing patches. Also DT still not working on N900 (file contains
> only small subset of devices as in board files plus it is not in
> stable kernel) so I do not want to switch to DT. I need something
> which is working and not something new non-working. I belive that
> you and other kernel guys do not remove all n900 board files until
> every one line will be rewritten to DT and tested that everything
> working. And from this and other conversation it looks for me
> that you are going to do that. So please clarify what you want to
> do (and when) with board-rx51-* files. Aftethat I can decide what
> to do in future.
Sorry if there's been some going back and forth with the patches,
I think pretty much everybody wants n900 support in the mainline.
It's obvious that we're moving everything to be devicetree only
as discussed on the mailing lists over past few years. For most
drivers it's already working, and we can still initialize platform
data too for the legacy devices until they have bindings, so it
should not be too intrusive except for the configuration changes
to use appended DTB or a chained bootloader with DTB support.
> For now I see this situation something like: I wrote patches,
> send them to ML and after half of year maintainer politely
> rejected them becuase my patches not using new uber cool
> technology with still not working and also was not available half
> year ago. What happen if I find another time to rework this patch
> and send it again in next 2 or 5 months?
Hmm hasn't there been pending comments until recently on your
patches?
It seems that with the changes I suggested your patches should
work for both legacy booting and DT based booting, so maybe just
try to update them over next few weeks, let's say by -rc3 rather
than wait 2 to 5 months? :) No need to test them currently on
the DT based booting if you don't have that set up, just move
the code out of the board-*.c file.
> Tony, if you did not have time for review this patch months ago
> or you found it only today - no problem, I understand it. But
> what I need to know is what will happen with board-rx51-* files
> (and when?) You can see that DT does not have definitions of all
> n900 hw parts (plus it is not in last 3.11 kernel!) which means
> that DT is not usable for me and other n900 people. This also
> means that I cannot rewrite my patches for DT and test if they
> working.
I usually stop looking at new code around -rc4 and concentrate
on fixes until -rc1 or so. So there can be easily one month
delays on reviewing stuff depending on where we are with the
current merge window or -rc cycle, sorry if that's annoying.
The .dts files will be separate from the kernel soonish, so
that's not be a show stopper. Just like all the board specific
.config files are separate from the kernel already. Too bad our
.dts changes did not get merged for v3.12 because of conflicts
with other branches, but hey, they should be independent from the
kernel anyways.
The board files will be going away as soon as things are working
with DT. We've been pretty much only applying fixes to them for
quite some time now. For the timeline, the earliest we'll be able
to remove the board-*.c files and platform data is for v3.13
assuming the PM dependencies get sorted out before that. Making
omap3 DT only is going remove about 25k LOC, so that's a good
reason for doing that.
Cheers,
Tony
Hello,
On Wednesday 18 September 2013 19:18:17 Tony Lindgren wrote:
> * Pali Rohár <[email protected]> [130918 01:41]:
> > I'm not very happy. I sent this patch 6 months ago and only
> > now you commented that needs rework again. This patch is
> > needed because all thumb-2 userspace binaries crashing. I
> > want to have working support for Nokia N900 and not always
> > rebasing and changing patches. Also DT still not working on
> > N900 (file contains only small subset of devices as in
> > board files plus it is not in stable kernel) so I do not
> > want to switch to DT. I need something which is working and
> > not something new non-working. I belive that you and other
> > kernel guys do not remove all n900 board files until every
> > one line will be rewritten to DT and tested that everything
> > working. And from this and other conversation it looks for
> > me that you are going to do that. So please clarify what
> > you want to do (and when) with board-rx51-* files. Aftethat
> > I can decide what to do in future.
>
> Sorry if there's been some going back and forth with the
> patches, I think pretty much everybody wants n900 support in
> the mainline.
>
> It's obvious that we're moving everything to be devicetree
> only as discussed on the mailing lists over past few years.
> For most drivers it's already working, and we can still
> initialize platform data too for the legacy devices until
> they have bindings, so it should not be too intrusive except
> for the configuration changes to use appended DTB or a
> chained bootloader with DTB support.
>
> > For now I see this situation something like: I wrote
> > patches, send them to ML and after half of year maintainer
> > politely rejected them becuase my patches not using new
> > uber cool technology with still not working and also was
> > not available half year ago. What happen if I find another
> > time to rework this patch and send it again in next 2 or 5
> > months?
>
> Hmm hasn't there been pending comments until recently on your
> patches?
>
Since 10.07.2013 I do not have any emails for patch 2/2. If I
missed something from you, please resend me it.
> It seems that with the changes I suggested your patches should
> work for both legacy booting and DT based booting, so maybe
> just try to update them over next few weeks, let's say by
> -rc3 rather than wait 2 to 5 months? :) No need to test them
> currently on the DT based booting if you don't have that set
> up, just move the code out of the board-*.c file.
>
Ok, no problem. I will move code as you suggested.
> > Tony, if you did not have time for review this patch months
> > ago or you found it only today - no problem, I understand
> > it. But what I need to know is what will happen with
> > board-rx51-* files (and when?) You can see that DT does not
> > have definitions of all n900 hw parts (plus it is not in
> > last 3.11 kernel!) which means that DT is not usable for me
> > and other n900 people. This also means that I cannot
> > rewrite my patches for DT and test if they working.
>
> I usually stop looking at new code around -rc4 and concentrate
> on fixes until -rc1 or so. So there can be easily one month
> delays on reviewing stuff depending on where we are with the
> current merge window or -rc cycle, sorry if that's annoying.
>
> The .dts files will be separate from the kernel soonish, so
> that's not be a show stopper. Just like all the board specific
> .config files are separate from the kernel already. Too bad
> our .dts changes did not get merged for v3.12 because of
> conflicts with other branches, but hey, they should be
> independent from the kernel anyways.
>
> The board files will be going away as soon as things are
> working with DT. We've been pretty much only applying fixes
> to them for quite some time now. For the timeline, the
> earliest we'll be able to remove the board-*.c files and
> platform data is for v3.13 assuming the PM dependencies get
> sorted out before that. Making omap3 DT only is going remove
> about 25k LOC, so that's a good reason for doing that.
>
> Cheers,
>
> Tony
Thanks for clarification.
--
Pali Rohár
[email protected]
* Pali Rohár <[email protected]> [130918 11:21]:
> On Wednesday 18 September 2013 19:18:17 Tony Lindgren wrote:
> >
> > Hmm hasn't there been pending comments until recently on your
> > patches?
> >
>
> Since 10.07.2013 I do not have any emails for patch 2/2. If I
> missed something from you, please resend me it.
Sorry I noticed it only when looking at them again from "keep
in my inbox for applying" or "mark as read and archive" point
of view. So no other comments from me are pending.
> > It seems that with the changes I suggested your patches should
> > work for both legacy booting and DT based booting, so maybe
> > just try to update them over next few weeks, let's say by
> > -rc3 rather than wait 2 to 5 months? :) No need to test them
> > currently on the DT based booting if you don't have that set
> > up, just move the code out of the board-*.c file.
> >
>
> Ok, no problem. I will move code as you suggested.
Thanks,
Tony
Hello Tony,
here is new v3 patch. I just only moved functions rx51_secure_dispatcher and
rx51_secure_update_aux_cr to omap-secure.c and added header to omap-secure.h
Because I only moved two functions to other source file I tested only compilation.
It is OK now?
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 773510556..db168c9 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -2,6 +2,8 @@
* Board support file for Nokia N900 (aka RX-51).
*
* Copyright (C) 2007, 2008 Nokia
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -31,7 +33,9 @@
#include "mux.h"
#include "gpmc.h"
#include "pm.h"
+#include "soc.h"
#include "sdram-nokia.h"
+#include "omap-secure.h"
#define RX51_GPIO_SLEEP_IND 162
@@ -103,6 +107,14 @@ static void __init rx51_init(void)
usb_musb_init(&musb_board_data);
rx51_peripherals_init();
+ if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
+#ifdef CONFIG_ARM_ERRATA_430973
+ pr_info("RX-51: Enabling ARM errata 430973 workaround\n");
+ /* set IBE to 1 */
+ rx51_secure_update_aux_cr(BIT(6), 0);
+#endif
+ }
+
/* Ensure SDRC pins are mux'd for self-refresh */
omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c
index b970440..eba74cf 100644
--- a/arch/arm/mach-omap2/omap-secure.c
+++ b/arch/arm/mach-omap2/omap-secure.c
@@ -3,6 +3,8 @@
*
* Copyright (C) 2011 Texas Instruments, Inc.
* Santosh Shilimkar <[email protected]>
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
*
* This program is free software,you can redistribute it and/or modify
@@ -70,3 +72,63 @@ phys_addr_t omap_secure_ram_mempool_base(void)
{
return omap_secure_memblock_base;
}
+
+/**
+ * rx51_secure_dispatcher: Routine to dispatch secure PPA API calls
+ * @idx: The PPA API index
+ * @process: Process ID
+ * @flag: The flag indicating criticality of operation
+ * @nargs: Number of valid arguments out of four.
+ * @arg1, arg2, arg3 args4: Parameters passed to secure API
+ *
+ * Return the non-zero error value on failure.
+ */
+u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
+ u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+{
+ u32 ret;
+ u32 param[5];
+
+ param[0] = nargs+1; /* RX-51 needs number of arguments + 1 */
+ param[1] = arg1;
+ param[2] = arg2;
+ param[3] = arg3;
+ param[4] = arg4;
+
+ /*
+ * Secure API needs physical address
+ * pointer for the parameters
+ */
+ local_irq_disable();
+ local_fiq_disable();
+ flush_cache_all();
+ outer_clean_range(__pa(param), __pa(param + 5));
+ ret = omap_smc3(idx, process, flag, __pa(param));
+ flush_cache_all();
+ local_fiq_enable();
+ local_irq_enable();
+
+ return ret;
+}
+
+/**
+ * rx51_secure_update_aux_cr: Routine to modify the contents of Auxiliary Control Register
+ * @set_bits: bits to set in ACR
+ * @clr_bits: bits to clear in ACR
+ *
+ * Return the non-zero error value on failure.
+*/
+u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits)
+{
+ u32 acr;
+
+ /* Read ACR */
+ asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
+ acr &= ~clear_bits;
+ acr |= set_bits;
+
+ return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
+ 0,
+ FLAG_START_CRITICAL,
+ 1, acr, 0, 0, 0);
+}
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index c4586f4..51b59c6 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -3,6 +3,8 @@
*
* Copyright (C) 2011 Texas Instruments, Inc.
* Santosh Shilimkar <[email protected]>
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -46,6 +48,11 @@
#define OMAP4_PPA_L2_POR_INDEX 0x23
#define OMAP4_PPA_CPU_ACTRL_SMP_INDEX 0x25
+/* Secure RX-51 PPA (Primary Protected Application) APIs */
+#define RX51_PPA_HWRNG 29
+#define RX51_PPA_L2_INVAL 40
+#define RX51_PPA_WRITE_ACR 42
+
#ifndef __ASSEMBLER__
extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
@@ -55,6 +62,10 @@ extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
extern phys_addr_t omap_secure_ram_mempool_base(void);
extern int omap_secure_ram_reserve_memblock(void);
+extern u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
+ u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+extern u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits);
+
#ifdef CONFIG_OMAP4_ERRATA_I688
extern int omap_barrier_reserve_memblock(void);
#else
--
Pali Rohár
[email protected]
* Pali Rohár <[email protected]> [130918 12:29]:
> Hello Tony,
>
> here is new v3 patch. I just only moved functions rx51_secure_dispatcher and
> rx51_secure_update_aux_cr to omap-secure.c and added header to omap-secure.h
> Because I only moved two functions to other source file I tested only compilation.
> It is OK now?
Yes looks OK to me for v3.13 for the patch.
For the patch description, maybe also add something saying
that the rx51_secure_dispatcher() is different from the existing
smc function as that may be easy to miss while glancing over
the code?
Regards,
Tony
> diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
> index 773510556..db168c9 100644
> --- a/arch/arm/mach-omap2/board-rx51.c
> +++ b/arch/arm/mach-omap2/board-rx51.c
> @@ -2,6 +2,8 @@
> * Board support file for Nokia N900 (aka RX-51).
> *
> * Copyright (C) 2007, 2008 Nokia
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Rohár <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -31,7 +33,9 @@
> #include "mux.h"
> #include "gpmc.h"
> #include "pm.h"
> +#include "soc.h"
> #include "sdram-nokia.h"
> +#include "omap-secure.h"
>
> #define RX51_GPIO_SLEEP_IND 162
>
> @@ -103,6 +107,14 @@ static void __init rx51_init(void)
> usb_musb_init(&musb_board_data);
> rx51_peripherals_init();
>
> + if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
> +#ifdef CONFIG_ARM_ERRATA_430973
> + pr_info("RX-51: Enabling ARM errata 430973 workaround\n");
> + /* set IBE to 1 */
> + rx51_secure_update_aux_cr(BIT(6), 0);
> +#endif
> + }
> +
> /* Ensure SDRC pins are mux'd for self-refresh */
> omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
> omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
> diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c
> index b970440..eba74cf 100644
> --- a/arch/arm/mach-omap2/omap-secure.c
> +++ b/arch/arm/mach-omap2/omap-secure.c
> @@ -3,6 +3,8 @@
> *
> * Copyright (C) 2011 Texas Instruments, Inc.
> * Santosh Shilimkar <[email protected]>
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Rohár <[email protected]>
> *
> *
> * This program is free software,you can redistribute it and/or modify
> @@ -70,3 +72,63 @@ phys_addr_t omap_secure_ram_mempool_base(void)
> {
> return omap_secure_memblock_base;
> }
> +
> +/**
> + * rx51_secure_dispatcher: Routine to dispatch secure PPA API calls
> + * @idx: The PPA API index
> + * @process: Process ID
> + * @flag: The flag indicating criticality of operation
> + * @nargs: Number of valid arguments out of four.
> + * @arg1, arg2, arg3 args4: Parameters passed to secure API
> + *
> + * Return the non-zero error value on failure.
> + */
> +u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
> + u32 arg1, u32 arg2, u32 arg3, u32 arg4)
> +{
> + u32 ret;
> + u32 param[5];
> +
> + param[0] = nargs+1; /* RX-51 needs number of arguments + 1 */
> + param[1] = arg1;
> + param[2] = arg2;
> + param[3] = arg3;
> + param[4] = arg4;
> +
> + /*
> + * Secure API needs physical address
> + * pointer for the parameters
> + */
> + local_irq_disable();
> + local_fiq_disable();
> + flush_cache_all();
> + outer_clean_range(__pa(param), __pa(param + 5));
> + ret = omap_smc3(idx, process, flag, __pa(param));
> + flush_cache_all();
> + local_fiq_enable();
> + local_irq_enable();
> +
> + return ret;
> +}
> +
> +/**
> + * rx51_secure_update_aux_cr: Routine to modify the contents of Auxiliary Control Register
> + * @set_bits: bits to set in ACR
> + * @clr_bits: bits to clear in ACR
> + *
> + * Return the non-zero error value on failure.
> +*/
> +u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits)
> +{
> + u32 acr;
> +
> + /* Read ACR */
> + asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
> + acr &= ~clear_bits;
> + acr |= set_bits;
> +
> + return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
> + 0,
> + FLAG_START_CRITICAL,
> + 1, acr, 0, 0, 0);
> +}
> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> index c4586f4..51b59c6 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -3,6 +3,8 @@
> *
> * Copyright (C) 2011 Texas Instruments, Inc.
> * Santosh Shilimkar <[email protected]>
> + * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
> + * Copyright (C) 2013 Pali Rohár <[email protected]>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -46,6 +48,11 @@
> #define OMAP4_PPA_L2_POR_INDEX 0x23
> #define OMAP4_PPA_CPU_ACTRL_SMP_INDEX 0x25
>
> +/* Secure RX-51 PPA (Primary Protected Application) APIs */
> +#define RX51_PPA_HWRNG 29
> +#define RX51_PPA_L2_INVAL 40
> +#define RX51_PPA_WRITE_ACR 42
> +
> #ifndef __ASSEMBLER__
>
> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> @@ -55,6 +62,10 @@ extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> extern phys_addr_t omap_secure_ram_mempool_base(void);
> extern int omap_secure_ram_reserve_memblock(void);
>
> +extern u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
> + u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> +extern u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits);
> +
> #ifdef CONFIG_OMAP4_ERRATA_I688
> extern int omap_barrier_reserve_memblock(void);
> #else
>
>
> --
> Pali Rohár
> [email protected]
Closed and signed Nokia X-Loader bootloader stored in RX-51 nand does not set
IBE bit in ACTLR and starting kernel in non-secure mode. So direct write to
ACTLR by our kernel does not working and the code for ARM errata 430973 in
commit 7ce236fcd6fd45b0441a2d49acb2ceb2de2e8a47 that sets IBE bit is a noop.
In order to have workaround for ARM errata 430973 from non-secure world on
RX-51 we needs Secure Monitor Call to set IBE BIT in ACTLR.
This patch adds RX-51 specific secure support code and sets IBE bit in ACTLR
during board init code for ARM errata 430973 workaround.
Note that new function rx51_secure_dispatcher() differs from existing
omap_secure_dispatcher(). It calling omap_smc3() and param[0] is nargs+1.
ARM errata 430973 workaround is needed for thumb-2 ISA compiled userspace
binaries. Without this workaround thumb-2 binaries crashing. So with this
patch it is possible to recompile and run applications/binaries with thumb-2
ISA on RX-51.
Signed-off-by: Ivaylo Dimitrov <[email protected]>
Signed-off-by: Pali Rohár <[email protected]>
---
I added note about difference between rx51_secure_dispatcher and
omap_secure_dispatcher to commit message and also to code.
arch/arm/mach-omap2/board-rx51.c | 12 +++++++
arch/arm/mach-omap2/omap-secure.c | 65 +++++++++++++++++++++++++++++++++++++
arch/arm/mach-omap2/omap-secure.h | 11 +++++++
3 files changed, 88 insertions(+)
diff --git a/arch/arm/mach-omap2/board-rx51.c b/arch/arm/mach-omap2/board-rx51.c
index 773510556..db168c9 100644
--- a/arch/arm/mach-omap2/board-rx51.c
+++ b/arch/arm/mach-omap2/board-rx51.c
@@ -2,6 +2,8 @@
* Board support file for Nokia N900 (aka RX-51).
*
* Copyright (C) 2007, 2008 Nokia
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -31,7 +33,9 @@
#include "mux.h"
#include "gpmc.h"
#include "pm.h"
+#include "soc.h"
#include "sdram-nokia.h"
+#include "omap-secure.h"
#define RX51_GPIO_SLEEP_IND 162
@@ -103,6 +107,14 @@ static void __init rx51_init(void)
usb_musb_init(&musb_board_data);
rx51_peripherals_init();
+ if (omap_type() == OMAP2_DEVICE_TYPE_SEC) {
+#ifdef CONFIG_ARM_ERRATA_430973
+ pr_info("RX-51: Enabling ARM errata 430973 workaround\n");
+ /* set IBE to 1 */
+ rx51_secure_update_aux_cr(BIT(6), 0);
+#endif
+ }
+
/* Ensure SDRC pins are mux'd for self-refresh */
omap_mux_init_signal("sdrc_cke0", OMAP_PIN_OUTPUT);
omap_mux_init_signal("sdrc_cke1", OMAP_PIN_OUTPUT);
diff --git a/arch/arm/mach-omap2/omap-secure.c b/arch/arm/mach-omap2/omap-secure.c
index b970440..146a7c4 100644
--- a/arch/arm/mach-omap2/omap-secure.c
+++ b/arch/arm/mach-omap2/omap-secure.c
@@ -3,6 +3,8 @@
*
* Copyright (C) 2011 Texas Instruments, Inc.
* Santosh Shilimkar <[email protected]>
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
*
* This program is free software,you can redistribute it and/or modify
@@ -70,3 +72,66 @@ phys_addr_t omap_secure_ram_mempool_base(void)
{
return omap_secure_memblock_base;
}
+
+/**
+ * rx51_secure_dispatcher: Routine to dispatch secure PPA API calls
+ * @idx: The PPA API index
+ * @process: Process ID
+ * @flag: The flag indicating criticality of operation
+ * @nargs: Number of valid arguments out of four.
+ * @arg1, arg2, arg3 args4: Parameters passed to secure API
+ *
+ * Return the non-zero error value on failure.
+ *
+ * NOTE: rx51_secure_dispatcher differs from omap_secure_dispatcher because
+ * it calling omap_smc3() instead omap_smc2() and param[0] is nargs+1
+ */
+u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
+ u32 arg1, u32 arg2, u32 arg3, u32 arg4)
+{
+ u32 ret;
+ u32 param[5];
+
+ param[0] = nargs+1; /* RX-51 needs number of arguments + 1 */
+ param[1] = arg1;
+ param[2] = arg2;
+ param[3] = arg3;
+ param[4] = arg4;
+
+ /*
+ * Secure API needs physical address
+ * pointer for the parameters
+ */
+ local_irq_disable();
+ local_fiq_disable();
+ flush_cache_all();
+ outer_clean_range(__pa(param), __pa(param + 5));
+ ret = omap_smc3(idx, process, flag, __pa(param));
+ flush_cache_all();
+ local_fiq_enable();
+ local_irq_enable();
+
+ return ret;
+}
+
+/**
+ * rx51_secure_update_aux_cr: Routine to modify the contents of Auxiliary Control Register
+ * @set_bits: bits to set in ACR
+ * @clr_bits: bits to clear in ACR
+ *
+ * Return the non-zero error value on failure.
+*/
+u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits)
+{
+ u32 acr;
+
+ /* Read ACR */
+ asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r" (acr));
+ acr &= ~clear_bits;
+ acr |= set_bits;
+
+ return rx51_secure_dispatcher(RX51_PPA_WRITE_ACR,
+ 0,
+ FLAG_START_CRITICAL,
+ 1, acr, 0, 0, 0);
+}
diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
index c4586f4..51b59c6 100644
--- a/arch/arm/mach-omap2/omap-secure.h
+++ b/arch/arm/mach-omap2/omap-secure.h
@@ -3,6 +3,8 @@
*
* Copyright (C) 2011 Texas Instruments, Inc.
* Santosh Shilimkar <[email protected]>
+ * Copyright (C) 2012 Ivaylo Dimitrov <[email protected]>
+ * Copyright (C) 2013 Pali Rohár <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -46,6 +48,11 @@
#define OMAP4_PPA_L2_POR_INDEX 0x23
#define OMAP4_PPA_CPU_ACTRL_SMP_INDEX 0x25
+/* Secure RX-51 PPA (Primary Protected Application) APIs */
+#define RX51_PPA_HWRNG 29
+#define RX51_PPA_L2_INVAL 40
+#define RX51_PPA_WRITE_ACR 42
+
#ifndef __ASSEMBLER__
extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
@@ -55,6 +62,10 @@ extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
extern phys_addr_t omap_secure_ram_mempool_base(void);
extern int omap_secure_ram_reserve_memblock(void);
+extern u32 rx51_secure_dispatcher(u32 idx, u32 process, u32 flag, u32 nargs,
+ u32 arg1, u32 arg2, u32 arg3, u32 arg4);
+extern u32 rx51_secure_update_aux_cr(u32 set_bits, u32 clear_bits);
+
#ifdef CONFIG_OMAP4_ERRATA_I688
extern int omap_barrier_reserve_memblock(void);
#else
--
1.7.10.4
Hi!
> > Tony, if you did not have time for review this patch months ago
> > or you found it only today - no problem, I understand it. But
> > what I need to know is what will happen with board-rx51-* files
> > (and when?) You can see that DT does not have definitions of all
> > n900 hw parts (plus it is not in last 3.11 kernel!) which means
> > that DT is not usable for me and other n900 people. This also
> > means that I cannot rewrite my patches for DT and test if they
> > working.
>
> I usually stop looking at new code around -rc4 and concentrate
> on fixes until -rc1 or so. So there can be easily one month
> delays on reviewing stuff depending on where we are with the
> current merge window or -rc cycle, sorry if that's annoying.
>
> The .dts files will be separate from the kernel soonish, so
> that's not be a show stopper. Just like all the board specific
> .config files are separate from the kernel already. Too bad our
> .dts changes did not get merged for v3.12 because of conflicts
> with other branches, but hey, they should be independent from the
> kernel anyways.
Well... I'd say it is still time to merge them. "New hardware support"
is ok after -rc1.
> The board files will be going away as soon as things are working
> with DT. We've been pretty much only applying fixes to them for
> quite some time now. For the timeline, the earliest we'll be able
> to remove the board-*.c files and platform data is for v3.13
And it would be good to merge n900 dts support for 3.12. Adding
device tree at the same release as removing board support files in
same release leaves zero room for testing, and practically guarantees
regressions.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
* Pavel Machek <[email protected]> [130923 17:23]:
> Hi!
>
> > > Tony, if you did not have time for review this patch months ago
> > > or you found it only today - no problem, I understand it. But
> > > what I need to know is what will happen with board-rx51-* files
> > > (and when?) You can see that DT does not have definitions of all
> > > n900 hw parts (plus it is not in last 3.11 kernel!) which means
> > > that DT is not usable for me and other n900 people. This also
> > > means that I cannot rewrite my patches for DT and test if they
> > > working.
> >
> > I usually stop looking at new code around -rc4 and concentrate
> > on fixes until -rc1 or so. So there can be easily one month
> > delays on reviewing stuff depending on where we are with the
> > current merge window or -rc cycle, sorry if that's annoying.
> >
> > The .dts files will be separate from the kernel soonish, so
> > that's not be a show stopper. Just like all the board specific
> > .config files are separate from the kernel already. Too bad our
> > .dts changes did not get merged for v3.12 because of conflicts
> > with other branches, but hey, they should be independent from the
> > kernel anyways.
>
> Well... I'd say it is still time to merge them. "New hardware support"
> is ok after -rc1.
Sure if there's a good reason like "driver foo is already device
tree only and cannot be used for board bar without blah".
> > The board files will be going away as soon as things are working
> > with DT. We've been pretty much only applying fixes to them for
> > quite some time now. For the timeline, the earliest we'll be able
> > to remove the board-*.c files and platform data is for v3.13
>
> And it would be good to merge n900 dts support for 3.12. Adding
> device tree at the same release as removing board support files in
> same release leaves zero room for testing, and practically guarantees
> regressions.
Yes we experienced that with omap4 conversion where it caused
some wl12xx regression. So we might want to keep some omap3
board files around until v3.14 if necessary.
Also please take a look at the "[PATCH 0/4] Clean up legacy platform
data handling for omaps for v3.13" patches I posted few days ago,
those should make it easy to have board specific pdata quirks
for DT based booting.
Regards,
Tony
On 08/09/13 09:43, Pali Rohár wrote:
> Here is new version (v4) of omap secure part patch:
>
> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> but Nokia RX-51 board needs to call smc #1 for PPA access.
>
> Signed-off-by: Ivaylo Dimitrov<[email protected]>
> Signed-off-by: Pali Rohár<[email protected]>
> ---
> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> index 0e72917..c4586f4 100644
> --- a/arch/arm/mach-omap2/omap-secure.h
> +++ b/arch/arm/mach-omap2/omap-secure.h
> @@ -51,6 +51,7 @@
> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> extern phys_addr_t omap_secure_ram_mempool_base(void);
> extern int omap_secure_ram_reserve_memblock(void);
>
> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> index f6441c1..fd90125 100644
> --- a/arch/arm/mach-omap2/omap-smc.S
> +++ b/arch/arm/mach-omap2/omap-smc.S
> @@ -1,9 +1,11 @@
> /*
> - * OMAP44xx secure APIs file.
> + * OMAP34xx and OMAP44xx secure APIs file.
> *
> * Copyright (C) 2010 Texas Instruments, Inc.
> * Written by Santosh Shilimkar<[email protected]>
> *
> + * Copyright (C) 2012 Ivaylo Dimitrov<[email protected]>
> + * Copyright (C) 2013 Pali Rohár<[email protected]>
> *
> * This program is free software,you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> ldmfd sp!, {r4-r12, pc}
> ENDPROC(omap_smc2)
>
> +/**
> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> + * Low level common routine for secure HAL and PPA APIs via smc #1
> + * r0 - @service_id: Secure Service ID
> + * r1 - @process_id: Process ID
> + * r2 - @flag: Flag to indicate the criticality of operation
> + * r3 - @pargs: Physical address of parameter list
> + */
> +ENTRY(omap_smc3)
> + stmfd sp!, {r4-r11, lr}
> + mov r12, r0 @ Copy the secure service ID
I think you should save r12 in the call.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
On Tue, Oct 08, 2013 at 09:13:16AM +0200, Ben Dooks wrote:
> On 08/09/13 09:43, Pali Roh?r wrote:
>> Here is new version (v4) of omap secure part patch:
>>
>> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
>> but Nokia RX-51 board needs to call smc #1 for PPA access.
>>
>> Signed-off-by: Ivaylo Dimitrov<[email protected]>
>> Signed-off-by: Pali Roh?r<[email protected]>
>> ---
>> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
>> index 0e72917..c4586f4 100644
>> --- a/arch/arm/mach-omap2/omap-secure.h
>> +++ b/arch/arm/mach-omap2/omap-secure.h
>> @@ -51,6 +51,7 @@
>> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
>> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
>> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
>> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
>> extern phys_addr_t omap_secure_ram_mempool_base(void);
>> extern int omap_secure_ram_reserve_memblock(void);
>>
>> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
>> index f6441c1..fd90125 100644
>> --- a/arch/arm/mach-omap2/omap-smc.S
>> +++ b/arch/arm/mach-omap2/omap-smc.S
>> @@ -1,9 +1,11 @@
>> /*
>> - * OMAP44xx secure APIs file.
>> + * OMAP34xx and OMAP44xx secure APIs file.
>> *
>> * Copyright (C) 2010 Texas Instruments, Inc.
>> * Written by Santosh Shilimkar<[email protected]>
>> *
>> + * Copyright (C) 2012 Ivaylo Dimitrov<[email protected]>
>> + * Copyright (C) 2013 Pali Roh?r<[email protected]>
>> *
>> * This program is free software,you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
>> ldmfd sp!, {r4-r12, pc}
>> ENDPROC(omap_smc2)
>>
>> +/**
>> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
>> + * Low level common routine for secure HAL and PPA APIs via smc #1
>> + * r0 - @service_id: Secure Service ID
>> + * r1 - @process_id: Process ID
>> + * r2 - @flag: Flag to indicate the criticality of operation
>> + * r3 - @pargs: Physical address of parameter list
>> + */
>> +ENTRY(omap_smc3)
>> + stmfd sp!, {r4-r11, lr}
>> + mov r12, r0 @ Copy the secure service ID
>
> I think you should save r12 in the call.
Not necessary.
* Russell King - ARM Linux <[email protected]> [131008 01:17]:
> On Tue, Oct 08, 2013 at 09:13:16AM +0200, Ben Dooks wrote:
> > On 08/09/13 09:43, Pali Rohár wrote:
> >> Here is new version (v4) of omap secure part patch:
> >>
> >> Other secure functions omap_smc1() and omap_smc2() calling instruction smc #0
> >> but Nokia RX-51 board needs to call smc #1 for PPA access.
> >>
> >> Signed-off-by: Ivaylo Dimitrov<[email protected]>
> >> Signed-off-by: Pali Rohár<[email protected]>
> >> ---
> >> diff --git a/arch/arm/mach-omap2/omap-secure.h b/arch/arm/mach-omap2/omap-secure.h
> >> index 0e72917..c4586f4 100644
> >> --- a/arch/arm/mach-omap2/omap-secure.h
> >> +++ b/arch/arm/mach-omap2/omap-secure.h
> >> @@ -51,6 +51,7 @@
> >> extern u32 omap_secure_dispatcher(u32 idx, u32 flag, u32 nargs,
> >> u32 arg1, u32 arg2, u32 arg3, u32 arg4);
> >> extern u32 omap_smc2(u32 id, u32 falg, u32 pargs);
> >> +extern u32 omap_smc3(u32 id, u32 process, u32 flag, u32 pargs);
> >> extern phys_addr_t omap_secure_ram_mempool_base(void);
> >> extern int omap_secure_ram_reserve_memblock(void);
> >>
> >> diff --git a/arch/arm/mach-omap2/omap-smc.S b/arch/arm/mach-omap2/omap-smc.S
> >> index f6441c1..fd90125 100644
> >> --- a/arch/arm/mach-omap2/omap-smc.S
> >> +++ b/arch/arm/mach-omap2/omap-smc.S
> >> @@ -1,9 +1,11 @@
> >> /*
> >> - * OMAP44xx secure APIs file.
> >> + * OMAP34xx and OMAP44xx secure APIs file.
> >> *
> >> * Copyright (C) 2010 Texas Instruments, Inc.
> >> * Written by Santosh Shilimkar<[email protected]>
> >> *
> >> + * Copyright (C) 2012 Ivaylo Dimitrov<[email protected]>
> >> + * Copyright (C) 2013 Pali Rohár<[email protected]>
> >> *
> >> * This program is free software,you can redistribute it and/or modify
> >> * it under the terms of the GNU General Public License version 2 as
> >> @@ -54,6 +56,23 @@ ENTRY(omap_smc2)
> >> ldmfd sp!, {r4-r12, pc}
> >> ENDPROC(omap_smc2)
> >>
> >> +/**
> >> + * u32 omap_smc3(u32 service_id, u32 process_id, u32 flag, u32 pargs)
> >> + * Low level common routine for secure HAL and PPA APIs via smc #1
> >> + * r0 - @service_id: Secure Service ID
> >> + * r1 - @process_id: Process ID
> >> + * r2 - @flag: Flag to indicate the criticality of operation
> >> + * r3 - @pargs: Physical address of parameter list
> >> + */
> >> +ENTRY(omap_smc3)
> >> + stmfd sp!, {r4-r11, lr}
> >> + mov r12, r0 @ Copy the secure service ID
> >
> > I think you should save r12 in the call.
>
> Not necessary.
Assuming there are no other comments I'll apply these into
omap-for-v3.13/n900 branch.
Regards,
Tony