vrend/decode: refactor parameters check in vrend_create_surface

Part-of: <https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1228>

4 files changed, 73 insertions, 24 deletions

diff --git a/src/vrend_decode.c b/src/vrend_decode.c
index ab056c9..5d1a21e 100644
--- a/src/vrend_decode.c
+++ b/src/vrend_decode.c
@@ -605,15 +605,25 @@ static int vrend_decode_create_rasterizer(struct vrend_context *ctx, const uint3
 
 static int vrend_decode_create_surface_common(struct vrend_context *ctx, const uint32_t *buf, uint32_t handle, uint32_t sample_count)
 {
-   uint32_t res_handle, format, val0, val1;
+   uint32_t res_handle = get_buf_entry(buf, VIRGL_OBJ_SURFACE_RES_HANDLE);
+
+   struct vrend_resource *res = vrend_renderer_ctx_res_lookup(ctx, res_handle);
+   if (!res) {
+      vrend_report_context_error(ctx, VIRGL_ERROR_CTX_ILLEGAL_RESOURCE, res_handle);
+      return EINVAL;
+   }
+
+   enum virgl_formats format = get_buf_entry(buf, VIRGL_OBJ_SURFACE_FORMAT);
+
+   if (format >= PIPE_FORMAT_COUNT) {
+      return EINVAL;
+   }
 
-   res_handle = get_buf_entry(buf, VIRGL_OBJ_SURFACE_RES_HANDLE);
-   format = get_buf_entry(buf, VIRGL_OBJ_SURFACE_FORMAT);
    /* decide later if these are texture or buffer */
-   val0 = get_buf_entry(buf, VIRGL_OBJ_SURFACE_BUFFER_FIRST_ELEMENT);
-   val1 = get_buf_entry(buf, VIRGL_OBJ_SURFACE_BUFFER_LAST_ELEMENT);
+   uint32_t val0 = get_buf_entry(buf, VIRGL_OBJ_SURFACE_BUFFER_FIRST_ELEMENT);
+   uint32_t val1 = get_buf_entry(buf, VIRGL_OBJ_SURFACE_BUFFER_LAST_ELEMENT);
 
-   return vrend_create_surface(ctx, handle, res_handle, format, val0, val1, sample_count);
+   return vrend_create_surface(ctx, handle, res, format, val0, val1, sample_count);
 }
 
 static int vrend_decode_create_surface(struct vrend_context *ctx, const uint32_t *buf, uint32_t handle, uint16_t length)
diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
index f3a28d8..f3420f2 100644
--- a/src/vrend_renderer.c
+++ b/src/vrend_renderer.c
@@ -2252,25 +2252,13 @@ void vrend_sync_make_current(virgl_gl_context gl_cxt) {
 }
 
 int vrend_create_surface(struct vrend_context *ctx,
-                         uint32_t handle,
-                         uint32_t res_handle, uint32_t format,
-                         uint32_t val0, uint32_t val1,
+                         uint32_t handle, struct vrend_resource *res,
+                         enum virgl_formats format, uint32_t val0, uint32_t val1,
                          uint32_t nr_samples)
 {
    struct vrend_surface *surf;
-   struct vrend_resource *res;
    uint32_t ret_handle;
 
-   if (format >= PIPE_FORMAT_COUNT) {
-      return EINVAL;
-   }
-
-   res = vrend_renderer_ctx_res_lookup(ctx, res_handle);
-   if (!res) {
-      vrend_report_context_error(ctx, VIRGL_ERROR_CTX_ILLEGAL_RESOURCE, res_handle);
-      return EINVAL;
-   }
-
    surf = CALLOC_STRUCT(vrend_surface);
    if (!surf)
       return ENOMEM;
diff --git a/src/vrend_renderer.h b/src/vrend_renderer.h
index c98b72d..27f4a42 100644
--- a/src/vrend_renderer.h
+++ b/src/vrend_renderer.h
@@ -231,10 +231,9 @@ struct pipe_resource *
 vrend_renderer_resource_create(const struct vrend_renderer_resource_create_args *args,
                                void *image_eos);
 
-int vrend_create_surface(struct vrend_context *ctx,
-                         uint32_t handle,
-                         uint32_t res_handle, uint32_t format,
-                         uint32_t val0, uint32_t val1,
+int vrend_create_surface(struct vrend_context *ctx, uint32_t handle,
+                         struct vrend_resource *res,
+                         enum virgl_formats format, uint32_t val0, uint32_t val1,
                          uint32_t nr_samples);
 int vrend_create_sampler_view(struct vrend_context *ctx, uint32_t handle,
                               struct vrend_resource *res, enum virgl_formats format,
diff --git a/tests/test_virgl_cmd.c b/tests/test_virgl_cmd.c
index 7971f88..88fdeea 100644
--- a/tests/test_virgl_cmd.c
+++ b/tests/test_virgl_cmd.c
@@ -1258,6 +1258,56 @@ START_TEST(virgl_test_encode_sampler_view)
 }
 END_TEST
 
+static void test_create_surface(enum pipe_format format, int expected_error)
+{
+   struct virgl_context ctx;
+   struct virgl_resource res;
+   struct virgl_surface surf;
+   int ret;
+
+   ret = testvirgl_init_ctx_cmdbuf(&ctx);
+   ck_assert_int_eq(ret, 0);
+
+   /* init and create simple 2D resource */
+   ret = testvirgl_create_backed_simple_2d_res(&res, 1, 50, 50);
+   ck_assert_int_eq(ret, 0);
+
+   /* attach resource to context */
+   virgl_renderer_ctx_attach_resource(ctx.ctx_id, res.handle);
+
+   /* create a surface for the resource */
+   memset(&surf, 0, sizeof(surf));
+   surf.base.format = format;
+   surf.handle = 1;
+   surf.base.texture = &res.base;
+
+   ret = virgl_encoder_create_surface(&ctx, surf.handle, &res, &surf.base);
+   ck_assert_int_eq(ret, 0);
+
+   /* submit the cmd stream */
+   ret = testvirgl_ctx_send_cmdbuf(&ctx);
+   ck_assert_int_eq(ret, expected_error);
+
+   /* cleanup */
+   virgl_renderer_ctx_detach_resource(ctx.ctx_id, res.handle);
+
+   testvirgl_destroy_backed_res(&res);
+
+   testvirgl_fini_ctx_cmdbuf(&ctx);
+}
+
+START_TEST(virgl_test_create_surface_pass)
+{
+   test_create_surface(VIRGL_FORMAT_B8G8R8A8_UNORM, 0);
+}
+END_TEST
+
+START_TEST(virgl_test_create_surface_fail_format)
+{
+   test_create_surface(VIRGL_FORMAT_MAX, EINVAL);
+}
+END_TEST
+
 static Suite *virgl_init_suite(void)
 {
   Suite *s;
@@ -1277,6 +1327,8 @@ static Suite *virgl_init_suite(void)
   tcase_add_test(tc_core, virgl_decode_set_constant_buffer);
   tcase_add_test(tc_core, virgl_decode_bind_sampler_states);
   tcase_add_test(tc_core, virgl_test_encode_sampler_view);
+  tcase_add_test(tc_core, virgl_test_create_surface_pass);
+  tcase_add_test(tc_core, virgl_test_create_surface_fail_format);
 
   suite_add_tcase(s, tc_core);
   return s;