linux-kernel - Re: [PATCH v5] drm/virtio: Add drm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ca0382c-a410-4424-a5a4-7c1b8be7ebed@collabora.com>
Date: Mon, 2 Dec 2024 19:19:31 +0300
From: Dmitry Osipenko <dmitry.osipenko@...labora.com>
To: Ryosuke Yasuoka <ryasuoka@...hat.com>, airlied@...hat.com,
 kraxel@...hat.com, maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
 tzimmermann@...e.de, daniel@...ll.ch, jfalempe@...hat.com,
 christophe.jaillet@...adoo.fr
Cc: virtualization@...ts.linux.dev, spice-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v5] drm/virtio: Add drm_panic support

On 12/2/24 17:29, Dmitry Osipenko wrote:
> On 11/29/24 15:24, Ryosuke Yasuoka wrote:
> ....
>> +static int virtio_drm_get_scanout_buffer(struct drm_plane *plane,
>> +					 struct drm_scanout_buffer *sb)
>> +{
>> +	struct virtio_gpu_object *bo;
>> +
>> +	if (!plane->state || !plane->state->fb || !plane->state->visible)
>> +		return -ENODEV;
>> +
>> +	bo = gem_to_virtio_gpu_obj(plane->state->fb->obj[0]);
>> +	if (virtio_gpu_is_vram(bo))
>> +		return -ENODEV;
> 
> VirtIO-GPU now supports importing external dmabufs, we should reject
> bo->base.base.import_attach here now too.
> 
>> +
>> +	/* try to vmap it if possible */
>> +	if (!bo->base.vaddr) {
>> +		int ret;
>> +
>> +		ret = drm_gem_shmem_vmap(&bo->base, &sb->map[0]);
>> +		if (ret)
>> +			return ret;
> 
> I've now noticed that drm_gem_shmem_vmap() expects BO reservation lock
> to be held and we can't take lock it at a panic time.
> 
> https://elixir.bootlin.com/linux/v6.12.1/source/drivers/gpu/drm/drm_gem_shmem_helper.c#L330
> 
> This resv warning isn't triggered because bo->base.vaddr is set for VT
> framebufffer BO when panic happens.
> 
> We actually should reject all BOs that don't have bo->base.vaddr set at
> the panic time. Please correct it in v6 and rebase on top of a recent
> drm-next tree.

Think ideally we need to have a pre-allocated and pre-mapped BO. Then
when panic happens, use that BO and tell host to display it, i.e. not to
reuse currently displayed BO. This will make panic display work in all
conditions. WDYT?

-- 
Best regards,
Dmitry