| Message ID | 20190725011003.30837-6-robh@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/panfrost: Add heap and no execute buffer allocation | expand |
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@collabora.com> > Cc: Boris Brezillon <boris.brezillon@collabora.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Steven Price <steven.price@arm.com> > Acked-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> > Signed-off-by: Rob Herring <robh@kernel.org> Reviewed-by: Steven Price <steven.price@arm.com> > --- > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
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. *