From: Akihiko Odaki <[email protected]>
virtio_gpu_find_check_resource() checks if the resource has backing
storage if require_backing is true, but the condition conflates backing
storage attachment with host representation; it checks
!res->iov || (!res->image && !res->blob), but !res->iov is sufficient.
Furthermore, its callers passing true as require_backing have different
requirements:
- virtio_gpu_transfer_to_host_2d() requires a non-blob with
backing storage.
- virtio_gpu_set_scanout() requires a non-blob but does not require
backing storage.
- virtio_gpu_set_scanout_blob() requires a blob with backing storage.
- virtio_gpu_resource_detach_backing() accepts any resource.
Remove the require_backing parameter and open-code checks appropriate
for each function instead.
Fixes: 25c001a40346 ("virtio-gpu: Add virtio_gpu_find_check_resource")
Fixes: e0933d91b1cd ("virtio-gpu: Add virtio_gpu_resource_create_blob")
Fixes: 32db3c63ae11 ("virtio-gpu: Add virtio_gpu_set_scanout_blob")
Signed-off-by: Akihiko Odaki <[email protected]>
Reviewed-by: Marc-André Lureau <[email protected]>
Message-ID: <[email protected]>
---
hw/display/virtio-gpu.c | 66 +++++++++++++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 21 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 01549d29d8a2..55a1c7f80fb8 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -37,7 +37,6 @@
static struct virtio_gpu_simple_resource *
virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
- bool require_backing,
const char *caller, uint32_t *error);
static void virtio_gpu_reset_bh(void *opaque);
@@ -50,8 +49,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
uint32_t pixels;
void *data;
- res = virtio_gpu_find_check_resource(g, resource_id, false,
- __func__, NULL);
+ res = virtio_gpu_find_check_resource(g, resource_id, __func__, NULL);
if (!res) {
return;
}
@@ -128,7 +126,6 @@ virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id)
static struct virtio_gpu_simple_resource *
virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
- bool require_backing,
const char *caller, uint32_t *error)
{
struct virtio_gpu_simple_resource *res;
@@ -143,17 +140,6 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t
resource_id,
return NULL;
}
- if (require_backing) {
- if (!res->iov || (!res->image && !res->blob)) {
- qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
- caller, resource_id);
- if (error) {
- *error = VIRTIO_GPU_RESP_ERR_UNSPEC;
- }
- return NULL;
- }
- }
-
return res;
}
@@ -474,9 +460,24 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
virtio_gpu_t2d_bswap(&t2d);
trace_virtio_gpu_cmd_res_xfer_toh_2d(t2d.resource_id);
- res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
+ res = virtio_gpu_find_check_resource(g, t2d.resource_id,
__func__, &cmd->error);
- if (!res || res->blob) {
+ if (!res) {
+ return;
+ }
+
+ if (!res->image) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource %d is a blob\n",
+ __func__, t2d.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ if (!res->iov) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: resource %d has no backing storage\n",
+ __func__, t2d.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
return;
}
@@ -533,7 +534,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
trace_virtio_gpu_cmd_res_flush(rf.resource_id,
rf.r.width, rf.r.height, rf.r.x, rf.r.y);
- res = virtio_gpu_find_check_resource(g, rf.resource_id, false,
+ res = virtio_gpu_find_check_resource(g, rf.resource_id,
__func__, &cmd->error);
if (!res) {
return;
@@ -771,12 +772,19 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
return;
}
- res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
+ res = virtio_gpu_find_check_resource(g, ss.resource_id,
__func__, &cmd->error);
if (!res) {
return;
}
+ if (!res->image) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: resource %d is a blob\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
fb.format = pixman_image_get_format(res->image);
bytes_pp = virtio_gpu_format_bytes_pp(fb.format);
fb.width = pixman_image_get_width(res->image);
@@ -866,12 +874,28 @@ static void virtio_gpu_set_scanout_blob(VirtIOGPU *g,
return;
}
- res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
+ res = virtio_gpu_find_check_resource(g, ss.resource_id,
__func__, &cmd->error);
if (!res) {
return;
}
+ if (res->image) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: resource %d is not a blob\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
+ if (!res->iov) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: resource %d has no backing storage\n",
+ __func__, ss.resource_id);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+ return;
+ }
+
if (!virtio_gpu_scanout_blob_to_fb(&fb, &ss, res->blob_size)) {
cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
return;
@@ -1067,7 +1091,7 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
virtio_gpu_bswap_32(&detach, sizeof(detach));
trace_virtio_gpu_cmd_res_back_detach(detach.resource_id);
- res = virtio_gpu_find_check_resource(g, detach.resource_id, true,
+ res = virtio_gpu_find_check_resource(g, detach.resource_id,
__func__, &cmd->error);
if (!res) {
return;
--
2.55.0.543.g5ebe2ebe4ea8
