AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/radeon: fix up some badness when probe fails
@ 2025-10-18  5:44 Daniel Palmer
  2025-10-18  5:44 ` [PATCH 1/3] drm/radeon: Clean up pdev->dev instances in probe Daniel Palmer
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Daniel Palmer @ 2025-10-18  5:44 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona, wuhoipok
  Cc: amd-gfx, dri-devel, linux-kernel, Daniel Palmer

I have been trying to get a Radeon 9250 running on an Amiga 4000[0].

On that setup it fails to find the BIOS and the probe fails which is
expected but then a bunch of WARN_ON()s etc are triggered.

I though maybe this is "m68k problems" so I bought an old x86 board and
there if I have a different primary VGA card the BIOS part of the
probe fails in the same way and the same scary messages[1] are showing
up in the console.

It seems like the probe failure path wasn't tested when some previous
cleaning up happened.

I'll fix the issues with not finding the BIOS if the card wasn't
initialised in the normal x86 way later.

0 - https://lore.kernel.org/lkml/20251007092313.755856-1-daniel@thingy.jp/
1 - https://gist.github.com/fifteenhex/b971bd62c49383a0558395c62c05ce3b

Daniel Palmer (3):
  drm/radeon: Clean up pdev->dev instances in probe
  drm/radeon: Do not kfree() devres managed rdev
  drm/radeon: Remove calls to drm_put_dev()

 drivers/gpu/drm/radeon/radeon_drv.c | 34 ++++++++---------------------
 drivers/gpu/drm/radeon/radeon_kms.c |  1 -
 2 files changed, 9 insertions(+), 26 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] drm/radeon: Clean up pdev->dev instances in probe
  2025-10-18  5:44 [PATCH 0/3] drm/radeon: fix up some badness when probe fails Daniel Palmer
@ 2025-10-18  5:44 ` Daniel Palmer
  2025-10-18  5:44 ` [PATCH 2/3] drm/radeon: Do not kfree() devres managed rdev Daniel Palmer
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Daniel Palmer @ 2025-10-18  5:44 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona, wuhoipok
  Cc: amd-gfx, dri-devel, linux-kernel, Daniel Palmer

Get a struct device pointer from the start and use it.

Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 88e821d67af7..eb5138da8d5b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -262,6 +262,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 	unsigned long flags = 0;
 	struct drm_device *ddev;
 	struct radeon_device *rdev;
+	struct device *dev = &pdev->dev;
 	const struct drm_format_info *format;
 	int ret;
 
@@ -277,7 +278,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 		case CHIP_VERDE:
 		case CHIP_OLAND:
 		case CHIP_HAINAN:
-			dev_info(&pdev->dev,
+			dev_info(dev,
 				 "SI support disabled by module param\n");
 			return -ENODEV;
 		}
@@ -289,7 +290,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 		case CHIP_HAWAII:
 		case CHIP_KABINI:
 		case CHIP_MULLINS:
-			dev_info(&pdev->dev,
+			dev_info(dev,
 				 "CIK support disabled by module param\n");
 			return -ENODEV;
 		}
@@ -303,11 +304,11 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	rdev = devm_drm_dev_alloc(&pdev->dev, &kms_driver, typeof(*rdev), ddev);
+	rdev = devm_drm_dev_alloc(dev, &kms_driver, typeof(*rdev), ddev);
 	if (IS_ERR(rdev))
 		return PTR_ERR(rdev);
 
-	rdev->dev = &pdev->dev;
+	rdev->dev = dev;
 	rdev->pdev = pdev;
 	ddev = rdev_to_drm(rdev);
 	ddev->dev_private = rdev;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] drm/radeon: Do not kfree() devres managed rdev
  2025-10-18  5:44 [PATCH 0/3] drm/radeon: fix up some badness when probe fails Daniel Palmer
  2025-10-18  5:44 ` [PATCH 1/3] drm/radeon: Clean up pdev->dev instances in probe Daniel Palmer
@ 2025-10-18  5:44 ` Daniel Palmer
  2025-10-18  5:44 ` [PATCH 3/3] drm/radeon: Remove calls to drm_put_dev() Daniel Palmer
  2025-10-20 17:32 ` [PATCH 0/3] drm/radeon: fix up some badness when probe fails Alex Deucher
  3 siblings, 0 replies; 5+ messages in thread
From: Daniel Palmer @ 2025-10-18  5:44 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona, wuhoipok
  Cc: amd-gfx, dri-devel, linux-kernel, Daniel Palmer

Since the allocation of the drivers main structure was changed to
devm_drm_dev_alloc() rdev is managed by devres and we shouldn't be calling
kfree() on it.

This fixes things exploding if the driver probe fails and devres cleans up
the rdev after we already free'd it.

Fixes: a9ed2f052c5c ("drm/radeon: change drm_dev_alloc to devm_drm_dev_alloc")
Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 645e33bf7947..ba1446acd703 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -84,7 +84,6 @@ void radeon_driver_unload_kms(struct drm_device *dev)
 	rdev->agp = NULL;
 
 done_free:
-	kfree(rdev);
 	dev->dev_private = NULL;
 }
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] drm/radeon: Remove calls to drm_put_dev()
  2025-10-18  5:44 [PATCH 0/3] drm/radeon: fix up some badness when probe fails Daniel Palmer
  2025-10-18  5:44 ` [PATCH 1/3] drm/radeon: Clean up pdev->dev instances in probe Daniel Palmer
  2025-10-18  5:44 ` [PATCH 2/3] drm/radeon: Do not kfree() devres managed rdev Daniel Palmer
@ 2025-10-18  5:44 ` Daniel Palmer
  2025-10-20 17:32 ` [PATCH 0/3] drm/radeon: fix up some badness when probe fails Alex Deucher
  3 siblings, 0 replies; 5+ messages in thread
From: Daniel Palmer @ 2025-10-18  5:44 UTC (permalink / raw)
  To: alexander.deucher, christian.koenig, airlied, simona, wuhoipok
  Cc: amd-gfx, dri-devel, linux-kernel, Daniel Palmer

Since the allocation of the drivers main structure was changed to
devm_drm_dev_alloc() drm_put_dev()'ing to trigger it to be free'd
should be done by devres.

However, drm_put_dev() is still in the probe error and device remove
paths. When the driver fails to probe warnings like the following are
shown because devres is trying to drm_put_dev() after the driver
already did it.

[    5.642230] radeon 0000:01:05.0: probe with driver radeon failed with error -22
[    5.649605] ------------[ cut here ]------------
[    5.649607] refcount_t: underflow; use-after-free.
[    5.649620] WARNING: CPU: 0 PID: 357 at lib/refcount.c:28 refcount_warn_saturate+0xbe/0x110

Fixes: a9ed2f052c5c ("drm/radeon: change drm_dev_alloc to devm_drm_dev_alloc")
Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index eb5138da8d5b..350f88af888d 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -315,17 +315,17 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 
 	ret = pci_enable_device(pdev);
 	if (ret)
-		goto err_free;
+		return ret;
 
 	pci_set_drvdata(pdev, ddev);
 
 	ret = radeon_driver_load_kms(ddev, flags);
 	if (ret)
-		goto err_agp;
+		goto err;
 
 	ret = drm_dev_register(ddev, flags);
 	if (ret)
-		goto err_agp;
+		goto err;
 
 	if (rdev->mc.real_vram_size <= (8 * 1024 * 1024))
 		format = drm_format_info(DRM_FORMAT_C8);
@@ -338,30 +338,14 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 
 	return 0;
 
-err_agp:
+err:
 	pci_disable_device(pdev);
-err_free:
-	drm_dev_put(ddev);
 	return ret;
 }
 
-static void
-radeon_pci_remove(struct pci_dev *pdev)
-{
-	struct drm_device *dev = pci_get_drvdata(pdev);
-
-	drm_put_dev(dev);
-}
-
 static void
 radeon_pci_shutdown(struct pci_dev *pdev)
 {
-	/* if we are running in a VM, make sure the device
-	 * torn down properly on reboot/shutdown
-	 */
-	if (radeon_device_is_virtual())
-		radeon_pci_remove(pdev);
-
 #if defined(CONFIG_PPC64) || defined(CONFIG_MACH_LOONGSON64)
 	/*
 	 * Some adapters need to be suspended before a
@@ -614,7 +598,6 @@ static struct pci_driver radeon_kms_pci_driver = {
 	.name = DRIVER_NAME,
 	.id_table = pciidlist,
 	.probe = radeon_pci_probe,
-	.remove = radeon_pci_remove,
 	.shutdown = radeon_pci_shutdown,
 	.driver.pm = &radeon_pm_ops,
 };
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] drm/radeon: fix up some badness when probe fails
  2025-10-18  5:44 [PATCH 0/3] drm/radeon: fix up some badness when probe fails Daniel Palmer
                   ` (2 preceding siblings ...)
  2025-10-18  5:44 ` [PATCH 3/3] drm/radeon: Remove calls to drm_put_dev() Daniel Palmer
@ 2025-10-20 17:32 ` Alex Deucher
  3 siblings, 0 replies; 5+ messages in thread
From: Alex Deucher @ 2025-10-20 17:32 UTC (permalink / raw)
  To: Daniel Palmer
  Cc: alexander.deucher, christian.koenig, airlied, simona, wuhoipok,
	amd-gfx, dri-devel, linux-kernel

Applied.  Thanks!

Alex

On Sat, Oct 18, 2025 at 1:51 AM Daniel Palmer <daniel@0x0f.com> wrote:
>
> I have been trying to get a Radeon 9250 running on an Amiga 4000[0].
>
> On that setup it fails to find the BIOS and the probe fails which is
> expected but then a bunch of WARN_ON()s etc are triggered.
>
> I though maybe this is "m68k problems" so I bought an old x86 board and
> there if I have a different primary VGA card the BIOS part of the
> probe fails in the same way and the same scary messages[1] are showing
> up in the console.
>
> It seems like the probe failure path wasn't tested when some previous
> cleaning up happened.
>
> I'll fix the issues with not finding the BIOS if the card wasn't
> initialised in the normal x86 way later.
>
> 0 - https://lore.kernel.org/lkml/20251007092313.755856-1-daniel@thingy.jp/
> 1 - https://gist.github.com/fifteenhex/b971bd62c49383a0558395c62c05ce3b
>
> Daniel Palmer (3):
>   drm/radeon: Clean up pdev->dev instances in probe
>   drm/radeon: Do not kfree() devres managed rdev
>   drm/radeon: Remove calls to drm_put_dev()
>
>  drivers/gpu/drm/radeon/radeon_drv.c | 34 ++++++++---------------------
>  drivers/gpu/drm/radeon/radeon_kms.c |  1 -
>  2 files changed, 9 insertions(+), 26 deletions(-)
>
> --
> 2.51.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-10-20 17:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-18  5:44 [PATCH 0/3] drm/radeon: fix up some badness when probe fails Daniel Palmer
2025-10-18  5:44 ` [PATCH 1/3] drm/radeon: Clean up pdev->dev instances in probe Daniel Palmer
2025-10-18  5:44 ` [PATCH 2/3] drm/radeon: Do not kfree() devres managed rdev Daniel Palmer
2025-10-18  5:44 ` [PATCH 3/3] drm/radeon: Remove calls to drm_put_dev() Daniel Palmer
2025-10-20 17:32 ` [PATCH 0/3] drm/radeon: fix up some badness when probe fails Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox