Re: [PATCH 1/2] drm/radeon: Check the VBIOS signature after copying it from VRAM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] radeon: inefficient 256KB MMIO copy and allocation before VBIOS signature check
--

commit e7cbdfb764010827fd7b2b511a029cf30846ddbe
Author: Imre Kaloz <kaloz@kernel.org>

drm/radeon: Check the VBIOS signature after copying it from VRAM

This commit modifies the VBIOS reading logic to check the signature after
copying the data from VRAM. This is done to avoid a sparc64 fault caused
by a plain load through an ioremap() cookie.

> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 8595f4c6e2e5c..a8367e4811821 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -63,10 +63,6 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>  		return false;
>  	}
>  
> -	if (size == 0 || bios[0] != 0x55 || bios[1] != 0xaa) {
> -		iounmap(bios);
> -		return false;
> -	}
>  	rdev->bios = kmalloc(size, GFP_KERNEL);
>  	if (rdev->bios == NULL) {
>  		iounmap(bios);
> @@ -74,6 +70,12 @@ static bool igp_read_bios_from_vram(struct radeon_device *rdev)
>  	}
>  	memcpy_fromio(rdev->bios, bios, size);
>  	iounmap(bios);
> +
> +	if (size == 0 || rdev->bios[0] != 0x55 || rdev->bios[1] != 0xaa) {

[Severity: Medium]
Does this sequence perform an expensive unconditional 256KB kmalloc() and
PCIe MMIO read (memcpy_fromio()) for discrete GPUs before validating the
VBIOS header signature?

During device probe, radeon_get_bios() tries multiple fallback methods and
calls igp_read_bios_from_vram(). This allows posted discrete GPUs to proceed.
The new logic maps VRAM, immediately allocates 256KB, copies it, and only
then checks if the first two bytes match the expected 0x55 and 0xaa
signature. This wastes allocation overhead and introduces a noticeable boot
delay, as reading 256KB of un-cached MMIO over PCIe takes significant time.

To fix the sparc64 __iomem access fault without creating a performance
regression, could this code use readb(&bios[0]) for the signature validation
before performing the 256KB allocation and copy, matching the pattern
established in radeon_read_bios()?

> +		kfree(rdev->bios);
> +		rdev->bios = NULL;
> +		return false;
> +	}
>  	return true;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927105555.8771-1-kaloz@kernel.org?part=1