* [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd
@ 2015-06-03 10:36 Marek Szyprowski
2015-06-03 10:36 ` [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration Marek Szyprowski
2015-06-18 13:24 ` [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Kamil Debski
0 siblings, 2 replies; 4+ messages in thread
From: Marek Szyprowski @ 2015-06-03 10:36 UTC (permalink / raw)
To: linux-media; +Cc: Marek Szyprowski, Sylwester Nawrocki
alloc_dev_context_buffer method might fail, so add proper return value
check.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
index f176096..b1b1491 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
@@ -37,8 +37,12 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
{
struct s5p_mfc_cmd_args h2r_args;
struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
+ int ret;
+
+ ret = s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
+ if (ret)
+ return ret;
- s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
--
1.9.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration
2015-06-03 10:36 [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Marek Szyprowski
@ 2015-06-03 10:36 ` Marek Szyprowski
2015-06-18 13:24 ` Kamil Debski
2015-06-18 13:24 ` [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Kamil Debski
1 sibling, 1 reply; 4+ messages in thread
From: Marek Szyprowski @ 2015-06-03 10:36 UTC (permalink / raw)
To: linux-media; +Cc: Marek Szyprowski, Sylwester Nawrocki
MFC hardware is known to trash random memory if one tries to use a
buffer buffer, which has lower DMA addresses than the configured DMA
base address. This patch adds a check for this case and proper error
handling.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/platform/s5p-mfc/s5p_mfc_opr.c | 11 +++++++++--
drivers/media/platform/s5p-mfc/s5p_mfc_opr.h | 2 +-
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 12 +++++++-----
drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 8 +++++---
4 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
index 00a1d8b..8d27f88 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
@@ -37,10 +37,9 @@ void s5p_mfc_init_regs(struct s5p_mfc_dev *dev)
dev->mfc_regs = s5p_mfc_init_regs_v6_plus(dev);
}
-int s5p_mfc_alloc_priv_buf(struct device *dev,
+int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
struct s5p_mfc_priv_buf *b)
{
-
mfc_debug(3, "Allocating priv: %zu\n", b->size);
b->virt = dma_alloc_coherent(dev, b->size, &b->dma, GFP_KERNEL);
@@ -50,6 +49,14 @@ int s5p_mfc_alloc_priv_buf(struct device *dev,
return -ENOMEM;
}
+ if (b->dma < base) {
+ mfc_err("Invaling memory configuration!\n");
+ mfc_err("Allocated buffer (%pad) is lower than memory base addres (%pad)\n",
+ &b->dma, &base);
+ dma_free_coherent(dev, b->size, b->virt, b->dma);
+ return -ENOMEM;
+ }
+
mfc_debug(3, "Allocated addr %p %pad\n", b->virt, &b->dma);
return 0;
}
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
index 22dfb3e..77a08b1 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
@@ -334,7 +334,7 @@ struct s5p_mfc_hw_ops {
void s5p_mfc_init_hw_ops(struct s5p_mfc_dev *dev);
void s5p_mfc_init_regs(struct s5p_mfc_dev *dev);
-int s5p_mfc_alloc_priv_buf(struct device *dev,
+int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
struct s5p_mfc_priv_buf *b);
void s5p_mfc_release_priv_buf(struct device *dev,
struct s5p_mfc_priv_buf *b);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index c7adc3d..b3f6700 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -41,7 +41,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
int ret;
ctx->dsc.size = buf_size->dsc;
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->dsc);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->dsc);
if (ret) {
mfc_err("Failed to allocate temporary buffer\n");
return ret;
@@ -172,7 +172,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
/* Allocate only if memory from bank 1 is necessary */
if (ctx->bank1.size > 0) {
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->bank1);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
+ &ctx->bank1);
if (ret) {
mfc_err("Failed to allocate Bank1 temporary buffer\n");
return ret;
@@ -181,7 +182,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
}
/* Allocate only if memory from bank 2 is necessary */
if (ctx->bank2.size > 0) {
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, &ctx->bank2);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, dev->bank2,
+ &ctx->bank2);
if (ret) {
mfc_err("Failed to allocate Bank2 temporary buffer\n");
s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
@@ -212,7 +214,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
else
ctx->ctx.size = buf_size->non_h264_ctx;
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->ctx);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
if (ret) {
mfc_err("Failed to allocate instance buffer\n");
return ret;
@@ -225,7 +227,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
/* Initialize shared memory */
ctx->shm.size = buf_size->shm;
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->shm);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->shm);
if (ret) {
mfc_err("Failed to allocate shared memory buffer\n");
s5p_mfc_release_priv_buf(dev->mem_dev_l, &ctx->ctx);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index cefad18..ed6b14c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -239,7 +239,8 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
/* Allocate only if memory from bank 1 is necessary */
if (ctx->bank1.size > 0) {
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->bank1);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
+ &ctx->bank1);
if (ret) {
mfc_err("Failed to allocate Bank1 memory\n");
return ret;
@@ -291,7 +292,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
break;
}
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->ctx);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
if (ret) {
mfc_err("Failed to allocate instance buffer\n");
return ret;
@@ -320,7 +321,8 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
mfc_debug_enter();
dev->ctx_buf.size = buf_size->dev_ctx;
- ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &dev->ctx_buf);
+ ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
+ &dev->ctx_buf);
if (ret) {
mfc_err("Failed to allocate device context buffer\n");
return ret;
--
1.9.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration
2015-06-03 10:36 ` [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration Marek Szyprowski
@ 2015-06-18 13:24 ` Kamil Debski
0 siblings, 0 replies; 4+ messages in thread
From: Kamil Debski @ 2015-06-18 13:24 UTC (permalink / raw)
To: Marek Szyprowski; +Cc: linux-media, Sylwester Nawrocki
On 3 June 2015 at 11:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> MFC hardware is known to trash random memory if one tries to use a
> buffer buffer, which has lower DMA addresses than the configured DMA
> base address. This patch adds a check for this case and proper error
> handling.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kamil Debski <kamil@wypas.org>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_opr.c | 11 +++++++++--
> drivers/media/platform/s5p-mfc/s5p_mfc_opr.h | 2 +-
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 12 +++++++-----
> drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 8 +++++---
> 4 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> index 00a1d8b..8d27f88 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
> @@ -37,10 +37,9 @@ void s5p_mfc_init_regs(struct s5p_mfc_dev *dev)
> dev->mfc_regs = s5p_mfc_init_regs_v6_plus(dev);
> }
>
> -int s5p_mfc_alloc_priv_buf(struct device *dev,
> +int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
> struct s5p_mfc_priv_buf *b)
> {
> -
> mfc_debug(3, "Allocating priv: %zu\n", b->size);
>
> b->virt = dma_alloc_coherent(dev, b->size, &b->dma, GFP_KERNEL);
> @@ -50,6 +49,14 @@ int s5p_mfc_alloc_priv_buf(struct device *dev,
> return -ENOMEM;
> }
>
> + if (b->dma < base) {
> + mfc_err("Invaling memory configuration!\n");
> + mfc_err("Allocated buffer (%pad) is lower than memory base addres (%pad)\n",
> + &b->dma, &base);
> + dma_free_coherent(dev, b->size, b->virt, b->dma);
> + return -ENOMEM;
> + }
> +
> mfc_debug(3, "Allocated addr %p %pad\n", b->virt, &b->dma);
> return 0;
> }
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> index 22dfb3e..77a08b1 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.h
> @@ -334,7 +334,7 @@ struct s5p_mfc_hw_ops {
>
> void s5p_mfc_init_hw_ops(struct s5p_mfc_dev *dev);
> void s5p_mfc_init_regs(struct s5p_mfc_dev *dev);
> -int s5p_mfc_alloc_priv_buf(struct device *dev,
> +int s5p_mfc_alloc_priv_buf(struct device *dev, dma_addr_t base,
> struct s5p_mfc_priv_buf *b);
> void s5p_mfc_release_priv_buf(struct device *dev,
> struct s5p_mfc_priv_buf *b);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> index c7adc3d..b3f6700 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
> @@ -41,7 +41,7 @@ static int s5p_mfc_alloc_dec_temp_buffers_v5(struct s5p_mfc_ctx *ctx)
> int ret;
>
> ctx->dsc.size = buf_size->dsc;
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->dsc);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->dsc);
> if (ret) {
> mfc_err("Failed to allocate temporary buffer\n");
> return ret;
> @@ -172,7 +172,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
> /* Allocate only if memory from bank 1 is necessary */
> if (ctx->bank1.size > 0) {
>
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->bank1);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> + &ctx->bank1);
> if (ret) {
> mfc_err("Failed to allocate Bank1 temporary buffer\n");
> return ret;
> @@ -181,7 +182,8 @@ static int s5p_mfc_alloc_codec_buffers_v5(struct s5p_mfc_ctx *ctx)
> }
> /* Allocate only if memory from bank 2 is necessary */
> if (ctx->bank2.size > 0) {
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, &ctx->bank2);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_r, dev->bank2,
> + &ctx->bank2);
> if (ret) {
> mfc_err("Failed to allocate Bank2 temporary buffer\n");
> s5p_mfc_release_priv_buf(ctx->dev->mem_dev_l, &ctx->bank1);
> @@ -212,7 +214,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
> else
> ctx->ctx.size = buf_size->non_h264_ctx;
>
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->ctx);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
> if (ret) {
> mfc_err("Failed to allocate instance buffer\n");
> return ret;
> @@ -225,7 +227,7 @@ static int s5p_mfc_alloc_instance_buffer_v5(struct s5p_mfc_ctx *ctx)
>
> /* Initialize shared memory */
> ctx->shm.size = buf_size->shm;
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->shm);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->shm);
> if (ret) {
> mfc_err("Failed to allocate shared memory buffer\n");
> s5p_mfc_release_priv_buf(dev->mem_dev_l, &ctx->ctx);
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index cefad18..ed6b14c 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -239,7 +239,8 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
>
> /* Allocate only if memory from bank 1 is necessary */
> if (ctx->bank1.size > 0) {
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->bank1);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> + &ctx->bank1);
> if (ret) {
> mfc_err("Failed to allocate Bank1 memory\n");
> return ret;
> @@ -291,7 +292,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
> break;
> }
>
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &ctx->ctx);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1, &ctx->ctx);
> if (ret) {
> mfc_err("Failed to allocate instance buffer\n");
> return ret;
> @@ -320,7 +321,8 @@ static int s5p_mfc_alloc_dev_context_buffer_v6(struct s5p_mfc_dev *dev)
> mfc_debug_enter();
>
> dev->ctx_buf.size = buf_size->dev_ctx;
> - ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, &dev->ctx_buf);
> + ret = s5p_mfc_alloc_priv_buf(dev->mem_dev_l, dev->bank1,
> + &dev->ctx_buf);
> if (ret) {
> mfc_err("Failed to allocate device context buffer\n");
> return ret;
> --
> 1.9.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd
2015-06-03 10:36 [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Marek Szyprowski
2015-06-03 10:36 ` [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration Marek Szyprowski
@ 2015-06-18 13:24 ` Kamil Debski
1 sibling, 0 replies; 4+ messages in thread
From: Kamil Debski @ 2015-06-18 13:24 UTC (permalink / raw)
To: Marek Szyprowski; +Cc: linux-media, Sylwester Nawrocki
On 3 June 2015 at 11:36, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> alloc_dev_context_buffer method might fail, so add proper return value
> check.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Kamil Debski <kamil@wypas.org>
> ---
> drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> index f176096..b1b1491 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
> @@ -37,8 +37,12 @@ static int s5p_mfc_sys_init_cmd_v6(struct s5p_mfc_dev *dev)
> {
> struct s5p_mfc_cmd_args h2r_args;
> struct s5p_mfc_buf_size_v6 *buf_size = dev->variant->buf_size->priv;
> + int ret;
> +
> + ret = s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
> + if (ret)
> + return ret;
>
> - s5p_mfc_hw_call(dev->mfc_ops, alloc_dev_context_buffer, dev);
> mfc_write(dev, dev->ctx_buf.dma, S5P_FIMV_CONTEXT_MEM_ADDR_V6);
> mfc_write(dev, buf_size->dev_ctx, S5P_FIMV_CONTEXT_MEM_SIZE_V6);
> return s5p_mfc_cmd_host2risc_v6(dev, S5P_FIMV_H2R_CMD_SYS_INIT_V6,
> --
> 1.9.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-06-18 13:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-03 10:36 [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Marek Szyprowski
2015-06-03 10:36 ` [PATCH 2/2] media: s5p-mfc: add additional check for incorrect memory configuration Marek Szyprowski
2015-06-18 13:24 ` Kamil Debski
2015-06-18 13:24 ` [PATCH 1/2] media: s5p-mfc: add return value check in mfc_sys_init_cmd Kamil Debski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.