* [PATCH 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support
@ 2025-11-10 12:52 Sergio Lopez
2025-11-10 12:52 ` [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT Sergio Lopez
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Sergio Lopez @ 2025-11-10 12:52 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
Cc: Sergio Lopez
There's an incresing number of machines supporting multiple page sizes
and on these machines the host and a guest can be running, each one,
with a different page size.
For what pertains to virtio-gpu, this is not a problem if the page size
of the guest happens to be bigger or equal than the host, but will
potentially lead to failures in memory allocations and/or mappings
otherwise.
To deal with this, the virtio-spec was extended to introduce with the
VIRTIO_GPU_F_BLOB_ALIGNMENT feature [1]. If this feature is negotiated,
we must use the "blob_alignment" field in the config to ensure every
CREATE_BLOB and MAP_BLOB is properly aligned before sending it to the
device.
We also introduce the VIRTGPU_PARAM_BLOB_ALIGNMENT parameter to allow
userspace to query the alignment restrictions for blobs.
This supersedes "drm/virtio: introduce the HOST_PAGE_SIZE feature" [2].
[1] https://github.com/oasis-tcs/virtio-spec/commit/f9abfd55cb663837dda1153b826216dcf4d25b84
[2] https://lkml.org/lkml/2024/7/23/438
Sergio Lopez (3):
drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
drm/virtio: honor blob_alignment requirements
drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params
drivers/gpu/drm/virtio/virtgpu_drv.c | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 4 +++-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 14 +++++++++++---
drivers/gpu/drm/virtio/virtgpu_object.c | 6 ++++--
drivers/gpu/drm/virtio/virtgpu_prime.c | 7 +++++--
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 +++++++++++-
drivers/gpu/drm/virtio/virtgpu_vram.c | 10 ++++++++--
include/uapi/drm/virtgpu_drm.h | 1 +
include/uapi/linux/virtio_gpu.h | 9 +++++++++
10 files changed, 58 insertions(+), 11 deletions(-)
--
2.51.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
2025-11-10 12:52 [PATCH 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support Sergio Lopez
@ 2025-11-10 12:52 ` Sergio Lopez
2025-11-12 12:14 ` Dmitry Osipenko
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
2025-11-10 12:52 ` [PATCH 3/3] drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params Sergio Lopez
2 siblings, 1 reply; 12+ messages in thread
From: Sergio Lopez @ 2025-11-10 12:52 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
Cc: Sergio Lopez
Support VIRTIO_GPU_F_BLOB_ALIGNMENT, a feature that indicates the device
provides a valid blob_alignment field in its configuration, and that
both RESOURCE_CREATE_BLOB and RESOURCE_MAP_BLOB requests must be aligned
to that value.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++
drivers/gpu/drm/virtio/virtgpu_kms.c | 14 +++++++++++---
include/uapi/linux/virtio_gpu.h | 9 +++++++++
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 71c6ccad4b99..ad675ee18b01 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -162,6 +162,7 @@ static unsigned int features[] = {
VIRTIO_GPU_F_RESOURCE_UUID,
VIRTIO_GPU_F_RESOURCE_BLOB,
VIRTIO_GPU_F_CONTEXT_INIT,
+ VIRTIO_GPU_F_BLOB_ALIGNMENT,
};
static struct virtio_driver virtio_gpu_driver = {
.feature_table = features,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..d1fa386a5a99 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -257,6 +257,7 @@ struct virtio_gpu_device {
bool has_resource_blob;
bool has_host_visible;
bool has_context_init;
+ bool has_blob_alignment;
struct virtio_shm_region host_visible_region;
struct drm_mm host_visible_mm;
@@ -270,6 +271,7 @@ struct virtio_gpu_device {
uint32_t num_capsets;
uint64_t capset_id_mask;
struct list_head cap_cache;
+ uint32_t blob_alignment;
/* protects uuid state when exporting */
spinlock_t resource_export_lock;
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 1c15cbf326b7..a20b62041f03 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -123,7 +123,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
struct virtio_gpu_device *vgdev;
/* this will expand later */
struct virtqueue *vqs[2];
- u32 num_scanouts, num_capsets;
+ u32 num_scanouts, num_capsets, blob_alignment;
int ret = 0;
if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
@@ -197,14 +197,22 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_CONTEXT_INIT))
vgdev->has_context_init = true;
+ if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_ALIGNMENT)) {
+ vgdev->has_blob_alignment = true;
+ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+ blob_alignment, &blob_alignment);
+ vgdev->blob_alignment = blob_alignment;
+ }
+
DRM_INFO("features: %cvirgl %cedid %cresource_blob %chost_visible",
vgdev->has_virgl_3d ? '+' : '-',
vgdev->has_edid ? '+' : '-',
vgdev->has_resource_blob ? '+' : '-',
vgdev->has_host_visible ? '+' : '-');
- DRM_INFO("features: %ccontext_init\n",
- vgdev->has_context_init ? '+' : '-');
+ DRM_INFO("features: %ccontext_init %cblob_alignment\n",
+ vgdev->has_context_init ? '+' : '-',
+ vgdev->has_blob_alignment ? '+' : '-');
ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
if (ret) {
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index be109777d10d..4f530d90058c 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -64,6 +64,14 @@
* context_init and multiple timelines
*/
#define VIRTIO_GPU_F_CONTEXT_INIT 4
+/*
+ * The device provides a valid blob_alignment
+ * field in its configuration and both
+ * VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB and
+ * VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB requests
+ * must be aligned to that value.
+ */
+#define VIRTIO_GPU_F_BLOB_ALIGNMENT 5
enum virtio_gpu_ctrl_type {
VIRTIO_GPU_UNDEFINED = 0,
@@ -365,6 +373,7 @@ struct virtio_gpu_config {
__le32 events_clear;
__le32 num_scanouts;
__le32 num_capsets;
+ __le32 blob_alignment;
};
/* simple formats for fbcon/X use */
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 [PATCH 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support Sergio Lopez
2025-11-10 12:52 ` [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT Sergio Lopez
@ 2025-11-10 12:52 ` Sergio Lopez
2025-11-13 2:20 ` Dmitry Osipenko
` (4 more replies)
2025-11-10 12:52 ` [PATCH 3/3] drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params Sergio Lopez
2 siblings, 5 replies; 12+ messages in thread
From: Sergio Lopez @ 2025-11-10 12:52 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
Cc: Sergio Lopez
If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, both
CREATE_BLOB and MAP_BLOB requests must be aligned to blob_alignment.
Introduce checks to ensure we don't send invalid requests to the
device.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
drivers/gpu/drm/virtio/virtgpu_object.c | 6 ++++--
drivers/gpu/drm/virtio/virtgpu_prime.c | 7 +++++--
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 +++++++++++-
drivers/gpu/drm/virtio/virtgpu_vram.c | 10 ++++++++--
5 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index d1fa386a5a99..11a55d4ccd54 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -433,7 +433,7 @@ int virtio_gpu_cmd_map(struct virtio_gpu_device *vgdev,
void virtio_gpu_cmd_unmap(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo);
-void
+int
virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo,
struct virtio_gpu_object_params *params,
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index e6363c887500..6118344bff84 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -246,8 +246,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
bo->guest_blob = true;
- virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
- ents, nents);
+ ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
+ ents, nents);
+ if (ret)
+ goto err_put_objs;
} else if (params->virgl) {
virtio_gpu_cmd_resource_create_3d(vgdev, bo, params,
objs, fence);
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index ce49282198cb..06593496c53f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -257,8 +257,11 @@ static int virtgpu_dma_buf_init_obj(struct drm_device *dev,
params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
params.size = attach->dmabuf->size;
- virtio_gpu_cmd_resource_create_blob(vgdev, bo, ¶ms,
- ents, nents);
+ ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, ¶ms,
+ ents, nents);
+ if (ret)
+ goto err_import;
+
bo->guest_blob = true;
dma_buf_unpin(attach);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 8181b22b9b46..d558ba2d213a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -1393,6 +1393,10 @@ int virtio_gpu_cmd_map(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf;
struct virtio_gpu_resp_map_info *resp_buf;
+ if (vgdev->has_blob_alignment &&
+ !IS_ALIGNED(offset, vgdev->blob_alignment))
+ return -EINVAL;
+
resp_buf = kzalloc(sizeof(*resp_buf), GFP_KERNEL);
if (!resp_buf)
return -ENOMEM;
@@ -1426,7 +1430,7 @@ void virtio_gpu_cmd_unmap(struct virtio_gpu_device *vgdev,
virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
}
-void
+int
virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo,
struct virtio_gpu_object_params *params,
@@ -1436,6 +1440,10 @@ virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
struct virtio_gpu_resource_create_blob *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
+ if (vgdev->has_blob_alignment &&
+ !IS_ALIGNED(params->size, vgdev->blob_alignment))
+ return -EINVAL;
+
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
@@ -1456,6 +1464,8 @@ virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
if (nents)
bo->attached = true;
+
+ return 0;
}
void virtio_gpu_cmd_set_scanout_blob(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_vram.c b/drivers/gpu/drm/virtio/virtgpu_vram.c
index 5ad3b7c6f73c..6c402eca5726 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vram.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vram.c
@@ -216,8 +216,14 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
return ret;
}
- virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
- 0);
+ ret = virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params,
+ NULL, 0);
+ if (ret) {
+ drm_gem_free_mmap_offset(obj);
+ kfree(vram);
+ return ret;
+ }
+
if (params->blob_flags & VIRTGPU_BLOB_FLAG_USE_MAPPABLE) {
ret = virtio_gpu_vram_map(&vram->base);
if (ret) {
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params
2025-11-10 12:52 [PATCH 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support Sergio Lopez
2025-11-10 12:52 ` [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT Sergio Lopez
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
@ 2025-11-10 12:52 ` Sergio Lopez
2 siblings, 0 replies; 12+ messages in thread
From: Sergio Lopez @ 2025-11-10 12:52 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
Cc: Sergio Lopez
Add VIRTGPU_PARAM_BLOB_ALIGNMENT as a param that can be read with
VIRTGPU_GETPARAM by userspace applications running in the guest to
obtain the host's page size and find out the right alignment to be used
in shared memory allocations.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
include/uapi/drm/virtgpu_drm.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index c33c057365f8..b7651f504f00 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -117,6 +117,11 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data,
case VIRTGPU_PARAM_EXPLICIT_DEBUG_NAME:
value = vgdev->has_context_init ? 1 : 0;
break;
+ case VIRTGPU_PARAM_BLOB_ALIGNMENT:
+ if (!vgdev->has_blob_alignment)
+ return -ENOENT;
+ value = vgdev->blob_alignment;
+ break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index 9debb320c34b..7d4e4884d942 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -98,6 +98,7 @@ struct drm_virtgpu_execbuffer {
#define VIRTGPU_PARAM_CONTEXT_INIT 6 /* DRM_VIRTGPU_CONTEXT_INIT */
#define VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs 7 /* Bitmask of supported capability set ids */
#define VIRTGPU_PARAM_EXPLICIT_DEBUG_NAME 8 /* Ability to set debug name from userspace */
+#define VIRTGPU_PARAM_BLOB_ALIGNMENT 9 /* Device alignment requirements for blobs */
struct drm_virtgpu_getparam {
__u64 param;
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
2025-11-10 12:52 ` [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT Sergio Lopez
@ 2025-11-12 12:14 ` Dmitry Osipenko
2025-11-12 16:49 ` Sergio Lopez Pascual
0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-12 12:14 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> + if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_ALIGNMENT)) {
> + vgdev->has_blob_alignment = true;
> + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
> + blob_alignment, &blob_alignment);
> + vgdev->blob_alignment = blob_alignment;
Shouldn't blob_alignment be max(guest_alignment, host_alignment)?
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
2025-11-12 12:14 ` Dmitry Osipenko
@ 2025-11-12 16:49 ` Sergio Lopez Pascual
2025-11-13 2:20 ` Dmitry Osipenko
0 siblings, 1 reply; 12+ messages in thread
From: Sergio Lopez Pascual @ 2025-11-12 16:49 UTC (permalink / raw)
To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
> On 11/10/25 15:52, Sergio Lopez wrote:
>> + if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_ALIGNMENT)) {
>> + vgdev->has_blob_alignment = true;
>> + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
>> + blob_alignment, &blob_alignment);
>> + vgdev->blob_alignment = blob_alignment;
>
> Shouldn't blob_alignment be max(guest_alignment, host_alignment)?
virtio_gpu_config is the minimum alignment required by the device/host.
If the guest requires a higher alignment than the device/host, I would
expect that to be found by a different mechanism, as it would happen on
gpu drivers other than virtio-gpu.
Sergio.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
2025-11-12 16:49 ` Sergio Lopez Pascual
@ 2025-11-13 2:20 ` Dmitry Osipenko
0 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:20 UTC (permalink / raw)
To: Sergio Lopez Pascual, David Airlie, Gerd Hoffmann,
Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Simona Vetter, Michael S. Tsirkin, Jason Wang,
Xuan Zhuo, Eugenio Pérez, dri-devel, virtualization,
linux-kernel
On 11/12/25 19:49, Sergio Lopez Pascual wrote:
> Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:
>
>> On 11/10/25 15:52, Sergio Lopez wrote:
>>> + if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_ALIGNMENT)) {
>>> + vgdev->has_blob_alignment = true;
>>> + virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
>>> + blob_alignment, &blob_alignment);
>>> + vgdev->blob_alignment = blob_alignment;
>>
>> Shouldn't blob_alignment be max(guest_alignment, host_alignment)?
>
> virtio_gpu_config is the minimum alignment required by the device/host.
> If the guest requires a higher alignment than the device/host, I would
> expect that to be found by a different mechanism, as it would happen on
> gpu drivers other than virtio-gpu.
Alright, perhaps that will work.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
@ 2025-11-13 2:20 ` Dmitry Osipenko
2025-11-13 2:20 ` Dmitry Osipenko
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:20 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
> index e6363c887500..6118344bff84 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -246,8 +246,10 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
> if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
> bo->guest_blob = true;
>
> - virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
> - ents, nents);
> + ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, params,
> + ents, nents);
> + if (ret)
> + goto err_put_objs;
Isn't it impossible for host to import guest blob when HOST_PAGE_SIZE >
GUEST_PAGE_SIZE?
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
2025-11-13 2:20 ` Dmitry Osipenko
@ 2025-11-13 2:20 ` Dmitry Osipenko
2025-11-13 2:28 ` Dmitry Osipenko
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:20 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> @@ -216,8 +216,14 @@ int virtio_gpu_vram_create(struct virtio_gpu_device *vgdev,
> return ret;
> }
>
> - virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params, NULL,
> - 0);
> + ret = virtio_gpu_cmd_resource_create_blob(vgdev, &vram->base, params,
> + NULL, 0);
> + if (ret) {
> + drm_gem_free_mmap_offset(obj);
> + kfree(vram);
> + return ret;
> + }
virtio_gpu_cleanup_object() shall be used to free obj, otherwise
resource_ida and etc is leaked
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
2025-11-13 2:20 ` Dmitry Osipenko
2025-11-13 2:20 ` Dmitry Osipenko
@ 2025-11-13 2:28 ` Dmitry Osipenko
2025-11-13 2:34 ` Dmitry Osipenko
2025-11-13 2:40 ` Dmitry Osipenko
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:28 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 8181b22b9b46..d558ba2d213a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -1393,6 +1393,10 @@ int virtio_gpu_cmd_map(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf;
> struct virtio_gpu_resp_map_info *resp_buf;
>
> + if (vgdev->has_blob_alignment &&
> + !IS_ALIGNED(offset, vgdev->blob_alignment))
> + return -EINVAL;
The vram offset should have been validated when obj is created. Checking
offset here on cmd_map() should be unnecessary.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
` (2 preceding siblings ...)
2025-11-13 2:28 ` Dmitry Osipenko
@ 2025-11-13 2:34 ` Dmitry Osipenko
2025-11-13 2:40 ` Dmitry Osipenko
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:34 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
> index ce49282198cb..06593496c53f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -257,8 +257,11 @@ static int virtgpu_dma_buf_init_obj(struct drm_device *dev,
> params.blob_flags = VIRTGPU_BLOB_FLAG_USE_SHAREABLE;
> params.size = attach->dmabuf->size;
>
> - virtio_gpu_cmd_resource_create_blob(vgdev, bo, ¶ms,
> - ents, nents);
> + ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, ¶ms,
> + ents, nents);
> + if (ret)
> + goto err_import;
The error handling is incorrect, dma_buf_unmap_attachment() won't be
invoked when it should be.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] drm/virtio: honor blob_alignment requirements
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
` (3 preceding siblings ...)
2025-11-13 2:34 ` Dmitry Osipenko
@ 2025-11-13 2:40 ` Dmitry Osipenko
4 siblings, 0 replies; 12+ messages in thread
From: Dmitry Osipenko @ 2025-11-13 2:40 UTC (permalink / raw)
To: Sergio Lopez, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez, dri-devel, virtualization, linux-kernel
On 11/10/25 15:52, Sergio Lopez wrote:
> -void
> +int
> virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_object *bo,
> struct virtio_gpu_object_params *params,
> @@ -1436,6 +1440,10 @@ virtio_gpu_cmd_resource_create_blob(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_resource_create_blob *cmd_p;
> struct virtio_gpu_vbuffer *vbuf;
>
> + if (vgdev->has_blob_alignment &&
> + !IS_ALIGNED(params->size, vgdev->blob_alignment))
> + return -EINVAL;
Perhaps will be better to validate params.size earlier, within
verify_blob(). Then all the added errors handling should become unnecessary.
--
Best regards,
Dmitry
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-11-13 2:40 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-10 12:52 [PATCH 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support Sergio Lopez
2025-11-10 12:52 ` [PATCH 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT Sergio Lopez
2025-11-12 12:14 ` Dmitry Osipenko
2025-11-12 16:49 ` Sergio Lopez Pascual
2025-11-13 2:20 ` Dmitry Osipenko
2025-11-10 12:52 ` [PATCH 2/3] drm/virtio: honor blob_alignment requirements Sergio Lopez
2025-11-13 2:20 ` Dmitry Osipenko
2025-11-13 2:20 ` Dmitry Osipenko
2025-11-13 2:28 ` Dmitry Osipenko
2025-11-13 2:34 ` Dmitry Osipenko
2025-11-13 2:40 ` Dmitry Osipenko
2025-11-10 12:52 ` [PATCH 3/3] drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params Sergio Lopez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).