[Mesa-dev] [PATCH v2] nvc0: ensure that pushbuf keeps ref to old text/tls bos

[Samuel Pitoiset]

Makes sense to me.

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

On 01/05/2018 07:17 AM, Ilia Mirkin wrote:
> If we free the bo, then the PTE may get deallocated immediately. We have
> to make sure that the submission includes a ref to the old bo so that it
> remains mapped for the duration of the command execution.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> v1 -> v2:
>    Ben pointed out that not deleting the BO is not enough -- we have to
>    actually ref it to the pushbuf, otherwise it could still get kicked
>    out of VRAM.
> 
>    I happened to notice that the TLS logic suffers from a similar issue,
>    although in practice it doesn't matter since we never resize TLS.
> 
>   src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> index 02562dd4eb2..cac4bb89271 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
> @@ -741,6 +741,13 @@ nvc0_screen_resize_tls_area(struct nvc0_screen *screen,
>                           NULL, &bo);
>      if (ret)
>         return ret;
> +
> +   /* Make sure that the pushbuf has acquired a reference to the old tls
> +    * segment, as it may have commands that will reference it.
> +    */
> +   if (screen->tls)
> +      PUSH_REFN(screen->base.pushbuf, screen->tls,
> +                NV_VRAM_DOMAIN(&screen->base) | NOUVEAU_BO_RDWR);
>      nouveau_bo_ref(NULL, &screen->tls);
>      screen->tls = bo;
>      return 0;
> @@ -758,6 +765,12 @@ nvc0_screen_resize_text_area(struct nvc0_screen *screen, uint64_t size)
>      if (ret)
>         return ret;
>   
> +   /* Make sure that the pushbuf has acquired a reference to the old text
> +    * segment, as it may have commands that will reference it.
> +    */
> +   if (screen->text)
> +      PUSH_REFN(push, screen->text,
> +                NV_VRAM_DOMAIN(&screen->base) | NOUVEAU_BO_RD);
>      nouveau_bo_ref(NULL, &screen->text);
>      screen->text = bo;
>   
>