Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Dave Martin]

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

[PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2387 bytes --]

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 <freemangordon@abv.bg>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
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 <santosh.shilimkar@ti.com>
  *
+ * Copyright (C) 2012 Ivaylo Dimitrov <freemangordon@abv.bg>
+ * Copyright (C) 2013 Pali Rohár <pali.rohar@gmail.com>
  *
  * 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
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2588 bytes --]

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 <freemangordon@abv.bg>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> 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 <santosh.shilimkar@ti.com>
>   *
> + * Copyright (C) 2012 Ivaylo Dimitrov <freemangordon@abv.bg>
> + * Copyright (C) 2013 Pali Rohár <pali.rohar@gmail.com>
>   *
>   * 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
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Dave Martin]

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 <freemangordon@abv.bg>
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> > 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 <santosh.shilimkar@ti.com>
> >   *
> > + * Copyright (C) 2012 Ivaylo Dimitrov <freemangordon@abv.bg>
> > + * Copyright (C) 2013 Pali Rohár <pali.rohar@gmail.com>
> >   *
> >   * 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 <Dave.Martin@arm.com>

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Tony Lindgren]

* Dave Martin <Dave.Martin@arm.com> [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 <Dave.Martin@arm.com>

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
 

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1656 bytes --]

On Tuesday 17 September 2013 17:43:31 Tony Lindgren wrote:
> * Dave Martin <Dave.Martin@arm.com> [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 <Dave.Martin@arm.com>
> 
> 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
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Tony Lindgren]

* Pali Rohár <pali.rohar@gmail.com> [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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Ben Dooks]

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<freemangordon@abv.bg>
> Signed-off-by: Pali Rohár<pali.rohar@gmail.com>
> ---
> 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<santosh.shilimkar@ti.com>
>    *
> + * Copyright (C) 2012 Ivaylo Dimitrov<freemangordon@abv.bg>
> + * Copyright (C) 2013 Pali Rohár<pali.rohar@gmail.com>
>    *
>    * 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Russell King - ARM Linux]

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<freemangordon@abv.bg>
>> Signed-off-by: Pali Rohár<pali.rohar@gmail.com>
>> ---
>> 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<santosh.shilimkar@ti.com>
>>    *
>> + * Copyright (C) 2012 Ivaylo Dimitrov<freemangordon@abv.bg>
>> + * Copyright (C) 2013 Pali Rohár<pali.rohar@gmail.com>
>>    *
>>    * 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.

Re: [PATCH v4 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [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<freemangordon@abv.bg>
> >> Signed-off-by: Pali Rohár<pali.rohar@gmail.com>
> >> ---
> >> 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<santosh.shilimkar@ti.com>
> >>    *
> >> + * Copyright (C) 2012 Ivaylo Dimitrov<freemangordon@abv.bg>
> >> + * Copyright (C) 2013 Pali Rohár<pali.rohar@gmail.com>
> >>    *
> >>    * 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