| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200218230627.GA8715@jcrouse1-lnx.qualcomm.com>
Date: Tue, 18 Feb 2020 16:06:27 -0700
From: Jordan Crouse <jcrouse@...eaurora.org>
To: Rob Clark <robdclark@...il.com>
Cc: dri-devel@...ts.freedesktop.org,
Rob Clark <robdclark@...omium.org>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<freedreno@...ts.freedesktop.org>, David Airlie <airlied@...ux.ie>,
"open list:DRM DRIVER FOR MSM ADRENO GPU"
<linux-arm-msm@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
Daniel Vetter <daniel@...ll.ch>, Sean Paul <sean@...rly.run>
Subject: Re: [Freedreno] [PATCH] drm/msm: devcoredump should dump
MSM_SUBMIT_BO_DUMP buffers
On Tue, Feb 18, 2020 at 01:00:21PM -0800, Rob Clark wrote:
> From: Rob Clark <robdclark@...omium.org>
>
> Also log buffers with the DUMP flag set, to ensure we capture all useful
> cmdstream in crashdump state with modern mesa.
>
> Otherwise we miss out on the contents of "state object" cmdstream
> buffers.
One nit, but with that:
Reviewed-by: Jordan Crouse <jcrouse@...eaurora.org>
> Signed-off-by: Rob Clark <robdclark@...omium.org>
> ---
> drivers/gpu/drm/msm/msm_gem.h | 10 ++++++++++
> drivers/gpu/drm/msm/msm_gpu.c | 28 +++++++++++++++++++++++-----
> drivers/gpu/drm/msm/msm_rd.c | 8 +-------
> 3 files changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 9e0953c2b7ce..22b4ccd7bb28 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -160,4 +160,14 @@ struct msm_gem_submit {
> } bos[0];
> };
>
> +/* helper to determine of a buffer in submit should be dumped, used for both
> + * devcoredump and debugfs cmdstream dumping:
> + */
> +static bool
Static inline? Surprised you didn't get an unused warning or two.
> +should_dump(struct msm_gem_submit *submit, int idx)
> +{
> + extern bool rd_full;
> + return rd_full || (submit->bos[idx].flags & MSM_SUBMIT_BO_DUMP);
> +}
> +
> #endif /* __MSM_GEM_H__ */
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 18f3a5c53ffb..615c5cda5389 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -355,16 +355,34 @@ static void msm_gpu_crashstate_capture(struct msm_gpu *gpu,
> state->cmd = kstrdup(cmd, GFP_KERNEL);
>
> if (submit) {
> - int i;
> -
> - state->bos = kcalloc(submit->nr_cmds,
> + int i, nr = 0;
> +
> + /* count # of buffers to dump: */
> + for (i = 0; i < submit->nr_bos; i++)
> + if (should_dump(submit, i))
> + nr++;
> + /* always dump cmd bo's, but don't double count them: */
> + for (i = 0; i < submit->nr_cmds; i++)
> + if (!should_dump(submit, submit->cmd[i].idx))
> + nr++;
> +
> + state->bos = kcalloc(nr,
> sizeof(struct msm_gpu_state_bo), GFP_KERNEL);
>
> + for (i = 0; i < submit->nr_bos; i++) {
> + if (should_dump(submit, i)) {
> + msm_gpu_crashstate_get_bo(state, submit->bos[i].obj,
> + submit->bos[i].iova, submit->bos[i].flags);
> + }
> + }
> +
> for (i = 0; state->bos && i < submit->nr_cmds; i++) {
> int idx = submit->cmd[i].idx;
>
> - msm_gpu_crashstate_get_bo(state, submit->bos[idx].obj,
> - submit->bos[idx].iova, submit->bos[idx].flags);
> + if (!should_dump(submit, submit->cmd[i].idx)) {
> + msm_gpu_crashstate_get_bo(state, submit->bos[idx].obj,
> + submit->bos[idx].iova, submit->bos[idx].flags);
> + }
> }
> }
>
> diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
> index af7ceb246c7c..732f65df5c4f 100644
> --- a/drivers/gpu/drm/msm/msm_rd.c
> +++ b/drivers/gpu/drm/msm/msm_rd.c
> @@ -43,7 +43,7 @@
> #include "msm_gpu.h"
> #include "msm_gem.h"
>
> -static bool rd_full = false;
> +bool rd_full = false;
> MODULE_PARM_DESC(rd_full, "If true, $debugfs/.../rd will snapshot all buffer contents");
> module_param_named(rd_full, rd_full, bool, 0600);
>
> @@ -336,12 +336,6 @@ static void snapshot_buf(struct msm_rd_state *rd,
> msm_gem_put_vaddr(&obj->base);
> }
>
> -static bool
> -should_dump(struct msm_gem_submit *submit, int idx)
> -{
> - return rd_full || (submit->bos[idx].flags & MSM_SUBMIT_BO_DUMP);
> -}
> -
> /* called under struct_mutex */
> void msm_rd_dump_submit(struct msm_rd_state *rd, struct msm_gem_submit *submit,
> const char *fmt, ...)
> --
> 2.24.1
>
> _______________________________________________
> Freedreno mailing list
> Freedreno@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Powered by blists - more mailing lists