virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [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, &params,
-					    ents, nents);
+	ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, &params,
+						  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, &params,
> -					    ents, nents);
> +	ret = virtio_gpu_cmd_resource_create_blob(vgdev, bo, &params,
> +						  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).