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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/07/2019 02:10, 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@xxxxxxxxxxxxx>
> Cc: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
> Cc: Robin Murphy <robin.murphy@xxxxxxx>
> Cc: Steven Price <steven.price@xxxxxxx>
> Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@xxxxxxxxxxxxx>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>

Reviewed-by: Steven Price <steven.price@xxxxxxx>

> ---
> v2:
> - Rework panfrost_drm_mm_color_adjust() from Steven and Robin
> 
>  drivers/gpu/drm/panfrost/panfrost_drv.c | 30 ++++++++++++++++++++++++-
>  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, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index d354b92964d5..7ebd82d8d570 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,32 @@ static struct drm_driver panfrost_drm_driver = {
>  	.gem_prime_mmap		= drm_gem_prime_mmap,
>  };
> 
> +#define PFN_4G		(SZ_4G >> PAGE_SHIFT)
> +#define PFN_4G_MASK	(PFN_4G - 1)
> +#define PFN_16M		(SZ_16M >> 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 & PANFROST_BO_NOEXEC)) {
> +		u64 next_seg;
> +
> +		if ((*start & PFN_4G_MASK) == 0)
> +			(*start)++;
> +
> +		if ((*end & PFN_4G_MASK) == 0)
> +			(*end)--;
> +
> +		next_seg = ALIGN(*start, PFN_4G);
> +		if (next_seg - *start <= PFN_16M)
> +			*start = next_seg + 1;
> +
> +		*end = min(*end, ALIGN(*start, PFN_4G) - 1);
> +	}
> +}
> +
>  static int panfrost_probe(struct platform_device *pdev)
>  {
>  	struct panfrost_device *pfdev;
> @@ -394,6 +421,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..63731f6c5223 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;
> +	unsigned long color = bo->noexec ? PANFROST_BO_NOEXEC : 0;
> +
> +	/*
> +	 * 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, color, 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 b4ac149b2399..eba6ce785ef0 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
> @@ -190,6 +190,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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux