| Message ID | 1542696136-5240-2-git-send-email-sumit.garg@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/3] libtomcrypt: Import SHA512/256 approved hash algorithm | expand |
On Tue, Nov 20, 2018 at 12:12:15PM +0530, Sumit Garg wrote: > Secure timer interrupt is configured to fire every 2ms. This is not really what I would call a description. I'd be thinking of something more like: > Currently there is no means to perform background housekeeping in > secure world on Synquacer platforms. Provide an (optional) periodic > timer to allow any housekeeping to be performed. > > Although it could be expanded, at present the code is fairly simple > because we expect only a single PTA to exploit the timer interrupt. > The secure timer interrupt is configured to fire every 2ms. Note that the general concept above is based on advise from some of our own training material although the exact form of the problem, why, solution, because will vary based on context (see slide 5): https://docs.google.com/presentation/d/1IpgMM2EpjPUq5rjxpa9gKcuibtTQnKq7v_NTk9ZmhfY/edit#slide=id.g11c2b4d937_0_5 BTW in general I review code like a kernel developer... OP-TEE has similar process but in some cases they might have different habits. Daniel. > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > core/arch/arm/include/arm64.h | 4 +++ > core/arch/arm/plat-synquacer/main.c | 30 +++++++++++++++--- > core/arch/arm/plat-synquacer/platform_config.h | 2 ++ > core/arch/arm/plat-synquacer/sub.mk | 1 + > core/arch/arm/plat-synquacer/timer_fiq.c | 43 ++++++++++++++++++++++++++ > core/arch/arm/plat-synquacer/timer_fiq.h | 15 +++++++++ > 6 files changed, 90 insertions(+), 5 deletions(-) > create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.c > create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.h > > diff --git a/core/arch/arm/include/arm64.h b/core/arch/arm/include/arm64.h > index 2c1fd8c..0cf14c0 100644 > --- a/core/arch/arm/include/arm64.h > +++ b/core/arch/arm/include/arm64.h > @@ -305,6 +305,10 @@ DEFINE_REG_READ_FUNC_(cntfrq, uint32_t, cntfrq_el0) > DEFINE_REG_READ_FUNC_(cntpct, uint64_t, cntpct_el0) > DEFINE_REG_READ_FUNC_(cntkctl, uint32_t, cntkctl_el1) > DEFINE_REG_WRITE_FUNC_(cntkctl, uint32_t, cntkctl_el1) > +DEFINE_REG_READ_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) > +DEFINE_REG_WRITE_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) > +DEFINE_REG_READ_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) > +DEFINE_REG_WRITE_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) > > DEFINE_REG_READ_FUNC_(pmccntr, uint64_t, pmccntr_el0) > > diff --git a/core/arch/arm/plat-synquacer/main.c b/core/arch/arm/plat-synquacer/main.c > index c3aac4c..714becd 100644 > --- a/core/arch/arm/plat-synquacer/main.c > +++ b/core/arch/arm/plat-synquacer/main.c > @@ -18,6 +18,7 @@ > #include <sm/optee_smc.h> > #include <tee/entry_fast.h> > #include <tee/entry_std.h> > +#include <timer_fiq.h> > > static void main_fiq(void); > > @@ -46,7 +47,7 @@ const struct thread_handlers *generic_boot_get_handlers(void) > > static void main_fiq(void) > { > - panic(); > + gic_it_handle(&gic_data); > } > > void console_init(void) > @@ -66,12 +67,31 @@ void main_init_gic(void) > if (!gicd_base) > panic(); > > - /* Initialize GIC */ > - gic_init(&gic_data, 0, gicd_base); > + /* On ARMv8-A, GIC configuration is initialized in TF-A */ > + gic_init_base_addr(&gic_data, 0, gicd_base); > + > itr_init(&gic_data.chip); > } > > -void main_secondary_init_gic(void) > +static enum itr_return timer_itr_cb(struct itr_handler *h __unused) > +{ > + /* Reset timer for next FIQ */ > + generic_timer_handler(); > + > + return ITRR_HANDLED; > +} > + > +static struct itr_handler timer_itr = { > + .it = IT_SEC_TIMER, > + .flags = ITRF_TRIGGER_LEVEL, > + .handler = timer_itr_cb, > +}; > + > +static TEE_Result init_timer_itr(void) > { > - gic_cpu_init(&gic_data); > + itr_add(&timer_itr); > + itr_enable(IT_SEC_TIMER); > + > + return TEE_SUCCESS; > } > +driver_init(init_timer_itr); > diff --git a/core/arch/arm/plat-synquacer/platform_config.h b/core/arch/arm/plat-synquacer/platform_config.h > index 4d6d545..f9b1b40 100644 > --- a/core/arch/arm/plat-synquacer/platform_config.h > +++ b/core/arch/arm/plat-synquacer/platform_config.h > @@ -19,6 +19,8 @@ > #define CONSOLE_UART_CLK_IN_HZ 62500000 > #define CONSOLE_BAUDRATE 115200 > > +#define IT_SEC_TIMER 29 > + > #define DRAM0_BASE 0x80000000 > > /* Platform specific defines */ > diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk > index 8ddc2fd..cfa1dc3 100644 > --- a/core/arch/arm/plat-synquacer/sub.mk > +++ b/core/arch/arm/plat-synquacer/sub.mk > @@ -1,2 +1,3 @@ > global-incdirs-y += . > srcs-y += main.c > +srcs-y += timer_fiq.c > diff --git a/core/arch/arm/plat-synquacer/timer_fiq.c b/core/arch/arm/plat-synquacer/timer_fiq.c > new file mode 100644 > index 0000000..e8395d9 > --- /dev/null > +++ b/core/arch/arm/plat-synquacer/timer_fiq.c > @@ -0,0 +1,43 @@ > +// SPDX-License-Identifier: BSD-2-Clause > +/* > + * Copyright (c) 2018, Linaro Limited > + */ > + > +#include <arm.h> > +#include <console.h> > +#include <drivers/gic.h> > +#include <io.h> > +#include <kernel/panic.h> > +#include <kernel/misc.h> > +#include <timer_fiq.h> > + > +bool timer_fiq_running = false; > + > +void generic_timer_start(void) > +{ > + uint64_t cval; > + uint32_t ctl = 1; > + > + /* The timer will fire every 2 ms */ > + cval = read_cntpct() + (read_cntfrq() / 500); > + write_cntps_cval(cval); > + > + /* Enable the secure physical timer */ > + write_cntps_ctl(ctl); > +} > + > +void generic_timer_stop(void) > +{ > + /* Disable the timer */ > + write_cntps_ctl(0); > +} > + > +void generic_timer_handler(void) > +{ > + /* Ensure that the timer did assert the interrupt */ > + assert((read_cntps_ctl() >> 2)); > + > + /* Disable the timer and reprogram it */ > + write_cntps_ctl(0); > + generic_timer_start(); > +} > diff --git a/core/arch/arm/plat-synquacer/timer_fiq.h b/core/arch/arm/plat-synquacer/timer_fiq.h > new file mode 100644 > index 0000000..ebbe5ba > --- /dev/null > +++ b/core/arch/arm/plat-synquacer/timer_fiq.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: BSD-2-Clause */ > +/* > + * Copyright (C) 2018, Linaro Limited > + */ > + > +#ifndef __TIMER_FIQ_H > +#define __TIMER_FIQ_H > + > +extern bool timer_fiq_running; > + > +void generic_timer_start(void); > +void generic_timer_stop(void); > +void generic_timer_handler(void); > + > +#endif /* __TIMER_FIQ_H */ > -- > 2.7.4 >
On Tue, 20 Nov 2018 at 20:00, Daniel Thompson <daniel.thompson@linaro.org> wrote: > > On Tue, Nov 20, 2018 at 12:12:15PM +0530, Sumit Garg wrote: > > Secure timer interrupt is configured to fire every 2ms. > > This is not really what I would call a description. I'd be thinking of > something more like: > > > Currently there is no means to perform background housekeeping in > > secure world on Synquacer platforms. Provide an (optional) periodic > > timer to allow any housekeeping to be performed. > > > > Although it could be expanded, at present the code is fairly simple > > because we expect only a single PTA to exploit the timer interrupt. > > The secure timer interrupt is configured to fire every 2ms. > Looks good to me. Will use this instead. > Note that the general concept above is based on advise from some of our > own training material although the exact form of the problem, why, > solution, because will vary based on context (see slide 5): > https://docs.google.com/presentation/d/1IpgMM2EpjPUq5rjxpa9gKcuibtTQnKq7v_NTk9ZmhfY/edit#slide=id.g11c2b4d937_0_5 > Thanks for sharing this info. -Sumit > BTW in general I review code like a kernel developer... OP-TEE has > similar process but in some cases they might have different habits. > > > Daniel. > > > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > > --- > > core/arch/arm/include/arm64.h | 4 +++ > > core/arch/arm/plat-synquacer/main.c | 30 +++++++++++++++--- > > core/arch/arm/plat-synquacer/platform_config.h | 2 ++ > > core/arch/arm/plat-synquacer/sub.mk | 1 + > > core/arch/arm/plat-synquacer/timer_fiq.c | 43 ++++++++++++++++++++++++++ > > core/arch/arm/plat-synquacer/timer_fiq.h | 15 +++++++++ > > 6 files changed, 90 insertions(+), 5 deletions(-) > > create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.c > > create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.h > > > > diff --git a/core/arch/arm/include/arm64.h b/core/arch/arm/include/arm64.h > > index 2c1fd8c..0cf14c0 100644 > > --- a/core/arch/arm/include/arm64.h > > +++ b/core/arch/arm/include/arm64.h > > @@ -305,6 +305,10 @@ DEFINE_REG_READ_FUNC_(cntfrq, uint32_t, cntfrq_el0) > > DEFINE_REG_READ_FUNC_(cntpct, uint64_t, cntpct_el0) > > DEFINE_REG_READ_FUNC_(cntkctl, uint32_t, cntkctl_el1) > > DEFINE_REG_WRITE_FUNC_(cntkctl, uint32_t, cntkctl_el1) > > +DEFINE_REG_READ_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) > > +DEFINE_REG_WRITE_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) > > +DEFINE_REG_READ_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) > > +DEFINE_REG_WRITE_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) > > > > DEFINE_REG_READ_FUNC_(pmccntr, uint64_t, pmccntr_el0) > > > > diff --git a/core/arch/arm/plat-synquacer/main.c b/core/arch/arm/plat-synquacer/main.c > > index c3aac4c..714becd 100644 > > --- a/core/arch/arm/plat-synquacer/main.c > > +++ b/core/arch/arm/plat-synquacer/main.c > > @@ -18,6 +18,7 @@ > > #include <sm/optee_smc.h> > > #include <tee/entry_fast.h> > > #include <tee/entry_std.h> > > +#include <timer_fiq.h> > > > > static void main_fiq(void); > > > > @@ -46,7 +47,7 @@ const struct thread_handlers *generic_boot_get_handlers(void) > > > > static void main_fiq(void) > > { > > - panic(); > > + gic_it_handle(&gic_data); > > } > > > > void console_init(void) > > @@ -66,12 +67,31 @@ void main_init_gic(void) > > if (!gicd_base) > > panic(); > > > > - /* Initialize GIC */ > > - gic_init(&gic_data, 0, gicd_base); > > + /* On ARMv8-A, GIC configuration is initialized in TF-A */ > > + gic_init_base_addr(&gic_data, 0, gicd_base); > > + > > itr_init(&gic_data.chip); > > } > > > > -void main_secondary_init_gic(void) > > +static enum itr_return timer_itr_cb(struct itr_handler *h __unused) > > +{ > > + /* Reset timer for next FIQ */ > > + generic_timer_handler(); > > + > > + return ITRR_HANDLED; > > +} > > + > > +static struct itr_handler timer_itr = { > > + .it = IT_SEC_TIMER, > > + .flags = ITRF_TRIGGER_LEVEL, > > + .handler = timer_itr_cb, > > +}; > > + > > +static TEE_Result init_timer_itr(void) > > { > > - gic_cpu_init(&gic_data); > > + itr_add(&timer_itr); > > + itr_enable(IT_SEC_TIMER); > > + > > + return TEE_SUCCESS; > > } > > +driver_init(init_timer_itr); > > diff --git a/core/arch/arm/plat-synquacer/platform_config.h b/core/arch/arm/plat-synquacer/platform_config.h > > index 4d6d545..f9b1b40 100644 > > --- a/core/arch/arm/plat-synquacer/platform_config.h > > +++ b/core/arch/arm/plat-synquacer/platform_config.h > > @@ -19,6 +19,8 @@ > > #define CONSOLE_UART_CLK_IN_HZ 62500000 > > #define CONSOLE_BAUDRATE 115200 > > > > +#define IT_SEC_TIMER 29 > > + > > #define DRAM0_BASE 0x80000000 > > > > /* Platform specific defines */ > > diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk > > index 8ddc2fd..cfa1dc3 100644 > > --- a/core/arch/arm/plat-synquacer/sub.mk > > +++ b/core/arch/arm/plat-synquacer/sub.mk > > @@ -1,2 +1,3 @@ > > global-incdirs-y += . > > srcs-y += main.c > > +srcs-y += timer_fiq.c > > diff --git a/core/arch/arm/plat-synquacer/timer_fiq.c b/core/arch/arm/plat-synquacer/timer_fiq.c > > new file mode 100644 > > index 0000000..e8395d9 > > --- /dev/null > > +++ b/core/arch/arm/plat-synquacer/timer_fiq.c > > @@ -0,0 +1,43 @@ > > +// SPDX-License-Identifier: BSD-2-Clause > > +/* > > + * Copyright (c) 2018, Linaro Limited > > + */ > > + > > +#include <arm.h> > > +#include <console.h> > > +#include <drivers/gic.h> > > +#include <io.h> > > +#include <kernel/panic.h> > > +#include <kernel/misc.h> > > +#include <timer_fiq.h> > > + > > +bool timer_fiq_running = false; > > + > > +void generic_timer_start(void) > > +{ > > + uint64_t cval; > > + uint32_t ctl = 1; > > + > > + /* The timer will fire every 2 ms */ > > + cval = read_cntpct() + (read_cntfrq() / 500); > > + write_cntps_cval(cval); > > + > > + /* Enable the secure physical timer */ > > + write_cntps_ctl(ctl); > > +} > > + > > +void generic_timer_stop(void) > > +{ > > + /* Disable the timer */ > > + write_cntps_ctl(0); > > +} > > + > > +void generic_timer_handler(void) > > +{ > > + /* Ensure that the timer did assert the interrupt */ > > + assert((read_cntps_ctl() >> 2)); > > + > > + /* Disable the timer and reprogram it */ > > + write_cntps_ctl(0); > > + generic_timer_start(); > > +} > > diff --git a/core/arch/arm/plat-synquacer/timer_fiq.h b/core/arch/arm/plat-synquacer/timer_fiq.h > > new file mode 100644 > > index 0000000..ebbe5ba > > --- /dev/null > > +++ b/core/arch/arm/plat-synquacer/timer_fiq.h > > @@ -0,0 +1,15 @@ > > +/* SPDX-License-Identifier: BSD-2-Clause */ > > +/* > > + * Copyright (C) 2018, Linaro Limited > > + */ > > + > > +#ifndef __TIMER_FIQ_H > > +#define __TIMER_FIQ_H > > + > > +extern bool timer_fiq_running; > > + > > +void generic_timer_start(void); > > +void generic_timer_stop(void); > > +void generic_timer_handler(void); > > + > > +#endif /* __TIMER_FIQ_H */ > > -- > > 2.7.4 > >
diff --git a/core/arch/arm/include/arm64.h b/core/arch/arm/include/arm64.h index 2c1fd8c..0cf14c0 100644 --- a/core/arch/arm/include/arm64.h +++ b/core/arch/arm/include/arm64.h @@ -305,6 +305,10 @@ DEFINE_REG_READ_FUNC_(cntfrq, uint32_t, cntfrq_el0) DEFINE_REG_READ_FUNC_(cntpct, uint64_t, cntpct_el0) DEFINE_REG_READ_FUNC_(cntkctl, uint32_t, cntkctl_el1) DEFINE_REG_WRITE_FUNC_(cntkctl, uint32_t, cntkctl_el1) +DEFINE_REG_READ_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) +DEFINE_REG_WRITE_FUNC_(cntps_ctl, uint32_t, cntps_ctl_el1) +DEFINE_REG_READ_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) +DEFINE_REG_WRITE_FUNC_(cntps_cval, uint32_t, cntps_cval_el1) DEFINE_REG_READ_FUNC_(pmccntr, uint64_t, pmccntr_el0) diff --git a/core/arch/arm/plat-synquacer/main.c b/core/arch/arm/plat-synquacer/main.c index c3aac4c..714becd 100644 --- a/core/arch/arm/plat-synquacer/main.c +++ b/core/arch/arm/plat-synquacer/main.c @@ -18,6 +18,7 @@ #include <sm/optee_smc.h> #include <tee/entry_fast.h> #include <tee/entry_std.h> +#include <timer_fiq.h> static void main_fiq(void); @@ -46,7 +47,7 @@ const struct thread_handlers *generic_boot_get_handlers(void) static void main_fiq(void) { - panic(); + gic_it_handle(&gic_data); } void console_init(void) @@ -66,12 +67,31 @@ void main_init_gic(void) if (!gicd_base) panic(); - /* Initialize GIC */ - gic_init(&gic_data, 0, gicd_base); + /* On ARMv8-A, GIC configuration is initialized in TF-A */ + gic_init_base_addr(&gic_data, 0, gicd_base); + itr_init(&gic_data.chip); } -void main_secondary_init_gic(void) +static enum itr_return timer_itr_cb(struct itr_handler *h __unused) +{ + /* Reset timer for next FIQ */ + generic_timer_handler(); + + return ITRR_HANDLED; +} + +static struct itr_handler timer_itr = { + .it = IT_SEC_TIMER, + .flags = ITRF_TRIGGER_LEVEL, + .handler = timer_itr_cb, +}; + +static TEE_Result init_timer_itr(void) { - gic_cpu_init(&gic_data); + itr_add(&timer_itr); + itr_enable(IT_SEC_TIMER); + + return TEE_SUCCESS; } +driver_init(init_timer_itr); diff --git a/core/arch/arm/plat-synquacer/platform_config.h b/core/arch/arm/plat-synquacer/platform_config.h index 4d6d545..f9b1b40 100644 --- a/core/arch/arm/plat-synquacer/platform_config.h +++ b/core/arch/arm/plat-synquacer/platform_config.h @@ -19,6 +19,8 @@ #define CONSOLE_UART_CLK_IN_HZ 62500000 #define CONSOLE_BAUDRATE 115200 +#define IT_SEC_TIMER 29 + #define DRAM0_BASE 0x80000000 /* Platform specific defines */ diff --git a/core/arch/arm/plat-synquacer/sub.mk b/core/arch/arm/plat-synquacer/sub.mk index 8ddc2fd..cfa1dc3 100644 --- a/core/arch/arm/plat-synquacer/sub.mk +++ b/core/arch/arm/plat-synquacer/sub.mk @@ -1,2 +1,3 @@ global-incdirs-y += . srcs-y += main.c +srcs-y += timer_fiq.c diff --git a/core/arch/arm/plat-synquacer/timer_fiq.c b/core/arch/arm/plat-synquacer/timer_fiq.c new file mode 100644 index 0000000..e8395d9 --- /dev/null +++ b/core/arch/arm/plat-synquacer/timer_fiq.c @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: BSD-2-Clause +/* + * Copyright (c) 2018, Linaro Limited + */ + +#include <arm.h> +#include <console.h> +#include <drivers/gic.h> +#include <io.h> +#include <kernel/panic.h> +#include <kernel/misc.h> +#include <timer_fiq.h> + +bool timer_fiq_running = false; + +void generic_timer_start(void) +{ + uint64_t cval; + uint32_t ctl = 1; + + /* The timer will fire every 2 ms */ + cval = read_cntpct() + (read_cntfrq() / 500); + write_cntps_cval(cval); + + /* Enable the secure physical timer */ + write_cntps_ctl(ctl); +} + +void generic_timer_stop(void) +{ + /* Disable the timer */ + write_cntps_ctl(0); +} + +void generic_timer_handler(void) +{ + /* Ensure that the timer did assert the interrupt */ + assert((read_cntps_ctl() >> 2)); + + /* Disable the timer and reprogram it */ + write_cntps_ctl(0); + generic_timer_start(); +} diff --git a/core/arch/arm/plat-synquacer/timer_fiq.h b/core/arch/arm/plat-synquacer/timer_fiq.h new file mode 100644 index 0000000..ebbe5ba --- /dev/null +++ b/core/arch/arm/plat-synquacer/timer_fiq.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/* + * Copyright (C) 2018, Linaro Limited + */ + +#ifndef __TIMER_FIQ_H +#define __TIMER_FIQ_H + +extern bool timer_fiq_running; + +void generic_timer_start(void); +void generic_timer_stop(void); +void generic_timer_handler(void); + +#endif /* __TIMER_FIQ_H */
Secure timer interrupt is configured to fire every 2ms. Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- core/arch/arm/include/arm64.h | 4 +++ core/arch/arm/plat-synquacer/main.c | 30 +++++++++++++++--- core/arch/arm/plat-synquacer/platform_config.h | 2 ++ core/arch/arm/plat-synquacer/sub.mk | 1 + core/arch/arm/plat-synquacer/timer_fiq.c | 43 ++++++++++++++++++++++++++ core/arch/arm/plat-synquacer/timer_fiq.h | 15 +++++++++ 6 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.c create mode 100644 core/arch/arm/plat-synquacer/timer_fiq.h -- 2.7.4