Re: [PATCH 3/5] drm/panfrost: Add a no execute flag for BO allocations

[Alyssa Rosenzweig <alyssa@rosenzweig.io>]

This patch is:

	Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>

On Wed, Jul 17, 2019 at 12:33:50PM -0600, Rob Herring wrote:
> Executable buffers have an alignment restriction that they can't cross
> 16MB boundary as the GPU program counter is 24-bits. This restriction is
> currently not handled and we just get lucky. As current userspace
> assumes all BOs are executable, that has to remain the default. So add a
> new PANFROST_BO_NOEXEC flag to allow userspace to indicate which BOs are
> not executable.
> 
> There is also a restriction that executable buffers cannot start or end
> on a 4GB boundary. This is mostly avoided as there is only 4GB of space
> currently and the beginning is already blocked out for NULL ptr
> detection. Add support to handle this restriction fully regardless of
> the current constraints.
> 
> For existing userspace, all created BOs remain executable, but the GPU
> VA alignment will be increased to the size of the BO. This shouldn't
> matter as there is plenty of GPU VA space.
> 
> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Cc: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 20 +++++++++++++++++++-
>  drivers/gpu/drm/panfrost/panfrost_gem.c | 18 ++++++++++++++++--
>  drivers/gpu/drm/panfrost/panfrost_gem.h |  3 ++-
>  drivers/gpu/drm/panfrost/panfrost_mmu.c |  3 +++
>  include/uapi/drm/panfrost_drm.h         |  2 ++
>  5 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d354b92964d5..b91e991bc6a3 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -49,7 +49,8 @@ static int panfrost_ioctl_create_bo(struct drm_device *dev, void *data,
>  	struct panfrost_gem_object *bo;
>  	struct drm_panfrost_create_bo *args = data;
>  
> -	if (!args->size || args->flags || args->pad)
> +	if (!args->size || args->pad ||
> +	    (args->flags & ~PANFROST_BO_NOEXEC))
>  		return -EINVAL;
>  
>  	bo = panfrost_gem_create_with_handle(file, dev, args->size, args->flags,
> @@ -367,6 +368,22 @@ static struct drm_driver panfrost_drm_driver = {
>  	.gem_prime_mmap		= drm_gem_prime_mmap,
>  };
>  
> +#define PFN_4G_MASK    ((SZ_4G - 1) >> PAGE_SHIFT)
> +
> +static void panfrost_drm_mm_color_adjust(const struct drm_mm_node *node,
> +					 unsigned long color,
> +					 u64 *start, u64 *end)
> +{
> +	/* Executable buffers can't start or end on a 4GB boundary */
> +	if (!color) {
> +		if ((*start & PFN_4G_MASK) == 0)
> +			(*start)++;
> +
> +		if ((*end & PFN_4G_MASK) == 0)
> +			(*end)--;
> +	}
> +}
> +
>  static int panfrost_probe(struct platform_device *pdev)
>  {
>  	struct panfrost_device *pfdev;
> @@ -394,6 +411,7 @@ static int panfrost_probe(struct platform_device *pdev)
>  
>  	/* 4G enough for now. can be 48-bit */
>  	drm_mm_init(&pfdev->mm, SZ_32M >> PAGE_SHIFT, (SZ_4G - SZ_32M) >> PAGE_SHIFT);
> +	pfdev->mm.color_adjust = panfrost_drm_mm_color_adjust;
>  
>  	pm_runtime_use_autosuspend(pfdev->dev);
>  	pm_runtime_set_autosuspend_delay(pfdev->dev, 50); /* ~3 frames */
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
> index df70dcf3cb2f..37ffec8391da 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
> @@ -66,11 +66,23 @@ static int panfrost_gem_map(struct panfrost_device *pfdev, struct panfrost_gem_o
>  {
>  	int ret;
>  	size_t size = bo->base.base.size;
> -	u64 align = size >= SZ_2M ? SZ_2M >> PAGE_SHIFT : 0;
> +	u64 align;
> +
> +	/*
> +	 * Executable buffers cannot cross a 16MB boundary as the program
> +	 * counter is 24-bits. We assume executable buffers will be less than
> +	 * 16MB and aligning executable buffers to their size will avoid
> +	 * crossing a 16MB boundary.
> +	 */
> +	if (!bo->noexec)
> +		align = size >> PAGE_SHIFT;
> +	else
> +		align = size >= SZ_2M ? SZ_2M >> PAGE_SHIFT : 0;
>  
>  	spin_lock(&pfdev->mm_lock);
>  	ret = drm_mm_insert_node_generic(&pfdev->mm, &bo->node,
> -					 size >> PAGE_SHIFT, align, 0, 0);
> +					 size >> PAGE_SHIFT, align,
> +					 bo->noexec, 0);
>  	spin_unlock(&pfdev->mm_lock);
>  	if (ret)
>  		return ret;
> @@ -96,6 +108,7 @@ panfrost_gem_create_with_handle(struct drm_file *file_priv,
>  		return ERR_CAST(shmem);
>  
>  	bo = to_panfrost_bo(&shmem->base);
> +	bo->noexec = !!(flags & PANFROST_BO_NOEXEC);
>  
>  	ret = panfrost_gem_map(pfdev, bo);
>  	if (ret)
> @@ -123,6 +136,7 @@ panfrost_gem_prime_import_sg_table(struct drm_device *dev,
>  		return ERR_CAST(obj);
>  
>  	pobj = to_panfrost_bo(obj);
> +	pobj->noexec = true;
>  
>  	ret = panfrost_gem_map(pfdev, pobj);
>  	if (ret)
> diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
> index ce065270720b..132f02399b7b 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gem.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
> @@ -11,7 +11,8 @@ struct panfrost_gem_object {
>  	struct drm_gem_shmem_object base;
>  
>  	struct drm_mm_node node;
> -	bool is_mapped;
> +	bool is_mapped		:1;
> +	bool noexec		:1;
>  };
>  
>  static inline
> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> index 5383b837f04b..d18484a07bfa 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -212,6 +212,9 @@ int panfrost_mmu_map(struct panfrost_gem_object *bo)
>  	if (WARN_ON(bo->is_mapped))
>  		return 0;
>  
> +	if (bo->noexec)
> +		prot |= IOMMU_NOEXEC;
> +
>  	sgt = drm_gem_shmem_get_pages_sgt(obj);
>  	if (WARN_ON(IS_ERR(sgt)))
>  		return PTR_ERR(sgt);
> diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
> index b5d370638846..17fb5d200f7a 100644
> --- a/include/uapi/drm/panfrost_drm.h
> +++ b/include/uapi/drm/panfrost_drm.h
> @@ -82,6 +82,8 @@ struct drm_panfrost_wait_bo {
>  	__s64 timeout_ns;	/* absolute */
>  };
>  
> +#define PANFROST_BO_NOEXEC	1
> +
>  /**
>   * struct drm_panfrost_create_bo - ioctl argument for creating Panfrost BOs.
>   *
> -- 
> 2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel