On Wed, Mar 18, 2020 at 10:16:23PM -0400, Mikel Rychliski wrote:
> On some EFI systems, the video BIOS is provided by the EFI firmware. The
> boot stub code stores the physical address of the ROM image in pdev->rom.
> Currently we attempt to access this pointer using phys_to_virt(), which
> doesn't work with CONFIG_HIGHMEM.
>
> On these systems, attempting to load the radeon module on a x86_32 kernel
> can result in the following:
>
> BUG: unable to handle page fault for address: 3e8ed03c
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x) - not-present page
> *pde =
> Oops: [#1] PREEMPT SMP
> CPU: 0 PID: 317 Comm: systemd-udevd Not tainted 5.6.0-rc3-next-20200228 #2
> Hardware name: Apple Computer, Inc. MacPro1,1/Mac-F4208DC8, BIOS
> MP11.88Z.005C.B08.0707021221 07/02/07
> EIP: radeon_get_bios+0x5ed/0xe50 [radeon]
> Code: 00 00 84 c0 0f 85 12 fd ff ff c7 87 64 01 00 00 00 00 00 00 8b 47
> 08 8b 55 b0 e8 1e 83 e1 d6 85 c0 74 1a 8b 55 c0 85 d2 74 13 <80> 38 55 75 0e
> 80 78 01 aa 0f 84 a4 03 00 00 8d 74 26 00 68 dc 06
> EAX: 3e8ed03c EBX: ECX: 3e8ed03c EDX: 0001
> ESI: 0004 EDI: eec04000 EBP: eef3fc60 ESP: eef3fbe0
> DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 EFLAGS: 00010206
> CR0: 80050033 CR2: 3e8ed03c CR3: 2ec77000 CR4: 06d0
> Call Trace:
> ? register_client+0x34/0xe0
> ? register_client+0xab/0xe0
> r520_init+0x26/0x240 [radeon]
> radeon_device_init+0x533/0xa50 [radeon]
> radeon_driver_load_kms+0x80/0x220 [radeon]
> drm_dev_register+0xa7/0x180 [drm]
> radeon_pci_probe+0x10f/0x1a0 [radeon]
> pci_device_probe+0xd4/0x140
> really_probe+0x13d/0x3b0
> driver_probe_device+0x56/0xd0
> device_driver_attach+0x49/0x50
> __driver_attach+0x79/0x130
> ? device_driver_attach+0x50/0x50
> bus_for_each_dev+0x5b/0xa0
> driver_attach+0x19/0x20
> ? device_driver_attach+0x50/0x50
> bus_add_driver+0x117/0x1d0
> ? pci_bus_num_vf+0x20/0x20
> driver_register+0x66/0xb0
> ? 0xf80f4000
> __pci_register_driver+0x3d/0x40
> radeon_init+0x82/0x1000 [radeon]
> do_one_initcall+0x42/0x200
> ? kvfree+0x25/0x30
> ? __vunmap+0x206/0x230
> ? kmem_cache_alloc_trace+0x16f/0x220
> ? do_init_module+0x21/0x220
> do_init_module+0x50/0x220
> load_module+0x1f26/0x2200
> sys_init_module+0x12d/0x160
> do_fast_syscall_32+0x82/0x250
> entry_SYSENTER_32+0xa5/0xf8
>
> Fix the issue by updating all drivers which can access a platform
> provided ROM. Instead of calling the helper function pci_platform_rom()
> which uses phys_to_virt(), call ioremap() directly on the pdev->rom.
>
> radeon_read_platform_bios() previously directly accessed an __iomem
> pointer. Avoid this by calling memcpy_fromio() instead of kmemdup().
>
> pci_platform_rom() now has no remaining callers, so remove it.
>
> Signed-off-by: Mikel Rychliski
I applied this to pci/resource for v5.7. I would feel better if some
of the graphics guys chimed in, or even applied it via the DRM tree
since most of the changes are actually in drivers/gpu.
Feel free to add my
Acked-by: Bjorn Helgaas
and let me know if you do that.
> ---
>
> Tested on a MacPro 1,1 with a Radeon X1900 XT card and 32-bit kernel.
>
> Changes in v3:
> - Inline pci_platform_rom()
>
> Changes in v2:
> - Add iounmap() call in nouveau
> - Update function comment for pci_platform_rom()
> - Minor changes to commit messages
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 31
> +-
> .../gpu/drm/nouveau/nvkm/subdev/bios/shadowpci.c | 17 ++--
> drivers/gpu/drm/radeon/radeon_bios.c | 30 +
> drivers/pci/rom.c | 17
> include/linux/pci.h| 1 -
> 5 files changed, 52 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index 50dff69a0f6e..b1172d93c99c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -192,30 +192,35 @@ static bool amdgpu_read_bios_from_rom(struct
> amdgpu_device *adev)
>
> static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
> {
> - uint8_t __iomem *bios;
> - size_t size;
> + phys_addr_t rom = adev->pdev->rom;
> + size_t romlen = adev->pdev->romlen;
> + void __iomem *bios;
>
> adev->bios = NULL;
>
> - bios = pci_platform_rom(adev->pdev, );
> - if (!bios) {
> + if (!rom || romlen == 0)
> return false;
> - }
>
> - adev->bios = kzalloc(size, GFP_KERNEL);
> - if (adev->bios == NULL)
> + adev->bios = kzalloc(romlen, GFP_KERNEL);
> + if (!adev->bios)
> return false;
>
> - memcpy_fromio(adev->bios, bios, size);
> + bios =
