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 <[email protected]> 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/[email protected]?part=1
