| Message ID | 20190523171653.138678-1-sean@poorly.run (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v2,1/6] drm/msm/a6xx: Avoid freeing gmu resources multiple times | expand |
On Thu, May 23, 2019 at 01:16:40PM -0400, Sean Paul wrote: > From: Sean Paul <seanpaul@chromium.org> > > The driver checks for gmu->mmio as a sign that the device has been > initialized, however there are failures in probe below the mmio init. > If one of those is hit, mmio will be non-null but freed. > > In that case, a6xx_gmu_probe will return an error to a6xx_gpu_init which > will in turn call a6xx_gmu_remove which checks gmu->mmio and tries to free > resources for a second time. This causes a great boom. > > Fix this by adding an initialized member to gmu which is set on > successful probe and cleared on removal. > > Changes in v2: > - None > > Cc: Jordan Crouse <jcrouse@codeaurora.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Jordan Crouse <jcrouse@codeaurora.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 14 +++++++++----- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 + > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index 38e2cfa9cec7..aa84edb25d91 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -74,7 +74,7 @@ bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu) > u32 val; > > /* This can be called from gpu state code so make sure GMU is valid */ > - if (IS_ERR_OR_NULL(gmu->mmio)) > + if (!gmu->initialized) > return false; > > val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); > @@ -90,7 +90,7 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) > u32 val; > > /* This can be called from gpu state code so make sure GMU is valid */ > - if (IS_ERR_OR_NULL(gmu->mmio)) > + if (!gmu->initialized) > return false; > > val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); > @@ -695,7 +695,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > int status, ret; > > - if (WARN(!gmu->mmio, "The GMU is not set up yet\n")) > + if (WARN(!gmu->initialized, "The GMU is not set up yet\n")) > return 0; > > gmu->hung = false; > @@ -765,7 +765,7 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) > { > u32 reg; > > - if (!gmu->mmio) > + if (!gmu->initialized) > return true; > > reg = gmu_read(gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_STATUS); > @@ -1227,7 +1227,7 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) > { > struct a6xx_gmu *gmu = &a6xx_gpu->gmu; > > - if (IS_ERR_OR_NULL(gmu->mmio)) > + if (!gmu->initialized) > return; > > a6xx_gmu_stop(a6xx_gpu); > @@ -1245,6 +1245,8 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) > iommu_detach_device(gmu->domain, gmu->dev); > > iommu_domain_free(gmu->domain); > + > + gmu->initialized = false; > } > > int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) > @@ -1309,6 +1311,8 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) > /* Set up the HFI queues */ > a6xx_hfi_init(gmu); > > + gmu->initialized = true; > + > return 0; > err: > a6xx_gmu_memory_free(gmu, gmu->hfi); > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > index bedd8e6a63aa..39a26dd63674 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > @@ -75,6 +75,7 @@ struct a6xx_gmu { > > struct a6xx_hfi_queue queues[2]; > > + bool initialized; > bool hung; > }; > > -- > Sean Paul, Software Engineer, Google / Chromium OS >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 38e2cfa9cec7..aa84edb25d91 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -74,7 +74,7 @@ bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu) u32 val; /* This can be called from gpu state code so make sure GMU is valid */ - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return false; val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); @@ -90,7 +90,7 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu) u32 val; /* This can be called from gpu state code so make sure GMU is valid */ - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return false; val = gmu_read(gmu, REG_A6XX_GMU_SPTPRAC_PWR_CLK_STATUS); @@ -695,7 +695,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) struct a6xx_gmu *gmu = &a6xx_gpu->gmu; int status, ret; - if (WARN(!gmu->mmio, "The GMU is not set up yet\n")) + if (WARN(!gmu->initialized, "The GMU is not set up yet\n")) return 0; gmu->hung = false; @@ -765,7 +765,7 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu) { u32 reg; - if (!gmu->mmio) + if (!gmu->initialized) return true; reg = gmu_read(gmu, REG_A6XX_GPU_GMU_AO_GPU_CX_BUSY_STATUS); @@ -1227,7 +1227,7 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) { struct a6xx_gmu *gmu = &a6xx_gpu->gmu; - if (IS_ERR_OR_NULL(gmu->mmio)) + if (!gmu->initialized) return; a6xx_gmu_stop(a6xx_gpu); @@ -1245,6 +1245,8 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) iommu_detach_device(gmu->domain, gmu->dev); iommu_domain_free(gmu->domain); + + gmu->initialized = false; } int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) @@ -1309,6 +1311,8 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) /* Set up the HFI queues */ a6xx_hfi_init(gmu); + gmu->initialized = true; + return 0; err: a6xx_gmu_memory_free(gmu, gmu->hfi); diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h index bedd8e6a63aa..39a26dd63674 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h @@ -75,6 +75,7 @@ struct a6xx_gmu { struct a6xx_hfi_queue queues[2]; + bool initialized; bool hung; };