On Fri, Nov 08, 2019 at 01:13:53PM -0800, Lucas De Marchi wrote:
> When we map the VBT through pci_map_rom() we may not be allowed
> to simply discard the address space and go on reading the memory.
> That doesn't work on my test system, but by dumping the rom via
> sysfs I can can get the correct vbt. So change our find_vbt() to do
> the same as done by pci_read_rom(), i.e. use memcpy_fromio().
>
> v2: the just the minimal changes by not bothering with the unaligned io
> reads: this can be done on top (from Ville and Jani)
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 51 +++++++++++++++++------
> 1 file changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index c79781e1ccbf..c079febae9c8 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1811,28 +1811,52 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size)
> return vbt;
> }
>
> -static const struct vbt_header *find_vbt(void __iomem *oprom, size_t size)
> +static const struct vbt_header *copy_vbt(void __iomem *oprom, size_t size)
> {
> + void __iomem *p = NULL;
> + struct vbt_header *vbt;
> + u16 vbt_size;
> size_t i;
>
> /* Scour memory looking for the VBT signature. */
> for (i = 0; i + 4 < size; i++) {
> - void *vbt;
> -
> if (ioread32(oprom + i) != *((const u32 *)"$VBT"))
> continue;
>
> - /*
> - * This is the one place where we explicitly discard the address
> - * space (__iomem) of the BIOS/VBT.
> - */
> - vbt = (void __force *)oprom + i;
> - if (intel_bios_is_valid_vbt(vbt, size - i))
> - return vbt;
> -
> + p = oprom + i;
> + size -= i;
> break;
> }
>
> + if (!p)
> + return NULL;
> +
> + if (sizeof(struct vbt_header) > size) {
> + DRM_DEBUG_DRIVER("VBT header incomplete\n");
> + return NULL;
> + }
> +
> + vbt_size = ioread16(p + offsetof(struct vbt_header, vbt_size));
> + if (vbt_size > size) {
> + DRM_DEBUG_DRIVER("VBT incomplete (vbt_size overflows)\n");
> + return NULL;
> + }
> +
> + /* The rest will be validated by intel_bios_is_valid_vbt() */
> + vbt = kmalloc(vbt_size, GFP_KERNEL);
> + if (!vbt)
> + return NULL;
> +
> + memcpy_fromio(vbt, p, vbt_size);
> +
> + if (!intel_bios_is_valid_vbt(vbt, vbt_size))
> + goto err_free_vbt;
> +
> + return vbt;
> +
> +err_free_vbt:
> + kfree(vbt);
> +
> return NULL;
> }
>
> @@ -1866,7 +1890,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
> if (!oprom)
> goto out;
>
> - vbt = find_vbt(oprom, size);
> + vbt = copy_vbt(oprom, size);
> if (!vbt)
> goto out;
>
> @@ -1902,6 +1926,9 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>
> if (oprom)
> pci_unmap_rom(pdev, oprom);
lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
As a followup we could now move the rom map/unamp into copy_vbt() as
there's no longer any need to keep it mapped across the whole thing.
> +
> + if (vbt != dev_priv->opregion.vbt)
> + kfree(vbt);
> }
>
> /**
> --
> 2.24.0
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
