drm/nouveau/device: audit and version NVIF_CONTROL class and methods

The full object interfaces are about to be exposed to userspace, so we
need to check for any security-related issues and version the structs
to make it easier to handle any changes we may need in the future.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>

diff --git a/drivers/gpu/drm/nouveau/core/core/object.c b/drivers/gpu/drm/nouveau/core/core/object.c
index d6fea944a5a7808248cc823133536ef9cf2f0956..b08630577c820061c63d8d5eeb2015f4972acf65 100644 (file)
--- a/drivers/gpu/drm/nouveau/core/core/object.c
+++ b/drivers/gpu/drm/nouveau/core/core/object.c
@@ -58,7 +58,7 @@ nouveau_object_create_(struct nouveau_object *parent,
        return 0;
 }
 
-static int
+int
 _nouveau_object_ctor(struct nouveau_object *parent,
                     struct nouveau_object *engine,
                     struct nouveau_oclass *oclass, void *data, u32 size,

diff --git a/drivers/gpu/drm/nouveau/core/engine/device/ctrl.c b/drivers/gpu/drm/nouveau/core/engine/device/ctrl.c
index fb546f3a1af04e9f42410f9ea783dfdaeb49388c..e34101a3490ee3210c2382cba07a9393b924d843 100644 (file)
--- a/drivers/gpu/drm/nouveau/core/engine/device/ctrl.c
+++ b/drivers/gpu/drm/nouveau/core/engine/device/ctrl.c
@@ -22,59 +22,82 @@
  * Authors: Ben Skeggs <bskeggs@redhat.com>
  */
 
+#include <core/client.h>
 #include <core/object.h>
-#include <core/class.h>
+#include <nvif/unpack.h>
+#include <nvif/class.h>
+#include <nvif/ioctl.h>
 
 #include <subdev/clock.h>
 
 #include "priv.h"
 
 static int
-nouveau_control_mthd_pstate_info(struct nouveau_object *object, u32 mthd,
-                               void *data, u32 size)
+nouveau_control_mthd_pstate_info(struct nouveau_object *object,
+                                void *data, u32 size)
 {
+       union {
+               struct nvif_control_pstate_info_v0 v0;
+       } *args = data;
        struct nouveau_clock *clk = nouveau_clock(object);
-       struct nv_control_pstate_info *args = data;
+       int ret;
 
-       if (size < sizeof(*args))
-               return -EINVAL;
+       nv_ioctl(object, "control pstate info size %d\n", size);
+       if (nvif_unpack(args->v0, 0, 0, false)) {
+               nv_ioctl(object, "control pstate info vers %d\n",
+                        args->v0.version);
+       } else
+               return ret;
 
        if (clk) {
-               args->count = clk->state_nr;
-               args->ustate_ac = clk->ustate_ac;
-               args->ustate_dc = clk->ustate_dc;
-               args->pwrsrc = clk->pwrsrc;
-               args->pstate = clk->pstate;
+               args->v0.count = clk->state_nr;
+               args->v0.ustate_ac = clk->ustate_ac;
+               args->v0.ustate_dc = clk->ustate_dc;
+               args->v0.pwrsrc = clk->pwrsrc;
+               args->v0.pstate = clk->pstate;
        } else {
-               args->count = 0;
-               args->ustate_ac = NV_CONTROL_PSTATE_INFO_USTATE_DISABLE;
-               args->ustate_dc = NV_CONTROL_PSTATE_INFO_USTATE_DISABLE;
-               args->pwrsrc = -ENOSYS;
-               args->pstate = NV_CONTROL_PSTATE_INFO_PSTATE_UNKNOWN;
+               args->v0.count = 0;
+               args->v0.ustate_ac = NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE;
+               args->v0.ustate_dc = NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE;
+               args->v0.pwrsrc = -ENOSYS;
+               args->v0.pstate = NVIF_CONTROL_PSTATE_INFO_V0_PSTATE_UNKNOWN;
        }
 
        return 0;
 }
 
 static int
-nouveau_control_mthd_pstate_attr(struct nouveau_object *object, u32 mthd,
-                               void *data, u32 size)
+nouveau_control_mthd_pstate_attr(struct nouveau_object *object,
+                                void *data, u32 size)
 {
+       union {
+               struct nvif_control_pstate_attr_v0 v0;
+       } *args = data;
        struct nouveau_clock *clk = nouveau_clock(object);
-       struct nv_control_pstate_attr *args = data;
        struct nouveau_clocks *domain;
        struct nouveau_pstate *pstate;
        struct nouveau_cstate *cstate;
        int i = 0, j = -1;
        u32 lo, hi;
-
-       if ((size < sizeof(*args)) || !clk ||
-           (args->state >= 0 && args->state >= clk->state_nr))
-               return -EINVAL;
+       int ret;
+
+       nv_ioctl(object, "control pstate attr size %d\n", size);
+       if (nvif_unpack(args->v0, 0, 0, false)) {
+               nv_ioctl(object, "control pstate attr vers %d state %d "
+                                "index %d\n",
+                        args->v0.version, args->v0.state, args->v0.index);
+               if (!clk)
+                       return -ENODEV;
+               if (args->v0.state < NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT)
+                       return -EINVAL;
+               if (args->v0.state >= clk->state_nr)
+                       return -EINVAL;
+       } else
+               return ret;
        domain = clk->domains;
 
        while (domain->name != nv_clk_src_max) {
-               if (domain->mname && ++j == args->index)
+               if (domain->mname && ++j == args->v0.index)
                        break;
                domain++;
        }
@@ -82,9 +105,9 @@ nouveau_control_mthd_pstate_attr(struct nouveau_object *object, u32 mthd,
        if (domain->name == nv_clk_src_max)
                return -EINVAL;
 
-       if (args->state != NV_CONTROL_PSTATE_ATTR_STATE_CURRENT) {
+       if (args->v0.state != NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT) {
                list_for_each_entry(pstate, &clk->states, head) {
-                       if (i++ == args->state)
+                       if (i++ == args->v0.state)
                                break;
                }
 
@@ -95,21 +118,21 @@ nouveau_control_mthd_pstate_attr(struct nouveau_object *object, u32 mthd,
                        hi = max(hi, cstate->domain[domain->name]);
                }
 
-               args->state = pstate->pstate;
+               args->v0.state = pstate->pstate;
        } else {
                lo = max(clk->read(clk, domain->name), 0);
                hi = lo;
        }
 
-       snprintf(args->name, sizeof(args->name), "%s", domain->mname);
-       snprintf(args->unit, sizeof(args->unit), "MHz");
-       args->min = lo / domain->mdiv;
-       args->max = hi / domain->mdiv;
+       snprintf(args->v0.name, sizeof(args->v0.name), "%s", domain->mname);
+       snprintf(args->v0.unit, sizeof(args->v0.unit), "MHz");
+       args->v0.min = lo / domain->mdiv;
+       args->v0.max = hi / domain->mdiv;
 
-       args->index = 0;
+       args->v0.index = 0;
        while ((++domain)->name != nv_clk_src_max) {
                if (domain->mname) {
-                       args->index = ++j;
+                       args->v0.index = ++j;
                        break;
                }
        }
@@ -118,39 +141,65 @@ nouveau_control_mthd_pstate_attr(struct nouveau_object *object, u32 mthd,
 }
 
 static int
-nouveau_control_mthd_pstate_user(struct nouveau_object *object, u32 mthd,
-                               void *data, u32 size)
+nouveau_control_mthd_pstate_user(struct nouveau_object *object,
+                                void *data, u32 size)
 {
+       union {
+               struct nvif_control_pstate_user_v0 v0;
+       } *args = data;
        struct nouveau_clock *clk = nouveau_clock(object);
-       struct nv_control_pstate_user *args = data;
-       int ret = 0;
-
-       if (size < sizeof(*args) || !clk)
-               return -EINVAL;
-
-       if (args->pwrsrc >= 0) {
-               ret |= nouveau_clock_ustate(clk, args->ustate, args->pwrsrc);
+       int ret;
+
+       nv_ioctl(object, "control pstate user size %d\n", size);
+       if (nvif_unpack(args->v0, 0, 0, false)) {
+               nv_ioctl(object, "control pstate user vers %d ustate %d "
+                                "pwrsrc %d\n", args->v0.version,
+                        args->v0.ustate, args->v0.pwrsrc);
+               if (!clk)
+                       return -ENODEV;
+       } else
+               return ret;
+
+       if (args->v0.pwrsrc >= 0) {
+               ret |= nouveau_clock_ustate(clk, args->v0.ustate, args->v0.pwrsrc);
        } else {
-               ret |= nouveau_clock_ustate(clk, args->ustate, 0);
-               ret |= nouveau_clock_ustate(clk, args->ustate, 1);
+               ret |= nouveau_clock_ustate(clk, args->v0.ustate, 0);
+               ret |= nouveau_clock_ustate(clk, args->v0.ustate, 1);
        }
 
        return ret;
 }
 
+static int
+nouveau_control_mthd(struct nouveau_object *object, u32 mthd,
+                    void *data, u32 size)
+{
+       switch (mthd) {
+       case NVIF_CONTROL_PSTATE_INFO:
+               return nouveau_control_mthd_pstate_info(object, data, size);
+       case NVIF_CONTROL_PSTATE_ATTR:
+               return nouveau_control_mthd_pstate_attr(object, data, size);
+       case NVIF_CONTROL_PSTATE_USER:
+               return nouveau_control_mthd_pstate_user(object, data, size);
+       default:
+               break;
+       }
+       return -EINVAL;
+}
+
+static struct nouveau_ofuncs
+nouveau_control_ofuncs = {
+       .ctor = _nouveau_object_ctor,
+       .dtor = nouveau_object_destroy,
+       .init = nouveau_object_init,
+       .fini = nouveau_object_fini,
+       .mthd = nouveau_control_mthd,
+};
+
 struct nouveau_oclass
 nouveau_control_oclass[] = {
-       { .handle = NV_CONTROL_CLASS,
-         .ofuncs = &nouveau_object_ofuncs,
-         .omthds = (struct nouveau_omthds[]) {
-                 { NV_CONTROL_PSTATE_INFO,
-                   NV_CONTROL_PSTATE_INFO, nouveau_control_mthd_pstate_info },
-                 { NV_CONTROL_PSTATE_ATTR,
-                   NV_CONTROL_PSTATE_ATTR, nouveau_control_mthd_pstate_attr },
-                 { NV_CONTROL_PSTATE_USER,
-                   NV_CONTROL_PSTATE_USER, nouveau_control_mthd_pstate_user },
-                 {},
-         },
+       { .handle = NVIF_IOCTL_NEW_V0_CONTROL,
+         .ofuncs = &nouveau_control_ofuncs
        },
        {}
 };

diff --git a/drivers/gpu/drm/nouveau/core/include/core/class.h b/drivers/gpu/drm/nouveau/core/include/core/class.h
index 3df23606eb0268026f17cd45ba7d3f80c2091a94..79de03bdff969685e68d4b1a4a8c59a39799f45f 100644 (file)
--- a/drivers/gpu/drm/nouveau/core/include/core/class.h
+++ b/drivers/gpu/drm/nouveau/core/include/core/class.h
@@ -3,49 +3,6 @@
 
 #include <nvif/class.h>
 
-/* Device control class
- *
- * XXXX: NV_CONTROL
- */
-#define NV_CONTROL_CLASS                                             0x0000fffe
-
-#define NV_CONTROL_PSTATE_INFO                                       0x00000000
-#define NV_CONTROL_PSTATE_INFO_USTATE_DISABLE                              (-1)
-#define NV_CONTROL_PSTATE_INFO_USTATE_PERFMON                              (-2)
-#define NV_CONTROL_PSTATE_INFO_PSTATE_UNKNOWN                              (-1)
-#define NV_CONTROL_PSTATE_INFO_PSTATE_PERFMON                              (-2)
-#define NV_CONTROL_PSTATE_ATTR                                       0x00000001
-#define NV_CONTROL_PSTATE_ATTR_STATE_CURRENT                               (-1)
-#define NV_CONTROL_PSTATE_USER                                       0x00000002
-#define NV_CONTROL_PSTATE_USER_STATE_UNKNOWN                               (-1)
-#define NV_CONTROL_PSTATE_USER_STATE_PERFMON                               (-2)
-
-struct nv_control_pstate_info {
-       u32 count; /* out: number of power states */
-       s32 ustate_ac; /* out: target pstate index */
-       s32 ustate_dc; /* out: target pstate index */
-       s32 pwrsrc; /* out: current power source */
-       u32 pstate; /* out: current pstate index */
-};
-
-struct nv_control_pstate_attr {
-       s32 state; /*  in: index of pstate to query
-                   * out: pstate identifier
-                   */
-       u32 index; /*  in: index of attribute to query
-                   * out: index of next attribute, or 0 if no more
-                   */
-       char name[32];
-       char unit[16];
-       u32 min;
-       u32 max;
-};
-
-struct nv_control_pstate_user {
-       s32 ustate; /*  in: pstate identifier */
-       s32 pwrsrc; /*  in: target power source */
-};
-
 /* DMA FIFO channel classes
  *
  * 006b: NV03_CHANNEL_DMA

diff --git a/drivers/gpu/drm/nouveau/core/include/core/object.h b/drivers/gpu/drm/nouveau/core/include/core/object.h
index b22e8fd4005e0f0e8e84d2360f2e9eadcaf516cb..d7039482d6fd6cb035cba3ba4bb13cfd4a120d60 100644 (file)
--- a/drivers/gpu/drm/nouveau/core/include/core/object.h
+++ b/drivers/gpu/drm/nouveau/core/include/core/object.h
@@ -48,6 +48,10 @@ void nouveau_object_destroy(struct nouveau_object *);
 int  nouveau_object_init(struct nouveau_object *);
 int  nouveau_object_fini(struct nouveau_object *, bool suspend);
 
+int _nouveau_object_ctor(struct nouveau_object *, struct nouveau_object *,
+                        struct nouveau_oclass *, void *, u32,
+                        struct nouveau_object **);
+
 extern struct nouveau_ofuncs nouveau_object_ofuncs;
 
 /* Don't allocate dynamically, because lockdep needs lock_class_keys to be in

diff --git a/drivers/gpu/drm/nouveau/nouveau_sysfs.c b/drivers/gpu/drm/nouveau/nouveau_sysfs.c
index d14e6ef93a48e8d70659d8f0f646777f422174a5..32a23895abd5f52fe3cd2bcff82f91757daf6be7 100644 (file)
--- a/drivers/gpu/drm/nouveau/nouveau_sysfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sysfs.c
@@ -24,6 +24,7 @@
 
 #include <nvif/os.h>
 #include <nvif/class.h>
+#include <nvif/ioctl.h>
 
 #include "nouveau_sysfs.h"
 
@@ -43,25 +44,25 @@ static ssize_t
 nouveau_sysfs_pstate_get(struct device *d, struct device_attribute *a, char *b)
 {
        struct nouveau_sysfs *sysfs = nouveau_sysfs(drm_device(d));
-       struct nv_control_pstate_info info;
+       struct nvif_control_pstate_info_v0 info = {};
        size_t cnt = PAGE_SIZE;
        char *buf = b;
        int ret, i;
 
-       ret = nvif_exec(&sysfs->ctrl, NV_CONTROL_PSTATE_INFO,
+       ret = nvif_mthd(&sysfs->ctrl, NVIF_CONTROL_PSTATE_INFO,
                        &info, sizeof(info));
        if (ret)
                return ret;
 
        for (i = 0; i < info.count + 1; i++) {
                const s32 state = i < info.count ? i :
-                       NV_CONTROL_PSTATE_ATTR_STATE_CURRENT;
-               struct nv_control_pstate_attr attr = {
+                       NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT;
+               struct nvif_control_pstate_attr_v0 attr = {
                        .state = state,
                        .index = 0,
                };
 
-               ret = nvif_exec(&sysfs->ctrl, NV_CONTROL_PSTATE_ATTR,
+               ret = nvif_mthd(&sysfs->ctrl, NVIF_CONTROL_PSTATE_ATTR,
                                &attr, sizeof(attr));
                if (ret)
                        return ret;
@@ -76,7 +77,8 @@ nouveau_sysfs_pstate_get(struct device *d, struct device_attribute *a, char *b)
                attr.index = 0;
                do {
                        attr.state = state;
-                       ret = nvif_exec(&sysfs->ctrl, NV_CONTROL_PSTATE_ATTR,
+                       ret = nvif_mthd(&sysfs->ctrl,
+                                       NVIF_CONTROL_PSTATE_ATTR,
                                        &attr, sizeof(attr));
                        if (ret)
                                return ret;
@@ -112,7 +114,7 @@ nouveau_sysfs_pstate_set(struct device *d, struct device_attribute *a,
                         const char *buf, size_t count)
 {
        struct nouveau_sysfs *sysfs = nouveau_sysfs(drm_device(d));
-       struct nv_control_pstate_user args = { .pwrsrc = -EINVAL };
+       struct nvif_control_pstate_user_v0 args = { .pwrsrc = -EINVAL };
        long value, ret;
        char *tmp;
 
@@ -129,10 +131,10 @@ nouveau_sysfs_pstate_set(struct device *d, struct device_attribute *a,
        }
 
        if (!strcasecmp(buf, "none"))
-               args.ustate = NV_CONTROL_PSTATE_USER_STATE_UNKNOWN;
+               args.ustate = NVIF_CONTROL_PSTATE_USER_V0_STATE_UNKNOWN;
        else
        if (!strcasecmp(buf, "auto"))
-               args.ustate = NV_CONTROL_PSTATE_USER_STATE_PERFMON;
+               args.ustate = NVIF_CONTROL_PSTATE_USER_V0_STATE_PERFMON;
        else {
                ret = kstrtol(buf, 16, &value);
                if (ret)
@@ -140,7 +142,7 @@ nouveau_sysfs_pstate_set(struct device *d, struct device_attribute *a,
                args.ustate = value;
        }
 
-       ret = nvif_exec(&sysfs->ctrl, NV_CONTROL_PSTATE_USER,
+       ret = nvif_mthd(&sysfs->ctrl, NVIF_CONTROL_PSTATE_USER,
                        &args, sizeof(args));
        if (ret < 0)
                return ret;
@@ -179,8 +181,9 @@ nouveau_sysfs_init(struct drm_device *dev)
        if (!sysfs)
                return -ENOMEM;
 
-       ret = nvif_object_init(nvif_object(&drm->device), NULL, NVDRM_CONTROL,
-                              NV_CONTROL_CLASS, NULL, 0, &sysfs->ctrl);
+       ret = nvif_object_init(nvif_object(device), NULL, NVDRM_CONTROL,
+                              NVIF_IOCTL_NEW_V0_CONTROL, NULL, 0,
+                             &sysfs->ctrl);
        if (ret == 0)
                device_create_file(nv_device_base(nvkm_device(device)), &dev_attr_pstate);
 

diff --git a/drivers/gpu/drm/nouveau/nvif/class.h b/drivers/gpu/drm/nouveau/nvif/class.h
index decca22ea5280ac5c2e1d3bd69ac1ab5524532d0..7d6c130268557605a0ac21d3c9872d73d6544abf 100644 (file)
--- a/drivers/gpu/drm/nouveau/nvif/class.h
+++ b/drivers/gpu/drm/nouveau/nvif/class.h
@@ -185,4 +185,52 @@ struct nvif_perfctr_read_v0 {
        __u32 clk;
 };
 
+
+/*******************************************************************************
+ * device control
+ ******************************************************************************/
+
+#define NVIF_CONTROL_PSTATE_INFO                                           0x00
+#define NVIF_CONTROL_PSTATE_ATTR                                           0x01
+#define NVIF_CONTROL_PSTATE_USER                                           0x02
+
+struct nvif_control_pstate_info_v0 {
+       __u8  version;
+       __u8  count; /* out: number of power states */
+#define NVIF_CONTROL_PSTATE_INFO_V0_USTATE_DISABLE                         (-1)
+#define NVIF_CONTROL_PSTATE_INFO_V0_USTATE_PERFMON                         (-2)
+       __s8  ustate_ac; /* out: target pstate index */
+       __s8  ustate_dc; /* out: target pstate index */
+       __s8  pwrsrc; /* out: current power source */
+#define NVIF_CONTROL_PSTATE_INFO_V0_PSTATE_UNKNOWN                         (-1)
+#define NVIF_CONTROL_PSTATE_INFO_V0_PSTATE_PERFMON                         (-2)
+       __s8  pstate; /* out: current pstate index */
+       __u8  pad06[2];
+};
+
+struct nvif_control_pstate_attr_v0 {
+       __u8  version;
+#define NVIF_CONTROL_PSTATE_ATTR_V0_STATE_CURRENT                          (-1)
+       __s8  state; /*  in: index of pstate to query
+                     * out: pstate identifier
+                     */
+       __u8  index; /*  in: index of attribute to query
+                     * out: index of next attribute, or 0 if no more
+                     */
+       __u8  pad03[5];
+       __u32 min;
+       __u32 max;
+       char  name[32];
+       char  unit[16];
+};
+
+struct nvif_control_pstate_user_v0 {
+       __u8  version;
+#define NVIF_CONTROL_PSTATE_USER_V0_STATE_UNKNOWN                          (-1)
+#define NVIF_CONTROL_PSTATE_USER_V0_STATE_PERFMON                          (-2)
+       __s8  ustate; /*  in: pstate identifier */
+       __s8  pwrsrc; /*  in: target power source */
+       __u8  pad03[5];
+};
+
 #endif

diff --git a/drivers/gpu/drm/nouveau/nvif/ioctl.h b/drivers/gpu/drm/nouveau/nvif/ioctl.h
index 67a56711b18c91b7840f68b4310e189b93297c90..4cd8e323b23d7868d5fda2952ecc8207b9e151ad 100644 (file)
--- a/drivers/gpu/drm/nouveau/nvif/ioctl.h
+++ b/drivers/gpu/drm/nouveau/nvif/ioctl.h
@@ -50,6 +50,7 @@ struct nvif_ioctl_new_v0 {
        __u32 handle;
 /* these class numbers are made up by us, and not nvidia-assigned */
 #define NVIF_IOCTL_NEW_V0_PERFCTR                                    0x0000ffff
+#define NVIF_IOCTL_NEW_V0_CONTROL                                    0x0000fffe
        __u32 oclass;
        __u8  data[];           /* class data (class.h) */
 };