* Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1
@ 2013-08-11 18:36 Ивайло Димитров
2013-09-04 8:10 ` Pali Rohár
0 siblings, 1 reply; 5+ messages in thread
From: Ивайло Димитров @ 2013-08-11 18:36 UTC (permalink / raw)
To: Dave Martin
Cc: pali.rohar, tony, nm, linux, aaro.koskinen, pdeschrijver,
linux-kernel, santosh.shilimkar, pavel, linux-omap,
linux-arm-kernel
>-------- Оригинално писмо --------
>От: 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1
2013-08-11 18:36 [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1 Ивайло Димитров
@ 2013-09-04 8:10 ` Pali Rohár
0 siblings, 0 replies; 5+ messages in thread
From: Pali Rohár @ 2013-09-04 8:10 UTC (permalink / raw)
To: Ивайло
Димитров,
Dave Martin, tony, nm
Cc: linux, aaro.koskinen, pdeschrijver, linux-kernel,
santosh.shilimkar, pavel, linux-omap, linux-arm-kernel
[-- Attachment #1: Type: Text/Plain, Size: 3690 bytes --]
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
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1
@ 2013-09-04 12:43 Dave Martin
0 siblings, 0 replies; 5+ messages in thread
From: Dave Martin @ 2013-09-04 12:43 UTC (permalink / raw)
To: Pali Rohár
Cc: Ивайло
Димитров,
tony, nm, linux, aaro.koskinen, pdeschrijver, linux-kernel,
santosh.shilimkar, pavel, linux-omap, linux-arm-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1
2013-08-04 8:45 ` [PATCH v3 " Pali Rohár
@ 2013-08-05 13:29 ` Dave Martin
0 siblings, 0 replies; 5+ messages in thread
From: Dave Martin @ 2013-08-05 13:29 UTC (permalink / raw)
To: Pali Rohár
Cc: tony, nm, linux, aaro.koskinen, pdeschrijver, linux-kernel,
Ивайло
Димитров,
Santosh Shilimkar, pavel, linux-omap, linux-arm-kernel
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 <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..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 <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
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1
2013-07-12 10:24 ` Dave Martin
@ 2013-08-04 8:45 ` Pali Rohár
2013-08-05 13:29 ` Dave Martin
0 siblings, 1 reply; 5+ messages in thread
From: Pali Rohár @ 2013-08-04 8:45 UTC (permalink / raw)
To: tony
Cc: Dave Martin, Santosh Shilimkar,
Ивайло
Димитров,
nm, linux, aaro.koskinen, pdeschrijver, linux-kernel, pavel,
linux-omap, linux-arm-kernel
[-- Attachment #1: Type: Text/Plain, Size: 2341 bytes --]
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 <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..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 <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
+ 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 --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-09-04 12:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-11 18:36 [PATCH v3 1/2] ARM: OMAP: Add secure function omap_smc3() which calling instruction smc #1 Ивайло Димитров
2013-09-04 8:10 ` Pali Rohár
-- strict thread matches above, loose matches on Subject: below --
2013-09-04 12:43 Dave Martin
2013-07-11 19:43 [PATCH v2 " Ивайло Димитров
2013-07-11 19:54 ` Santosh Shilimkar
2013-07-12 10:24 ` Dave Martin
2013-08-04 8:45 ` [PATCH v3 " Pali Rohár
2013-08-05 13:29 ` Dave Martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).