[3/5] panfrost: Mark BOs as NOEXEC

Submitted by Tomeu Vizoso on Aug. 5, 2019, 3:18 p.m.

Details

Message ID 20190805151836.12293-3-tomeu.vizoso@collabora.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Tomeu Vizoso Aug. 5, 2019, 3:18 p.m.
Unless a BO has the EXECUTABLE flag, mark it as NOEXEC.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 include/drm-uapi/panfrost_drm.h           | 27 +++++++++++++++++++++++
 src/gallium/drivers/panfrost/pan_drm.c    |  7 +++++-
 src/gallium/drivers/panfrost/pan_screen.c | 12 ++++++++++
 src/gallium/drivers/panfrost/pan_screen.h |  4 ++++
 4 files changed, 49 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/include/drm-uapi/panfrost_drm.h b/include/drm-uapi/panfrost_drm.h
index a52e0283b90d..9150dd75aad8 100644
--- a/include/drm-uapi/panfrost_drm.h
+++ b/include/drm-uapi/panfrost_drm.h
@@ -18,6 +18,8 @@  extern "C" {
 #define DRM_PANFROST_MMAP_BO			0x03
 #define DRM_PANFROST_GET_PARAM			0x04
 #define DRM_PANFROST_GET_BO_OFFSET		0x05
+#define DRM_PANFROST_PERFCNT_ENABLE		0x06
+#define DRM_PANFROST_PERFCNT_DUMP		0x07
 
 #define DRM_IOCTL_PANFROST_SUBMIT		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_SUBMIT, struct drm_panfrost_submit)
 #define DRM_IOCTL_PANFROST_WAIT_BO		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_WAIT_BO, struct drm_panfrost_wait_bo)
@@ -26,6 +28,15 @@  extern "C" {
 #define DRM_IOCTL_PANFROST_GET_PARAM		DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_PARAM, struct drm_panfrost_get_param)
 #define DRM_IOCTL_PANFROST_GET_BO_OFFSET	DRM_IOWR(DRM_COMMAND_BASE + DRM_PANFROST_GET_BO_OFFSET, struct drm_panfrost_get_bo_offset)
 
+/*
+ * Unstable ioctl(s): only exposed when the unsafe unstable_ioctls module
+ * param is set to true.
+ * All these ioctl(s) are subject to deprecation, so please don't rely on
+ * them for anything but debugging purpose.
+ */
+#define DRM_IOCTL_PANFROST_PERFCNT_ENABLE	DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_ENABLE, struct drm_panfrost_perfcnt_enable)
+#define DRM_IOCTL_PANFROST_PERFCNT_DUMP		DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump)
+
 #define PANFROST_JD_REQ_FS (1 << 0)
 /**
  * struct drm_panfrost_submit - ioctl argument for submitting commands to the 3D
@@ -71,6 +82,9 @@  struct drm_panfrost_wait_bo {
 	__s64 timeout_ns;	/* absolute */
 };
 
+#define PANFROST_BO_NOEXEC	1
+#define PANFROST_BO_HEAP	2
+
 /**
  * struct drm_panfrost_create_bo - ioctl argument for creating Panfrost BOs.
  *
@@ -135,6 +149,19 @@  struct drm_panfrost_get_bo_offset {
 	__u64 offset;
 };
 
+struct drm_panfrost_perfcnt_enable {
+	__u32 enable;
+	/*
+	 * On bifrost we have 2 sets of counters, this parameter defines the
+	 * one to track.
+	 */
+	__u32 counterset;
+};
+
+struct drm_panfrost_perfcnt_dump {
+	__u64 buf_ptr;
+};
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/src/gallium/drivers/panfrost/pan_drm.c b/src/gallium/drivers/panfrost/pan_drm.c
index 8ae541ae11b6..a3f35aed4d0f 100644
--- a/src/gallium/drivers/panfrost/pan_drm.c
+++ b/src/gallium/drivers/panfrost/pan_drm.c
@@ -93,7 +93,12 @@  panfrost_drm_create_bo(struct panfrost_screen *screen, size_t size,
 
         unsigned translated_flags = 0;
 
-        /* TODO: translate flags to kernel flags, if the kernel supports */
+        if (screen->kernel_version >= 1) {
+                //if (flags & PAN_ALLOCATE_GROWABLE)
+                //        translated_flags |= PANFROST_BO_HEAP;
+                if (!(flags & PAN_ALLOCATE_EXECUTE))
+                        translated_flags |= PANFROST_BO_NOEXEC;
+        }
 
         struct drm_panfrost_create_bo create_bo = {
                 .size = size,
diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c
index 5b51faa7f566..e5fc321a8802 100644
--- a/src/gallium/drivers/panfrost/pan_screen.c
+++ b/src/gallium/drivers/panfrost/pan_screen.c
@@ -584,6 +584,7 @@  struct pipe_screen *
 panfrost_create_screen(int fd, struct renderonly *ro)
 {
         struct panfrost_screen *screen = rzalloc(NULL, struct panfrost_screen);
+	drmVersionPtr version;
 
         pan_debug = debug_get_option_pan_debug();
 
@@ -618,6 +619,17 @@  panfrost_create_screen(int fd, struct renderonly *ro)
                 return NULL;
         }
 
+	version = drmGetVersion(fd);
+	if (version->version_major != 1) {
+                debug_printf("panfrost: Unsupported version: %u.%u.%u",
+                             version->version_major, version->version_minor,
+                             version->version_patchlevel);
+        	drmFreeVersion(version);
+                return NULL;
+        }
+        screen->kernel_version = version->version_minor;
+	drmFreeVersion(version);
+
         util_dynarray_init(&screen->transient_bo, screen);
 
         for (unsigned i = 0; i < ARRAY_SIZE(screen->bo_cache); ++i)
diff --git a/src/gallium/drivers/panfrost/pan_screen.h b/src/gallium/drivers/panfrost/pan_screen.h
index 22cd7dcc9fea..0b355ed8fc53 100644
--- a/src/gallium/drivers/panfrost/pan_screen.h
+++ b/src/gallium/drivers/panfrost/pan_screen.h
@@ -29,6 +29,7 @@ 
 #ifndef PAN_SCREEN_H
 #define PAN_SCREEN_H
 
+#include <drm-uapi/drm.h>
 #include "pipe/p_screen.h"
 #include "pipe/p_defines.h"
 #include "renderonly/renderonly.h"
@@ -99,6 +100,8 @@  struct panfrost_screen {
         unsigned gpu_id;
         bool require_sfbd;
 
+        int kernel_version;
+
         struct renderonly *ro;
 
         /* Transient memory management is based on borrowing fixed-size slabs
@@ -180,6 +183,7 @@  panfrost_drm_fence_finish(struct pipe_screen *pscreen,
                           struct pipe_context *ctx,
                           struct pipe_fence_handle *fence,
                           uint64_t timeout);
+
 struct panfrost_bo *
 panfrost_bo_cache_fetch(
                 struct panfrost_screen *screen,

Comments

> +        if (screen->kernel_version >= 1) {

Maybe have some #defines for kernel versions instead of magic numbers?
Also, maybe make it clear that this is a minor version -- what does
happen if we bump the major version at some point...?

>  panfrost_create_screen(int fd, struct renderonly *ro)
>  {
>          struct panfrost_screen *screen = rzalloc(NULL, struct panfrost_screen);
> +	drmVersionPtr version;
>  
>          pan_debug = debug_get_option_pan_debug();
>  
> @@ -618,6 +619,17 @@ panfrost_create_screen(int fd, struct renderonly *ro)
>                  return NULL;
>          }
>  
> +	version = drmGetVersion(fd);
> +	if (version->version_major != 1) {
> +                debug_printf("panfrost: Unsupported version: %u.%u.%u",
> +                             version->version_major, version->version_minor,
> +                             version->version_patchlevel);
> +        	drmFreeVersion(version);
> +                return NULL;
> +        }
> +        screen->kernel_version = version->version_minor;
> +	drmFreeVersion(version);
> +
>          util_dynarray_init(&screen->transient_bo, screen);
>  

I don't really care *too* much but spacing is all over the place.
On Mon, 5 Aug 2019 at 19:06, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> > +        if (screen->kernel_version >= 1) {
>
> Maybe have some #defines for kernel versions instead of magic numbers?
> Also, maybe make it clear that this is a minor version -- what does
> happen if we bump the major version at some point...?

In that case we would need to change Mesa to also take into account
the major field. I cannot see why we would do it, though.

> >  panfrost_create_screen(int fd, struct renderonly *ro)
> >  {
> >          struct panfrost_screen *screen = rzalloc(NULL, struct panfrost_screen);
> > +     drmVersionPtr version;
> >
> >          pan_debug = debug_get_option_pan_debug();
> >
> > @@ -618,6 +619,17 @@ panfrost_create_screen(int fd, struct renderonly *ro)
> >                  return NULL;
> >          }
> >
> > +     version = drmGetVersion(fd);
> > +     if (version->version_major != 1) {
> > +                debug_printf("panfrost: Unsupported version: %u.%u.%u",
> > +                             version->version_major, version->version_minor,
> > +                             version->version_patchlevel);
> > +             drmFreeVersion(version);
> > +                return NULL;
> > +        }
> > +        screen->kernel_version = version->version_minor;
> > +     drmFreeVersion(version);
> > +
> >          util_dynarray_init(&screen->transient_bo, screen);
> >
>
> I don't really care *too* much but spacing is all over the place.

Will check before v2.

Thanks,

Tomeu