[Mesa-dev] [PATCH v3] virgl: Add command and flags to initiate debugging on the host (v2)

[Erik Faye-Lund]

On Wed, 2018-09-12 at 11:59 +0200, Gert Wollny wrote:
> From: Gert Wollny <gert.wollny at collabora.com>
> 
> On the host VREND_DEBUG=guestallow must be set to let the guest
> override
> the debug flags.
> 
> v2: Send flag string instead of flags, this avoids the need to keep
>     the flags in sync.
> v3: Only request host logging if the host actually understands the
> command
> 
> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>

Looks good to me!

Reviewed-by: Erik Faye-Lund <erik.faye-lund at collabora.com>

> ---
> The corresponding virglrenderer patches can be found in this MR: 
> https://gitlab.freedesktop.org/virgl/virglrenderer/merge_requests/39
> 
> Thanks for reviewing, 
> Gert 
> 
>  src/gallium/drivers/virgl/virgl_context.c  |  8 ++++++++
>  src/gallium/drivers/virgl/virgl_encode.c   | 24
> ++++++++++++++++++++++++
>  src/gallium/drivers/virgl/virgl_encode.h   |  3 +++
>  src/gallium/drivers/virgl/virgl_hw.h       |  1 +
>  src/gallium/drivers/virgl/virgl_protocol.h |  1 +
>  5 files changed, 37 insertions(+)
> 
> diff --git a/src/gallium/drivers/virgl/virgl_context.c
> b/src/gallium/drivers/virgl/virgl_context.c
> index 4511bf3b2f..96932c473d 100644
> --- a/src/gallium/drivers/virgl/virgl_context.c
> +++ b/src/gallium/drivers/virgl/virgl_context.c
> @@ -1164,6 +1164,7 @@ struct pipe_context
> *virgl_context_create(struct pipe_screen *pscreen,
>     struct virgl_context *vctx;
>     struct virgl_screen *rs = virgl_screen(pscreen);
>     vctx = CALLOC_STRUCT(virgl_context);
> +   const char *host_debug_flagstring;
>  
>     vctx->cbuf = rs->vws->cmd_buf_create(rs->vws);
>     if (!vctx->cbuf) {
> @@ -1268,6 +1269,13 @@ struct pipe_context
> *virgl_context_create(struct pipe_screen *pscreen,
>     virgl_encoder_create_sub_ctx(vctx, vctx->hw_sub_ctx_id);
>  
>     virgl_encoder_set_sub_ctx(vctx, vctx->hw_sub_ctx_id);
> +
> +   if (rs->caps.caps.v2.capability_bits &
> VIRGL_CAP_GUEST_MAY_INIT_LOG) {
> +      host_debug_flagstring = getenv("VIRGL_HOST_DEBUG");
> +      if (host_debug_flagstring)
> +         virgl_encode_host_debug_flagstring(vctx,
> host_debug_flagstring);
> +   }
> +
>     return &vctx->base;
>  fail:
>     return NULL;
> diff --git a/src/gallium/drivers/virgl/virgl_encode.c
> b/src/gallium/drivers/virgl/virgl_encode.c
> index e86d0711a5..400ba68474 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.c
> +++ b/src/gallium/drivers/virgl/virgl_encode.c
> @@ -1044,3 +1044,27 @@ int virgl_encode_texture_barrier(struct
> virgl_context *ctx,
>     virgl_encoder_write_dword(ctx->cbuf, flags);
>     return 0;
>  }
> +
> +int virgl_encode_host_debug_flagstring(struct virgl_context *ctx,
> +                                       char *flagstring)
> +{
> +   unsigned long slen = strlen(flagstring) + 1;
> +   uint32_t sslen;
> +   uint32_t string_length;
> +
> +   if (!slen)
> +      return 0;
> +
> +   if (slen > 4 * 0xffff) {
> +      debug_printf("VIRGL: host debug flag string too long, will be
> truncated\n");
> +      slen = 4 * 0xffff;
> +   }
> +
> +   sslen = (uint32_t )(slen + 3) / 4;
> +   string_length = (uint32_t)MIN2(sslen * 4, slen);
> +
> +   virgl_encoder_write_cmd_dword(ctx,
> VIRGL_CMD0(VIRGL_CCMD_SET_DEBUG_FLAGS, 0, sslen));
> +   virgl_encoder_write_block(ctx->cbuf, (uint8_t *)flagstring,
> string_length);
> +
> +   return 0;
> +}
> diff --git a/src/gallium/drivers/virgl/virgl_encode.h
> b/src/gallium/drivers/virgl/virgl_encode.h
> index 40e62d453b..80b943a6b3 100644
> --- a/src/gallium/drivers/virgl/virgl_encode.h
> +++ b/src/gallium/drivers/virgl/virgl_encode.h
> @@ -276,4 +276,7 @@ int virgl_encode_launch_grid(struct virgl_context
> *ctx,
>                               const struct pipe_grid_info
> *grid_info);
>  int virgl_encode_texture_barrier(struct virgl_context *ctx,
>                                   unsigned flags);
> +
> +int virgl_encode_host_debug_flagstring(struct virgl_context *ctx,
> +                                  char *envname);
>  #endif
> diff --git a/src/gallium/drivers/virgl/virgl_hw.h
> b/src/gallium/drivers/virgl/virgl_hw.h
> index 7736ceb935..e682c750e7 100644
> --- a/src/gallium/drivers/virgl/virgl_hw.h
> +++ b/src/gallium/drivers/virgl/virgl_hw.h
> @@ -231,6 +231,7 @@ enum virgl_formats {
>  #define VIRGL_CAP_SHADER_CLOCK         (1 << 11)
>  #define VIRGL_CAP_TEXTURE_BARRIER      (1 << 12)
>  #define VIRGL_CAP_TGSI_COMPONENTS      (1 << 13)
> +#define VIRGL_CAP_GUEST_MAY_INIT_LOG   (1 << 14)
>  
>  /* virgl bind flags - these are compatible with mesa 10.5 gallium.
>   * but are fixed, no other should be passed to virgl either.
> diff --git a/src/gallium/drivers/virgl/virgl_protocol.h
> b/src/gallium/drivers/virgl/virgl_protocol.h
> index 8d99c5ed47..3373121bf7 100644
> --- a/src/gallium/drivers/virgl/virgl_protocol.h
> +++ b/src/gallium/drivers/virgl/virgl_protocol.h
> @@ -92,6 +92,7 @@ enum virgl_context_cmd {
>     VIRGL_CCMD_SET_FRAMEBUFFER_STATE_NO_ATTACH,
>     VIRGL_CCMD_TEXTURE_BARRIER,
>     VIRGL_CCMD_SET_ATOMIC_BUFFERS,
> +   VIRGL_CCMD_SET_DEBUG_FLAGS,
>  };
>  
>  /*