From a532da976f17234375d3b34633ff5d48f71f62bc Mon Sep 17 00:00:00 2001 From: Ben Skeggs Date: Sun, 10 Aug 2014 04:10:25 +1000 Subject: [PATCH] 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 --- drivers/gpu/drm/nouveau/core/core/object.c | 2 +- .../gpu/drm/nouveau/core/engine/device/ctrl.c | 161 ++++++++++++------ .../gpu/drm/nouveau/core/include/core/class.h | 43 ----- .../drm/nouveau/core/include/core/object.h | 4 + drivers/gpu/drm/nouveau/nouveau_sysfs.c | 27 +-- drivers/gpu/drm/nouveau/nvif/class.h | 48 ++++++ drivers/gpu/drm/nouveau/nvif/ioctl.h | 1 + 7 files changed, 174 insertions(+), 112 deletions(-) diff --git a/drivers/gpu/drm/nouveau/core/core/object.c b/drivers/gpu/drm/nouveau/core/core/object.c index d6fea944a5a7..b08630577c82 100644 --- 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 fb546f3a1af0..e34101a3490e 100644 --- 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 */ +#include #include -#include +#include +#include +#include #include #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 3df23606eb02..79de03bdff96 100644 --- 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 -/* 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 b22e8fd4005e..d7039482d6fd 100644 --- 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 d14e6ef93a48..32a23895abd5 100644 --- a/drivers/gpu/drm/nouveau/nouveau_sysfs.c +++ b/drivers/gpu/drm/nouveau/nouveau_sysfs.c @@ -24,6 +24,7 @@ #include #include +#include #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 decca22ea528..7d6c13026855 100644 --- 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 67a56711b18c..4cd8e323b23d 100644 --- 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) */ }; -- 2.30.2