Maksym Wezdecki
2021-Nov-03 11:25 UTC
[PATCH] drm/virtio: new ioctl for pinning and lazy pinning
First patch implements new ioctl. If there are no pages pinned to bo object, then pin it. Second patch implements concepts of lazy pin. Userspace must opt-in for not pinning pages. I would like to record this work here and investigate another possibility. Mainly, problem statement for this two patches is to reduce memory usage consumed by Guest. If we can avoid pinning pages for such resources like textures, then we can use staging buffer to upload texture data to host.
Maksym Wezdecki
2021-Nov-03 11:25 UTC
[PATCH 1/2] drm/virtio: introduce ioctl for pinning pages
From: mwezdeck <maksym.wezdecki at collabora.co.uk>
Pinning pages happens in virtio_gpu_object_shmem_init()
function.
This ioctl allows to pin pages if it was not done earlier.
Signed-off-by: mwezdeck <maksym.wezdecki at collabora.co.uk>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 5 +++-
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 11 ++++++++
drivers/gpu/drm/virtio/virtgpu_object.c | 34 +++++++++++++++++++++++++
include/uapi/drm/virtgpu_drm.h | 9 +++++++
4 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index e0265fe74aa5..232933919b91 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -278,7 +278,7 @@ struct virtio_gpu_fpriv {
};
/* virtgpu_ioctl.c */
-#define DRM_VIRTIO_NUM_IOCTLS 12
+#define DRM_VIRTIO_NUM_IOCTLS 13
extern struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS];
void virtio_gpu_create_context(struct drm_device *dev, struct drm_file *file);
@@ -455,6 +455,9 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_object **bo_ptr,
struct virtio_gpu_fence *fence);
+int virtio_gpu_object_pin(struct drm_file *file,
+ struct virtio_gpu_device *vgdev, uint32_t handle);
+
bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
int virtio_gpu_resource_id_get(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index 0007e423d885..f6a3a760c32d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -836,6 +836,14 @@ static int virtio_gpu_context_init_ioctl(struct drm_device
*dev,
return ret;
}
+static int virtio_gpu_pin_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+ struct virtio_gpu_device *vgdev = dev->dev_private;
+ struct drm_virtgpu_pin *virtio_gpu_pin = data;
+ return virtio_gpu_object_pin(file, vgdev, virtio_gpu_pin->handle);
+}
+
struct drm_ioctl_desc virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = {
DRM_IOCTL_DEF_DRV(VIRTGPU_MAP, virtio_gpu_map_ioctl,
DRM_RENDER_ALLOW),
@@ -875,4 +883,7 @@ struct drm_ioctl_desc
virtio_gpu_ioctls[DRM_VIRTIO_NUM_IOCTLS] = {
DRM_IOCTL_DEF_DRV(VIRTGPU_CONTEXT_INIT, virtio_gpu_context_init_ioctl,
DRM_RENDER_ALLOW),
+
+ DRM_IOCTL_DEF_DRV(VIRTGPU_PIN, virtio_gpu_pin_ioctl,
+ DRM_RENDER_ALLOW),
};
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index f648b0e24447..064c50cb9846 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -280,3 +280,37 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
drm_gem_shmem_free_object(&shmem_obj->base);
return ret;
}
+
+int virtio_gpu_object_pin(struct drm_file *file,
+ struct virtio_gpu_device *vgdev, uint32_t handle)
+{
+ int ret;
+ struct drm_gem_object *gem;
+ struct virtio_gpu_object *bo;
+ struct virtio_gpu_object_shmem *shmem;
+ struct virtio_gpu_mem_entry *ents;
+ unsigned int nents;
+
+ gem = drm_gem_object_lookup(file, handle);
+ if (gem == NULL)
+ return -ENOENT;
+
+ bo = gem_to_virtio_gpu_obj(gem);
+ if (bo == NULL)
+ return -ENOENT;
+
+ shmem = to_virtio_gpu_shmem(bo);
+ if (shmem == NULL)
+ return -ENOENT;
+
+ if (!shmem->pages) {
+ ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
+ if (ret != 0) {
+ return -EFAULT;
+ }
+
+ virtio_gpu_object_attach(vgdev, bo, ents, nents);
+ }
+
+ return 0;
+}
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index a13e20cc66b4..bb661d53c0e9 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -48,6 +48,7 @@ extern "C" {
#define DRM_VIRTGPU_GET_CAPS 0x09
#define DRM_VIRTGPU_RESOURCE_CREATE_BLOB 0x0a
#define DRM_VIRTGPU_CONTEXT_INIT 0x0b
+#define DRM_VIRTGPU_PIN 0x0c
#define VIRTGPU_EXECBUF_FENCE_FD_IN 0x01
#define VIRTGPU_EXECBUF_FENCE_FD_OUT 0x02
@@ -196,6 +197,10 @@ struct drm_virtgpu_context_init {
__u64 ctx_set_params;
};
+struct drm_virtgpu_pin {
+ __u32 handle;
+};
+
#define DRM_IOCTL_VIRTGPU_MAP \
DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_MAP, struct drm_virtgpu_map)
@@ -239,6 +244,10 @@ struct drm_virtgpu_context_init {
DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_CONTEXT_INIT, \
struct drm_virtgpu_context_init)
+#define DRM_IOCTL_VIRTGPU_PIN \
+ DRM_IOWR(DRM_COMMAND_BASE + DRM_VIRTGPU_PIN, \
+ struct drm_virtgpu_pin)
+
#if defined(__cplusplus)
}
#endif
--
2.30.2
From: mwezdeck <maksym.wezdecki at collabora.co.uk>
Userspace can opt-in to not pin pages during resource
create ioctl.
In transfer_*_host and map ioctls check if memory is pinned.
If pages are not pinned, pin it. Otherwise, do nothing.
This change is transparent to userspace.
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 9 +++++++++
drivers/gpu/drm/virtio/virtgpu_object.c | 27 ++++++++++++++++---------
include/uapi/drm/virtgpu_drm.h | 9 +++++++++
3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index f6a3a760c32d..c01c5c15701c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -103,6 +103,8 @@ static int virtio_gpu_map_ioctl(struct drm_device *dev, void
*data,
struct virtio_gpu_device *vgdev = dev->dev_private;
struct drm_virtgpu_map *virtio_gpu_map = data;
+ virtio_gpu_object_pin(file, vgdev, virtio_gpu_map->handle);
+
return virtio_gpu_mode_dumb_mmap(file, vgdev->ddev,
virtio_gpu_map->handle,
&virtio_gpu_map->offset);
@@ -292,6 +294,9 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev,
void *data,
case VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs:
value = vgdev->capset_id_mask;
break;
+ case VIRTGPU_PARAM_PIN_ON_DEMAND:
+ value = 1;
+ break;
default:
return -EINVAL;
}
@@ -414,6 +419,8 @@ static int virtio_gpu_transfer_from_host_ioctl(struct
drm_device *dev,
goto err_put_free;
}
+ virtio_gpu_object_pin(file, vgdev, args->bo_handle);
+
if (!bo->host3d_blob && (args->stride || args->layer_stride))
{
ret = -EINVAL;
goto err_put_free;
@@ -465,6 +472,8 @@ static int virtio_gpu_transfer_to_host_ioctl(struct
drm_device *dev, void *data,
goto err_put_free;
}
+ virtio_gpu_object_pin(file, vgdev, args->bo_handle);
+
if (!vgdev->has_virgl_3d) {
virtio_gpu_cmd_transfer_to_host_2d
(vgdev, offset,
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 064c50cb9846..183e57ef10e8 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -219,7 +219,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
struct virtio_gpu_mem_entry *ents;
unsigned int nents;
int ret;
-
+ uint32_t backup_flags = params->flags;
*bo_ptr = NULL;
params->size = roundup(params->size, PAGE_SIZE);
@@ -246,13 +246,19 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
goto err_put_objs;
}
- ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
- if (ret != 0) {
- virtio_gpu_array_put_free(objs);
- virtio_gpu_free_object(&shmem_obj->base);
- return ret;
+ if (!(backup_flags & VIRTGPU_NOT_PIN_FLAG)) {
+ ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
+ if (ret != 0) {
+ virtio_gpu_array_put_free(objs);
+ virtio_gpu_free_object(&shmem_obj->base);
+ return ret;
+ }
}
+ // turn off these bits, as renderer doesn't support such bits
+ if (params->flags & VIRTGPU_NOT_PIN_FLAG)
+ params->flags &= ~(VIRTGPU_NOT_PIN_FLAG);
+
if (params->blob) {
if (params->blob_mem == VIRTGPU_BLOB_MEM_GUEST)
bo->guest_blob = true;
@@ -262,11 +268,13 @@ int virtio_gpu_object_create(struct virtio_gpu_device
*vgdev,
} else if (params->virgl) {
virtio_gpu_cmd_resource_create_3d(vgdev, bo, params,
objs, fence);
- virtio_gpu_object_attach(vgdev, bo, ents, nents);
+ if (!(backup_flags & VIRTGPU_NOT_PIN_FLAG))
+ virtio_gpu_object_attach(vgdev, bo, ents, nents);
} else {
virtio_gpu_cmd_create_resource(vgdev, bo, params,
objs, fence);
- virtio_gpu_object_attach(vgdev, bo, ents, nents);
+ if (!(backup_flags & VIRTGPU_NOT_PIN_FLAG))
+ virtio_gpu_object_attach(vgdev, bo, ents, nents);
}
*bo_ptr = bo;
@@ -305,9 +313,8 @@ int virtio_gpu_object_pin(struct drm_file *file,
if (!shmem->pages) {
ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, &nents);
- if (ret != 0) {
+ if (ret != 0)
return -EFAULT;
- }
virtio_gpu_object_attach(vgdev, bo, ents, nents);
}
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index bb661d53c0e9..0780234f946f 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -83,12 +83,21 @@ struct drm_virtgpu_execbuffer {
#define VIRTGPU_PARAM_CROSS_DEVICE 5 /* Cross virtio-device resource sharing
*/
#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_PIN_ON_DEMAND 8 /* is pinning on demand available? */
struct drm_virtgpu_getparam {
__u64 param;
__u64 value;
};
+/* it is used in resource_create_ioctl as resource
+ * flag.
+ * First 8 bits of uint32_t and 24th bit
+ * are reserved for user space driver.
+ * Userspace can opt-in to not pin pages.
+ */
+#define VIRTGPU_NOT_PIN_FLAG (1 << 9)
+
/* NO_BO flags? NO resource flag? */
/* resource flag for y_0_top */
struct drm_virtgpu_resource_create {
--
2.30.2