diff options
author | Sam Nelson | 2016-03-02 14:57:49 -0600 |
---|---|---|
committer | Suman Anna | 2016-03-03 18:15:35 -0600 |
commit | 1340944c3656ceb089615929f8efed651f6d4f9e (patch) | |
tree | 8bb61750bc8b0d3708e7fb3d6163aa8c5d42c6d3 | |
parent | 8528a646e7fc85833cdec2497e665f5d67c7319a (diff) | |
download | remoteproc-rproc-linux-4.1.y.tar.gz remoteproc-rproc-linux-4.1.y.tar.xz remoteproc-rproc-linux-4.1.y.zip |
remoteproc/keystone: fix boot and reset for images with resource tablerproc-linux-4.1.y
The remoteproc core supports booting and resetting of a remote processor
automatically for images with a resource table supporting virtio devices.
The current keystone remoteproc driver also supports this feature when
using the kernel remoteproc core loader, and supported this using two
dedicated ioctls (KEYSTONE_RPROC_IOC_DSP_RESET/BOOT), and another ioctl
(KEYSTONE_RPROC_IOC_SET_STATE) to create/delete the virtio devices when
using the userspace loader.
This design has a race condition for the first processor getting booted
using the userspace loader for the first time. The race is between the
processing and loading of the resource table into the remoteproc device
memory in the kernel upon a SET_STATE ioctl, and the booting of the
processor triggered from userspace with the DSP_BOOT ioctl. This caused
an incorrect execution of the firmware due to incorrect device memory
contents, specifically the loaded resource table. The timing delay is
attributed to the time taken for the autoloading of the virtio_rpmsg_bus
driver/module by udev when the first virtio device is registered during
the SET_STATE ioctl. The issue is not seen if virtio_rpmsg_bus module is
already loaded before loading and booting of a remote processor.
This patch fixes the above issue by modifying the existing design of
requiring separate IOCTL operations to perform DSP boot/reset for images
that supported resource table. The design change now automatically boots
or shuts down the remote processor during the KEYSTONE_RPROC_IOC_SET_STATE
ioctl, similar to the flow when using the kernel remoteproc core loader.
The boot address/entry point is now published during the SET_STATE ioctl
and is stored in a local variable in driver instance and later picked up
by remoteproc core driver through fw_ops. The DSP reset/start operations
itself are initiated by start/stop rproc fw_ops calls from the remoteproc
core. The resource table state variables would also have to be cleared
appropriately when the remote processor is shut down using SET_STATE
ioctls.
The KEYSTONE_RPROC_IOC_DSP_RESET/BOOT ioctls will continue to be used to
boot and reset a remote processor from userspace for images without a
resource table. Checks have been added to these ioctls to make sure these
are not used directly on images with a resource table.
NOTE:
This patch requires a corresponding logic update in the userpace loader
(MultiProcMgr).
Signed-off-by: Sam Nelson <sam.nelson@ti.com>
[s-anna@ti.com: revised comments/patch description & formatting changes]
Signed-off-by: Suman Anna <s-anna@ti.com>
-rw-r--r-- | drivers/remoteproc/keystone_remoteproc.c | 100 | ||||
-rw-r--r-- | include/uapi/linux/keystone_remoteproc.h | 19 |
2 files changed, 92 insertions, 27 deletions
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c index 7ec8126ab215..9e88100b9f73 100644 --- a/drivers/remoteproc/keystone_remoteproc.c +++ b/drivers/remoteproc/keystone_remoteproc.c | |||
@@ -1,7 +1,7 @@ | |||
1 | /* | 1 | /* |
2 | * TI Keystone DSP remoteproc driver | 2 | * TI Keystone DSP remoteproc driver |
3 | * | 3 | * |
4 | * Copyright (C) 2015 Texas Instruments, Inc. | 4 | * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ |
5 | * | 5 | * |
6 | * This program is free software; you can redistribute it and/or | 6 | * This program is free software; you can redistribute it and/or |
7 | * modify it under the terms of the GNU General Public License | 7 | * modify it under the terms of the GNU General Public License |
@@ -87,6 +87,7 @@ struct keystone_rproc_mem { | |||
87 | * @rsc_table: resource table pointer copied from userspace | 87 | * @rsc_table: resource table pointer copied from userspace |
88 | * @rsc_table_size: size of resource table | 88 | * @rsc_table_size: size of resource table |
89 | * @loaded_rsc_table: kernel pointer of loaded resource table | 89 | * @loaded_rsc_table: kernel pointer of loaded resource table |
90 | * @boot_addr: remote processor boot address used with userspace loader | ||
90 | * @open_count: fops open reference counter | 91 | * @open_count: fops open reference counter |
91 | */ | 92 | */ |
92 | struct keystone_rproc { | 93 | struct keystone_rproc { |
@@ -112,6 +113,7 @@ struct keystone_rproc { | |||
112 | struct resource_table *rsc_table; | 113 | struct resource_table *rsc_table; |
113 | int rsc_table_size; | 114 | int rsc_table_size; |
114 | void *loaded_rsc_table; | 115 | void *loaded_rsc_table; |
116 | u32 boot_addr; | ||
115 | int open_count; | 117 | int open_count; |
116 | }; | 118 | }; |
117 | 119 | ||
@@ -150,29 +152,52 @@ static int keystone_rproc_uio_irqcontrol(struct uio_info *uio, s32 irq_on) | |||
150 | return 0; | 152 | return 0; |
151 | } | 153 | } |
152 | 154 | ||
155 | /* Reset previously set rsc table variables */ | ||
156 | static void keystone_rproc_reset_rsc_table(struct keystone_rproc *ksproc) | ||
157 | { | ||
158 | kfree(ksproc->rsc_table); | ||
159 | ksproc->rsc_table = NULL; | ||
160 | ksproc->loaded_rsc_table = NULL; | ||
161 | ksproc->rsc_table_size = 0; | ||
162 | } | ||
163 | |||
153 | /* | 164 | /* |
154 | * Create/delete the virtio devices in kernel once the user-space loading is | 165 | * Create/delete the virtio devices in kernel once the user-space loading is |
155 | * complete, and configure the remoteproc states appropriately. The resource | 166 | * complete, configure the remoteproc states appropriately, and boot or reset |
156 | * table should have been published through the KEYSTONE_RPROC_IOC_SET_RSC_TABLE | 167 | * the remote processor. The resource table should have been published through |
157 | * and KEYSTONE_RPROC_IOC_SET_LOADED_RSC_TABLE ioctls before invoking this. | 168 | * KEYSTONE_RPROC_IOC_SET_RSC_TABLE & KEYSTONE_RPROC_IOC_SET_LOADED_RSC_TABLE |
169 | * ioctls before invoking this. The boot address is passed through the | ||
170 | * KEYSTONE_RPROC_IOC_SET_STATE ioctl when setting the KEYSTONE_RPROC_RUNNING | ||
171 | * state. | ||
158 | * | 172 | * |
159 | * XXX: Note that this is currently not really booting or resetting the | 173 | * NOTE: |
160 | * DSP devices. They are handled through KEYSTONE_RPROC_IOC_DSP_RESET and | 174 | * The ioctls KEYSTONE_RPROC_IOC_DSP_RESET and KEYSTONE_RPROC_IOC_DSP_BOOT |
161 | * KEYSTONE_RPROC_IOC_DSP_BOOT ioctls, and it needs to be evaluated if | 175 | * are now restricted to support the booting or resetting the DSP devices |
162 | * there is a real need for those ioctls. | 176 | * only for firmware images without any resource table. |
163 | */ | 177 | */ |
164 | static int keystone_rproc_set_state(struct keystone_rproc *ksproc, | 178 | static int keystone_rproc_set_state(struct keystone_rproc *ksproc, |
165 | enum keystone_rproc_state state) | 179 | void __user *argp) |
166 | { | 180 | { |
167 | struct rproc *rproc = ksproc->rproc; | 181 | struct rproc *rproc = ksproc->rproc; |
182 | struct keystone_rproc_set_state_params set_state_params; | ||
168 | int ret = 0; | 183 | int ret = 0; |
169 | 184 | ||
170 | switch (state) { | 185 | if (copy_from_user(&set_state_params, argp, sizeof(set_state_params))) |
186 | return -EFAULT; | ||
187 | |||
188 | switch (set_state_params.state) { | ||
171 | case KEYSTONE_RPROC_RUNNING: | 189 | case KEYSTONE_RPROC_RUNNING: |
172 | if (!ksproc->rsc_table || !ksproc->loaded_rsc_table) | 190 | if (!ksproc->rsc_table || !ksproc->loaded_rsc_table) |
173 | return -EINVAL; | 191 | return -EINVAL; |
174 | 192 | ||
175 | /* | 193 | /* |
194 | * store boot address for .get_boot_addr() rproc fw ops | ||
195 | * XXX: validate the boot address so it is not set to a | ||
196 | * random address | ||
197 | */ | ||
198 | ksproc->boot_addr = set_state_params.boot_addr; | ||
199 | |||
200 | /* | ||
176 | * add virtio devices, rproc_boot will be invoked by remoteproc | 201 | * add virtio devices, rproc_boot will be invoked by remoteproc |
177 | * core if they are present | 202 | * core if they are present |
178 | */ | 203 | */ |
@@ -197,6 +222,10 @@ static int keystone_rproc_set_state(struct keystone_rproc *ksproc, | |||
197 | 222 | ||
198 | rproc_remove_vdevs_direct(rproc); | 223 | rproc_remove_vdevs_direct(rproc); |
199 | 224 | ||
225 | mutex_lock(&ksproc->mlock); | ||
226 | keystone_rproc_reset_rsc_table(ksproc); | ||
227 | mutex_unlock(&ksproc->mlock); | ||
228 | |||
200 | break; | 229 | break; |
201 | 230 | ||
202 | default: | 231 | default: |
@@ -322,7 +351,7 @@ keystone_rproc_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) | |||
322 | 351 | ||
323 | switch (cmd) { | 352 | switch (cmd) { |
324 | case KEYSTONE_RPROC_IOC_SET_STATE: | 353 | case KEYSTONE_RPROC_IOC_SET_STATE: |
325 | ret = keystone_rproc_set_state(ksproc, arg); | 354 | ret = keystone_rproc_set_state(ksproc, argp); |
326 | break; | 355 | break; |
327 | 356 | ||
328 | case KEYSTONE_RPROC_IOC_SET_RSC_TABLE: | 357 | case KEYSTONE_RPROC_IOC_SET_RSC_TABLE: |
@@ -334,10 +363,20 @@ keystone_rproc_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) | |||
334 | break; | 363 | break; |
335 | 364 | ||
336 | case KEYSTONE_RPROC_IOC_DSP_RESET: | 365 | case KEYSTONE_RPROC_IOC_DSP_RESET: |
366 | if (ksproc->rsc_table) { | ||
367 | ret = -EINVAL; | ||
368 | break; | ||
369 | } | ||
370 | |||
337 | keystone_rproc_dsp_reset(ksproc); | 371 | keystone_rproc_dsp_reset(ksproc); |
338 | break; | 372 | break; |
339 | 373 | ||
340 | case KEYSTONE_RPROC_IOC_DSP_BOOT: | 374 | case KEYSTONE_RPROC_IOC_DSP_BOOT: |
375 | if (ksproc->rsc_table) { | ||
376 | ret = -EINVAL; | ||
377 | break; | ||
378 | } | ||
379 | |||
341 | ret = keystone_rproc_dsp_boot(ksproc, arg); | 380 | ret = keystone_rproc_dsp_boot(ksproc, arg); |
342 | break; | 381 | break; |
343 | 382 | ||
@@ -446,10 +485,7 @@ static int keystone_rproc_release(struct inode *inode, struct file *filp) | |||
446 | WARN_ON(rproc->state != RPROC_OFFLINE); | 485 | WARN_ON(rproc->state != RPROC_OFFLINE); |
447 | } | 486 | } |
448 | 487 | ||
449 | kfree(ksproc->rsc_table); | 488 | keystone_rproc_reset_rsc_table(ksproc); |
450 | ksproc->rsc_table = NULL; | ||
451 | ksproc->loaded_rsc_table = NULL; | ||
452 | ksproc->rsc_table_size = 0; | ||
453 | 489 | ||
454 | end: | 490 | end: |
455 | clk_disable_unprepare(ksproc->clk); | 491 | clk_disable_unprepare(ksproc->clk); |
@@ -509,12 +545,28 @@ keystone_rproc_find_loaded_rsc_table(struct rproc *rproc, | |||
509 | } | 545 | } |
510 | 546 | ||
511 | /* | 547 | /* |
548 | * Used only with userspace loader/boot mechanism, the boot address | ||
549 | * is published to the kernel-level through an ioctl call and is | ||
550 | * stored in a local variable in the keystone_rproc device structure. | ||
551 | * Return this address to the remoteproc core through the .get_boot_addr() | ||
552 | * remoteproc firmware ops | ||
553 | */ | ||
554 | static u32 keystone_rproc_get_boot_addr(struct rproc *rproc, | ||
555 | const struct firmware *fw) | ||
556 | { | ||
557 | struct keystone_rproc *ksproc = rproc->priv; | ||
558 | |||
559 | return ksproc->boot_addr; | ||
560 | } | ||
561 | |||
562 | /* | ||
512 | * Used only with userspace loader/boot mechanism, otherwise the remoteproc | 563 | * Used only with userspace loader/boot mechanism, otherwise the remoteproc |
513 | * core elf loader's functions are relied on. | 564 | * core elf loader's functions are relied on. |
514 | */ | 565 | */ |
515 | static struct rproc_fw_ops keystone_rproc_fw_ops = { | 566 | static struct rproc_fw_ops keystone_rproc_fw_ops = { |
516 | .find_rsc_table = keystone_rproc_find_rsc_table, | 567 | .find_rsc_table = keystone_rproc_find_rsc_table, |
517 | .find_loaded_rsc_table = keystone_rproc_find_loaded_rsc_table, | 568 | .find_loaded_rsc_table = keystone_rproc_find_loaded_rsc_table, |
569 | .get_boot_addr = keystone_rproc_get_boot_addr, | ||
518 | }; | 570 | }; |
519 | 571 | ||
520 | /* | 572 | /* |
@@ -609,15 +661,15 @@ static int keystone_rproc_start(struct rproc *rproc) | |||
609 | goto out; | 661 | goto out; |
610 | } | 662 | } |
611 | 663 | ||
612 | if (rproc->use_userspace_loader) | 664 | if (!rproc->use_userspace_loader) { |
613 | goto out; | 665 | ret = request_irq(ksproc->irq_fault, |
614 | 666 | keystone_rproc_exception_interrupt, | |
615 | ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt, | 667 | IRQF_ONESHOT, dev_name(ksproc->dev), ksproc); |
616 | IRQF_ONESHOT, dev_name(ksproc->dev), ksproc); | 668 | if (ret) { |
617 | if (ret) { | 669 | dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n", |
618 | dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n", | 670 | ret); |
619 | ret); | 671 | goto free_vring_irq; |
620 | goto free_vring_irq; | 672 | } |
621 | } | 673 | } |
622 | 674 | ||
623 | ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr); | 675 | ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr); |
diff --git a/include/uapi/linux/keystone_remoteproc.h b/include/uapi/linux/keystone_remoteproc.h index afdd4f8f2bf6..2c0395268662 100644 --- a/include/uapi/linux/keystone_remoteproc.h +++ b/include/uapi/linux/keystone_remoteproc.h | |||
@@ -1,5 +1,5 @@ | |||
1 | /* | 1 | /* |
2 | * Copyright (C) 2015 Texas Instruments Incorporated | 2 | * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/ |
3 | * | 3 | * |
4 | * This program is free software; you can redistribute it and/or | 4 | * This program is free software; you can redistribute it and/or |
5 | * modify it under the terms of the GNU General Public License as | 5 | * modify it under the terms of the GNU General Public License as |
@@ -30,6 +30,18 @@ enum keystone_rproc_state { | |||
30 | KEYSTONE_RPROC_RUNNING, | 30 | KEYSTONE_RPROC_RUNNING, |
31 | }; | 31 | }; |
32 | 32 | ||
33 | /** | ||
34 | * struct keystone_rproc_set_state_params - keystone remoteproc set state | ||
35 | * parameters structure | ||
36 | * | ||
37 | * @state: enumerated state value to set | ||
38 | * @boot_addr: boot address/entry point for the remote processor | ||
39 | */ | ||
40 | struct keystone_rproc_set_state_params { | ||
41 | enum keystone_rproc_state state; | ||
42 | uint32_t boot_addr; | ||
43 | }; | ||
44 | |||
33 | /* Macros used within mmap function */ | 45 | /* Macros used within mmap function */ |
34 | #define KEYSTONE_RPROC_UIO_MAP_INDEX_MASK (0x7) | 46 | #define KEYSTONE_RPROC_UIO_MAP_INDEX_MASK (0x7) |
35 | #define KEYSTONE_RPROC_UIO_MAP_OFFSET_SHIFT (3) | 47 | #define KEYSTONE_RPROC_UIO_MAP_OFFSET_SHIFT (3) |
@@ -38,8 +50,9 @@ enum keystone_rproc_state { | |||
38 | #define KEYSTONE_RPROC_IOC_MAGIC 'I' | 50 | #define KEYSTONE_RPROC_IOC_MAGIC 'I' |
39 | #define KEYSTONE_RPROC_IOC_SET_RSC_TABLE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ | 51 | #define KEYSTONE_RPROC_IOC_SET_RSC_TABLE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ |
40 | 0, void *) | 52 | 0, void *) |
41 | #define KEYSTONE_RPROC_IOC_SET_STATE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ | 53 | #define KEYSTONE_RPROC_IOC_SET_STATE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ |
42 | 1, enum keystone_rproc_state) | 54 | 1, \ |
55 | struct keystone_rproc_set_state_params) | ||
43 | #define KEYSTONE_RPROC_IOC_SET_LOADED_RSC_TABLE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ | 56 | #define KEYSTONE_RPROC_IOC_SET_LOADED_RSC_TABLE _IOW(KEYSTONE_RPROC_IOC_MAGIC, \ |
44 | 2, uint32_t) | 57 | 2, uint32_t) |
45 | #define KEYSTONE_RPROC_IOC_DSP_RESET _IO(KEYSTONE_RPROC_IOC_MAGIC, 3) | 58 | #define KEYSTONE_RPROC_IOC_DSP_RESET _IO(KEYSTONE_RPROC_IOC_MAGIC, 3) |