Re: [PATCH v4 10/10] drm/vboxvideo: fix mapping leaks

Hans de Goede
2024-03-03T19:41:43Z

Hi,

On 3/1/24 12:29, Philipp Stanner wrote:
> When the PCI devres API was introduced to this driver, it was wrongly
> assumed that initializing the device with pcim_enable_device() instead
> of pci_enable_device() will make all PCI functions managed.
>
> This is wrong and was caused by the quite confusing PCI devres API in
> which some, but not all, functions become managed that way.
>
> The function pci_iomap_range() is never managed.
>
> Replace pci_iomap_range() with the actually managed function
> pcim_iomap_range().
>
> CC: <stable@kernel.vger.org> # v5.10+
> Fixes: 8558de401b5f ("drm/vboxvideo: use managed pci functions")
> Signed-off-by: Philipp Stanner <pstanner@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Since this depends on the pcim_iomap_range() function which is new
in this series and since the vboxvideo code does not see a lot
of changes I think that it would be best for this patch to be
merged through the PCI tree together with the rest of the series.

Regards,

Hans

> ---
> drivers/gpu/drm/vboxvideo/vbox_main.c | 20 +++++++++-----------
> 1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_main.c b/drivers/gpu/drm/vboxvideo/vbox_main.c
> index 42c2d8a99509..d4ade9325401 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_main.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_main.c
> @@ -42,12 +42,11 @@ static int vbox_accel_init(struct vbox_private *vbox)
> /* Take a command buffer for each screen from the end of usable VRAM. */
> vbox->available_vram_size -= vbox->num_crtcs * VBVA_MIN_BUFFER_SIZE;
>
> - vbox->vbva_buffers = pci_iomap_range(pdev, 0,
> - vbox->available_vram_size,
> - vbox->num_crtcs *
> - VBVA_MIN_BUFFER_SIZE);
> - if (!vbox->vbva_buffers)
> - return -ENOMEM;
> + vbox->vbva_buffers = pcim_iomap_range(
> + pdev, 0, vbox->available_vram_size,
> + vbox->num_crtcs * VBVA_MIN_BUFFER_SIZE);
> + if (IS_ERR(vbox->vbva_buffers))
> + return PTR_ERR(vbox->vbva_buffers);
>
> for (i = 0; i < vbox->num_crtcs; ++i) {
> vbva_setup_buffer_context(&vbox->vbva_info[i],
> @@ -116,11 +115,10 @@ int vbox_hw_init(struct vbox_private *vbox)
> DRM_INFO("VRAM %08x\n", vbox->full_vram_size);
>
> /* Map guest-heap at end of vram */
> - vbox->guest_heap =
> - pci_iomap_range(pdev, 0, GUEST_HEAP_OFFSET(vbox),
> - GUEST_HEAP_SIZE);
> - if (!vbox->guest_heap)
> - return -ENOMEM;
> + vbox->guest_heap = pcim_iomap_range(pdev, 0,
> + GUEST_HEAP_OFFSET(vbox), GUEST_HEAP_SIZE);
> + if (IS_ERR(vbox->guest_heap))
> + return PTR_ERR(vbox->guest_heap);
>
> /* Create guest-heap mem-pool use 2^4 = 16 byte chunks */
> vbox->guest_pool = devm_gen_pool_create(vbox->ddev.dev, 4, -1,