| Message ID | 20240212211548.1094496-2-ttabi@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | command-line registry and gsp-rm logging | expand |
On 2/12/24 22:15, Timur Tabi wrote: > Add the NVreg_RegistryDwords command line parameter, which allows > specifying additional registry keys to be sent to GSP-RM. This > allows additional configuration, debugging, and experimentation > with GSP-RM, which uses these keys to alter its behavior. > > Note that these keys are passed as-is to GSP-RM, and Nouveau does > not parse them. This is in contrast to the Nvidia driver, which may > parse some of the keys to configure some functionality in concert with > GSP-RM. Therefore, any keys which also require action by the driver > may not function correctly when passed by Nouveau. Caveat emptor. > > The name and format of NVreg_RegistryDwords is the same as used by > the Nvidia driver, to maintain compatibility. > > Signed-off-by: Timur Tabi <ttabi@nvidia.com> > --- > v3: rebased to drm-next > > .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 6 + > .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 279 ++++++++++++++++-- > 2 files changed, 261 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h > index 6f5d376d8fcc..3fbc57b16a05 100644 > --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h > +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h > @@ -211,6 +211,12 @@ struct nvkm_gsp { > struct mutex mutex;; > struct idr idr; > } client_id; > + > + /* A linked list of registry items. The registry RPC will be built from it. */ > + struct list_head registry_list; > + > + /* The size of the registry RPC */ > + size_t registry_rpc_size; > }; > > static inline bool > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c > index 1c46e3f919be..86b62c7e1229 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c > @@ -54,6 +54,8 @@ > #include <nvrm/535.113.01/nvidia/kernel/inc/vgpu/rpc_global_enums.h> > > #include <linux/acpi.h> > +#include <linux/ctype.h> > +#include <linux/parser.h> > > #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE > #define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16 > @@ -1082,51 +1084,280 @@ r535_gsp_rpc_unloading_guest_driver(struct nvkm_gsp *gsp, bool suspend) > return nvkm_gsp_rpc_wr(gsp, rpc, true); > } > > +struct registry_list_entry { > + struct list_head list; Nit: 'head' or 'entry' might be more suitable. > + size_t name_len; > + u32 type; I prefer to represent type as enum or use a define for '1 = integer' instead. This also gets you rid of the need to explicitly explain it's meaning in the documentation of add_registry(). > + u32 data; > + u32 length; > + char name[]; > +}; > + > +/** > + * add_registry -- adds a registry entry > + * @gsp: gsp pointer > + * @name: name of the registry key > + * @type: type of data (1 = integer) > + * @data: value > + * @length: size of data, in bytes > + * > + * Adds a registry key/value pair to the registry database. > + * > + * Currently, only 32-bit integers (type == 1, length == 4) are supported. What if we pass something else? This function doesn't seem to ensure nothing else is accepted. Although I recognize that it's only used by add_registry_num() enforcing this limitation by it's function signature. > + * > + * This function collects the registry information in a linked list. After > + * all registry keys have been added, build_registry() is used to create the > + * RPC data structure. > + * > + * registry_rpc_size is a running total of the size of all registry keys. > + * It's used to avoid an O(n) calculation of the size when the RPC is built. > + * > + * Returns 0 on success, or negative error code on error. > + */ > +static int add_registry(struct nvkm_gsp *gsp, const char *name, u32 type, u32 data, u32 length) > +{ > + struct registry_list_entry *reg; > + size_t nlen = strlen(name) + 1; > + > + /* Set an arbitrary limit to avoid problems with broken command lines */ > + if (strlen(name) > 64) Could re-use nlen for this check. > + return -EFBIG; This error code doesn't seem to apply here, prefer EINVAL. > + > + reg = kmalloc(sizeof(struct registry_list_entry) + nlen, GFP_KERNEL); > + if (!reg) > + return -ENOMEM; > + > + memcpy(reg->name, name, nlen); > + reg->name_len = nlen; > + reg->type = type; > + reg->data = data; > + reg->length = length; > + > + nvkm_debug(&gsp->subdev, "adding GSP-RM registry '%s=%u'\n", name, data); > + list_add_tail(®->list, &gsp->registry_list); > + gsp->registry_rpc_size += sizeof(PACKED_REGISTRY_ENTRY) + nlen; > + > + return 0; > +} > + > +static int add_registry_num(struct nvkm_gsp *gsp, const char *name, u32 value) > +{ > + return add_registry(gsp, name, 1, value, sizeof(u32)); > +} > + > +/** > + * build_registry -- create the registry RPC data > + * @gsp: gsp pointer > + * @registry: pointer to the RPC payload to fill > + * > + * After all registry key/value pairs have been added, call this function to > + * build the RPC. > + * > + * The registry RPC looks like this: > + * > + * +-----------------+ > + * |NvU32 size; | > + * |NvU32 numEntries;| > + * +-----------------+ > + * +---------------------+ > + * |PACKED_REGISTRY_ENTRY| > + * +---------------------+ > + * |PACKED_REGISTRY_ENTRY| > + * +---------------------+ > + * ... (one copy for each entry) > + * > + * +----------------------------------+ > + * |Null-terminated string for entry 0| > + * +----------------------------------+ > + * |Null-terminated string for entry 1| > + * +----------------------------------+ > + * ... (one copy for each entry) > + * > + * All memory allocated by add_registry() is released. > + */ > +static void build_registry(struct nvkm_gsp *gsp, PACKED_REGISTRY_TABLE *registry) > +{ > + struct registry_list_entry *reg, *n; > + size_t str_offset; > + unsigned int i = 0; > + > + registry->numEntries = list_count_nodes(&gsp->registry_list); > + str_offset = struct_size(registry, entries, registry->numEntries); > + > + list_for_each_entry_safe(reg, n, &gsp->registry_list, list) { > + registry->entries[i].type = reg->type; > + registry->entries[i].data = reg->data; > + registry->entries[i].length = reg->length; > + registry->entries[i].nameOffset = str_offset; > + memcpy((void *)registry + str_offset, reg->name, reg->name_len); > + str_offset += reg->name_len; > + i++; > + > + list_del(®->list); > + kfree(reg); > + } > + > + /* Double-check that we calculated the sizes correctly */ > + WARN_ON(gsp->registry_rpc_size != str_offset); > + > + registry->size = gsp->registry_rpc_size; > +} > + > +/** > + * clean_registry -- clean up registry memory in case of error > + * @gsp: gsp pointer > + * > + * Call this function to clean up all memory allocated by add_registry() > + * in case of error and build_registry() is not called. > + */ > +static void clean_registry(struct nvkm_gsp *gsp) > +{ > + struct registry_list_entry *reg, *n; > + > + list_for_each_entry_safe(reg, n, &gsp->registry_list, list) { > + list_del(®->list); > + kfree(reg); > + } > + > + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE); > +} > + > +MODULE_PARM_DESC(NVreg_RegistryDwords, > + "A semicolon-separated list of key=integer pairs of GSP-RM registry keys"); > +static char *NVreg_RegistryDwords; > +module_param(NVreg_RegistryDwords, charp, 0400); > + > /* dword only */ > struct nv_gsp_registry_entries { > const char *name; > u32 value; > }; > > +/** > + * r535_registry_entries - required registry entries for GSP-RM > + * > + * This array lists registry entries that are required for GSP-RM to > + * function correctly. > + * > + * RMSecBusResetEnable - enables PCI secondary bus reset > + * RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration > + * registers on any PCI reset. > + */ > static const struct nv_gsp_registry_entries r535_registry_entries[] = { > { "RMSecBusResetEnable", 1 }, > { "RMForcePcieConfigSave", 1 }, > }; > #define NV_GSP_REG_NUM_ENTRIES ARRAY_SIZE(r535_registry_entries) > > +/** > + * strip - strips all characters in 'reject' from 's' > + * @s: string to strip > + * @reject: string of characters to remove > + * > + * 's' is modified. > + * > + * Returns the length of the new string. > + */ > +static size_t strip(char *s, const char *reject) > +{ > + char *p = s, *p2 = s; > + size_t length = 0; > + char c; > + > + do { > + while ((c = *p2) && strchr(reject, c)) > + p2++; > + > + *p++ = c = *p2++; > + length++; > + } while (c); > + > + return length; > +} > + > +/** > + * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM > + * @gsp: gsp pointer > + * > + * The GSP-RM registry is a set of key/value pairs that configure some aspects > + * of GSP-RM. The keys are strings, and the values are 32-bit integers. > + * > + * The registry is built from a combination of a static hard-coded list (see > + * above) and entries passed on the driver's command line. > + */ > static int > r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp) > { > PACKED_REGISTRY_TABLE *rpc; > - char *strings; > - int str_offset; > - int i; > - size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES); > + unsigned int i; > + int ret; > > - /* add strings + null terminator */ > - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) > - rpc_size += strlen(r535_registry_entries[i].name) + 1; > + INIT_LIST_HEAD(&gsp->registry_list); > + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE); > > - rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size); > - if (IS_ERR(rpc)) > - return PTR_ERR(rpc); > + /* Add the required registry entries first */ > + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { > + ret = add_registry_num(gsp, r535_registry_entries[i].name, > + r535_registry_entries[i].value); > + if (ret) { > + clean_registry(gsp); > + return ret; > + } > + } > + > + /* > + * The NVreg_RegistryDwords parameter is a string of key=value > + * pairs separated by semicolons. We need to extract and trim each > + * substring, and then parse the substring to extract the key and > + * value. > + */ > + if (NVreg_RegistryDwords) { > + char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL); > + char *start, *next = p, *equal; > + size_t length; > + > + /* Remove any whitespace from the parameter string */ > + length = strip(p, " \t\n"); > + > + while ((start = strsep(&next, ";"))) { > + long value; > + > + equal = strchr(start, '='); > + if (!equal || (equal == start) || !isdigit(equal[1])) { > + nvkm_error(&gsp->subdev, > + "ignoring invalid registry string '%s'\n", start); > + continue; > + } > > - rpc->numEntries = NV_GSP_REG_NUM_ENTRIES; > + ret = kstrtol(equal + 1, 0, &value); > + if (ret) { > + nvkm_error(&gsp->subdev, > + "ignoring invalid registry value in '%s'\n", start); > + continue; > + } At a first glance, the string parsing looks correct to me. Did you test with invalid strings as well? > > - str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]); > - strings = (char *)&rpc->entries[NV_GSP_REG_NUM_ENTRIES]; > - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { > - int name_len = strlen(r535_registry_entries[i].name) + 1; > - > - rpc->entries[i].nameOffset = str_offset; > - rpc->entries[i].type = 1; > - rpc->entries[i].data = r535_registry_entries[i].value; > - rpc->entries[i].length = 4; > - memcpy(strings, r535_registry_entries[i].name, name_len); > - strings += name_len; > - str_offset += name_len; > + /* Truncate the key=value string to just key */ > + *equal = 0; What's the purpose for that? > + > + ret = add_registry_num(gsp, start, value); > + if (ret) { > + nvkm_error(&gsp->subdev, > + "ignoring invalid registry key/value '%s=%lu'\n", > + start, value); > + continue; > + } > + } > + > + kfree(p); > } > - rpc->size = str_offset; > + > + rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size); > + if (IS_ERR(rpc)) { > + clean_registry(gsp); > + return PTR_ERR(rpc); > + } > + > + build_registry(gsp, rpc); > > return nvkm_gsp_rpc_wr(gsp, rpc, false); > }
On Tue, 2024-02-13 at 16:43 +0100, Danilo Krummrich wrote: > > +struct registry_list_entry { > > + struct list_head list; > > Nit: 'head' or 'entry' might be more suitable. Will fix in v4. > > > + size_t name_len; > > + u32 type; > > I prefer to represent type as enum or use a define for '1 = integer' instead. > This also gets you rid of the need to explicitly explain it's meaning in the > documentation of add_registry(). Will fix in v4. > > > + u32 data; > > + u32 length; > > + char name[]; > > +}; > > + > > +/** > > + * add_registry -- adds a registry entry > > + * @gsp: gsp pointer > > + * @name: name of the registry key > > + * @type: type of data (1 = integer) > > + * @data: value > > + * @length: size of data, in bytes > > + * > > + * Adds a registry key/value pair to the registry database. > > + * > > + * Currently, only 32-bit integers (type == 1, length == 4) are supported. > > What if we pass something else? This function doesn't seem to ensure nothing > else is accepted. Although I recognize that it's only used by add_registry_num() > enforcing this limitation by it's function signature. GSP-RM technically supports two other types: binary data and strings. For example, apparently it's possible to override the VBIOS itself with a registry key. However, I'm not familiar with any actual non-numeric registry keys that the user would set. I can add support for binary data and strings. > > > + * > > + * This function collects the registry information in a linked list. After > > + * all registry keys have been added, build_registry() is used to create the > > + * RPC data structure. > > + * > > + * registry_rpc_size is a running total of the size of all registry keys. > > + * It's used to avoid an O(n) calculation of the size when the RPC is built. > > + * > > + * Returns 0 on success, or negative error code on error. > > + */ > > +static int add_registry(struct nvkm_gsp *gsp, const char *name, u32 type, u32 data, u32 length) > > +{ > > + struct registry_list_entry *reg; > > + size_t nlen = strlen(name) + 1; > > + > > + /* Set an arbitrary limit to avoid problems with broken command lines */ > > + if (strlen(name) > 64) > > Could re-use nlen for this check. Will fix in v4. > > > + return -EFBIG; > > This error code doesn't seem to apply here, prefer EINVAL. You don't like creative usage of error codes? > > + while ((start = strsep(&next, ";"))) { > > + long value; > > + > > + equal = strchr(start, '='); > > + if (!equal || (equal == start) || !isdigit(equal[1])) { > > + nvkm_error(&gsp->subdev, > > + "ignoring invalid registry string '%s'\n", start); > > + continue; > > + } > > > > - rpc->numEntries = NV_GSP_REG_NUM_ENTRIES; > > + ret = kstrtol(equal + 1, 0, &value); > > + if (ret) { > > + nvkm_error(&gsp->subdev, > > + "ignoring invalid registry value in '%s'\n", start); > > + continue; > > + } > > At a first glance, the string parsing looks correct to me. Did you test with invalid > strings as well? Yes. It would be nice if there were a regex parser in the kernel, but I think I did a good job testing and rejecting strings. > > - str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]); > > - strings = (char *)&rpc->entries[NV_GSP_REG_NUM_ENTRIES]; > > - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { > > - int name_len = strlen(r535_registry_entries[i].name) + 1; > > - > > - rpc->entries[i].nameOffset = str_offset; > > - rpc->entries[i].type = 1; > > - rpc->entries[i].data = r535_registry_entries[i].value; > > - rpc->entries[i].length = 4; > > - memcpy(strings, r535_registry_entries[i].name, name_len); > > - strings += name_len; > > - str_offset += name_len; > > + /* Truncate the key=value string to just key */ > > + *equal = 0; > > What's the purpose for that? Take for example, this parameter passed to Nouveau: NVreg_RegistryDwords="RM1457588=1;TEST=53" The strsep() call points 'next' to "RM1457588=1", replacing the ; with \0. 'equal' then points to the '='. kstrtol() converts the "1" to 1. So all that's left is extracting the "RM1457588" into its own string. That's what "*equal = 0" does.
Hi Timur, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-intel/for-linux-next-fixes] [also build test WARNING on drm-tip/drm-tip linus/master v6.8-rc4 next-20240213] [cannot apply to drm-misc/drm-misc-next drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Timur-Tabi/nouveau-add-command-line-GSP-RM-registry-support/20240213-051852 base: git://anongit.freedesktop.org/drm-intel for-linux-next-fixes patch link: https://lore.kernel.org/r/20240212211548.1094496-2-ttabi%40nvidia.com patch subject: [PATCH 1/2] [v3] nouveau: add command-line GSP-RM registry support config: parisc-defconfig (https://download.01.org/0day-ci/archive/20240214/202402141030.X3uj4YDf-lkp@intel.com/config) compiler: hppa-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240214/202402141030.X3uj4YDf-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402141030.X3uj4YDf-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c: In function 'r535_gsp_rpc_set_registry': >> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1317:24: warning: variable 'length' set but not used [-Wunused-but-set-variable] 1317 | size_t length; | ^~~~~~ -- >> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1246: warning: cannot understand function prototype: 'const struct nv_gsp_registry_entries r535_registry_entries[] = ' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1673: warning: Function parameter or struct member 'priv' not described in 'r535_gsp_msg_run_cpu_sequencer' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1673: warning: Function parameter or struct member 'fn' not described in 'r535_gsp_msg_run_cpu_sequencer' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1673: warning: Function parameter or struct member 'repv' not described in 'r535_gsp_msg_run_cpu_sequencer' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1673: warning: Function parameter or struct member 'repc' not described in 'r535_gsp_msg_run_cpu_sequencer' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:2046: warning: Function parameter or struct member 'gsp' not described in 'r535_gsp_libos_init' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:2186: warning: Function parameter or struct member 'gsp' not described in 'nvkm_gsp_radix3_sg' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:2186: warning: Function parameter or struct member 'sgt' not described in 'nvkm_gsp_radix3_sg' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:2186: warning: Function parameter or struct member 'size' not described in 'nvkm_gsp_radix3_sg' drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:2186: warning: Function parameter or struct member 'rx3' not described in 'nvkm_gsp_radix3_sg' vim +/length +1317 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c 1235 1236 /** 1237 * r535_registry_entries - required registry entries for GSP-RM 1238 * 1239 * This array lists registry entries that are required for GSP-RM to 1240 * function correctly. 1241 * 1242 * RMSecBusResetEnable - enables PCI secondary bus reset 1243 * RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration 1244 * registers on any PCI reset. 1245 */ > 1246 static const struct nv_gsp_registry_entries r535_registry_entries[] = { 1247 { "RMSecBusResetEnable", 1 }, 1248 { "RMForcePcieConfigSave", 1 }, 1249 }; 1250 #define NV_GSP_REG_NUM_ENTRIES ARRAY_SIZE(r535_registry_entries) 1251 1252 /** 1253 * strip - strips all characters in 'reject' from 's' 1254 * @s: string to strip 1255 * @reject: string of characters to remove 1256 * 1257 * 's' is modified. 1258 * 1259 * Returns the length of the new string. 1260 */ 1261 static size_t strip(char *s, const char *reject) 1262 { 1263 char *p = s, *p2 = s; 1264 size_t length = 0; 1265 char c; 1266 1267 do { 1268 while ((c = *p2) && strchr(reject, c)) 1269 p2++; 1270 1271 *p++ = c = *p2++; 1272 length++; 1273 } while (c); 1274 1275 return length; 1276 } 1277 1278 /** 1279 * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM 1280 * @gsp: gsp pointer 1281 * 1282 * The GSP-RM registry is a set of key/value pairs that configure some aspects 1283 * of GSP-RM. The keys are strings, and the values are 32-bit integers. 1284 * 1285 * The registry is built from a combination of a static hard-coded list (see 1286 * above) and entries passed on the driver's command line. 1287 */ 1288 static int 1289 r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp) 1290 { 1291 PACKED_REGISTRY_TABLE *rpc; 1292 unsigned int i; 1293 int ret; 1294 1295 INIT_LIST_HEAD(&gsp->registry_list); 1296 gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE); 1297 1298 /* Add the required registry entries first */ 1299 for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { 1300 ret = add_registry_num(gsp, r535_registry_entries[i].name, 1301 r535_registry_entries[i].value); 1302 if (ret) { 1303 clean_registry(gsp); 1304 return ret; 1305 } 1306 } 1307 1308 /* 1309 * The NVreg_RegistryDwords parameter is a string of key=value 1310 * pairs separated by semicolons. We need to extract and trim each 1311 * substring, and then parse the substring to extract the key and 1312 * value. 1313 */ 1314 if (NVreg_RegistryDwords) { 1315 char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL); 1316 char *start, *next = p, *equal; > 1317 size_t length; 1318 1319 /* Remove any whitespace from the parameter string */ 1320 length = strip(p, " \t\n"); 1321 1322 while ((start = strsep(&next, ";"))) { 1323 long value; 1324 1325 equal = strchr(start, '='); 1326 if (!equal || (equal == start) || !isdigit(equal[1])) { 1327 nvkm_error(&gsp->subdev, 1328 "ignoring invalid registry string '%s'\n", start); 1329 continue; 1330 } 1331 1332 ret = kstrtol(equal + 1, 0, &value); 1333 if (ret) { 1334 nvkm_error(&gsp->subdev, 1335 "ignoring invalid registry value in '%s'\n", start); 1336 continue; 1337 } 1338 1339 /* Truncate the key=value string to just key */ 1340 *equal = 0; 1341 1342 ret = add_registry_num(gsp, start, value); 1343 if (ret) { 1344 nvkm_error(&gsp->subdev, 1345 "ignoring invalid registry key/value '%s=%lu'\n", 1346 start, value); 1347 continue; 1348 } 1349 } 1350 1351 kfree(p); 1352 } 1353 1354 rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size); 1355 if (IS_ERR(rpc)) { 1356 clean_registry(gsp); 1357 return PTR_ERR(rpc); 1358 } 1359 1360 build_registry(gsp, rpc); 1361 1362 return nvkm_gsp_rpc_wr(gsp, rpc, false); 1363 } 1364
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h index 6f5d376d8fcc..3fbc57b16a05 100644 --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h @@ -211,6 +211,12 @@ struct nvkm_gsp { struct mutex mutex;; struct idr idr; } client_id; + + /* A linked list of registry items. The registry RPC will be built from it. */ + struct list_head registry_list; + + /* The size of the registry RPC */ + size_t registry_rpc_size; }; static inline bool diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c index 1c46e3f919be..86b62c7e1229 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c @@ -54,6 +54,8 @@ #include <nvrm/535.113.01/nvidia/kernel/inc/vgpu/rpc_global_enums.h> #include <linux/acpi.h> +#include <linux/ctype.h> +#include <linux/parser.h> #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE #define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16 @@ -1082,51 +1084,280 @@ r535_gsp_rpc_unloading_guest_driver(struct nvkm_gsp *gsp, bool suspend) return nvkm_gsp_rpc_wr(gsp, rpc, true); } +struct registry_list_entry { + struct list_head list; + size_t name_len; + u32 type; + u32 data; + u32 length; + char name[]; +}; + +/** + * add_registry -- adds a registry entry + * @gsp: gsp pointer + * @name: name of the registry key + * @type: type of data (1 = integer) + * @data: value + * @length: size of data, in bytes + * + * Adds a registry key/value pair to the registry database. + * + * Currently, only 32-bit integers (type == 1, length == 4) are supported. + * + * This function collects the registry information in a linked list. After + * all registry keys have been added, build_registry() is used to create the + * RPC data structure. + * + * registry_rpc_size is a running total of the size of all registry keys. + * It's used to avoid an O(n) calculation of the size when the RPC is built. + * + * Returns 0 on success, or negative error code on error. + */ +static int add_registry(struct nvkm_gsp *gsp, const char *name, u32 type, u32 data, u32 length) +{ + struct registry_list_entry *reg; + size_t nlen = strlen(name) + 1; + + /* Set an arbitrary limit to avoid problems with broken command lines */ + if (strlen(name) > 64) + return -EFBIG; + + reg = kmalloc(sizeof(struct registry_list_entry) + nlen, GFP_KERNEL); + if (!reg) + return -ENOMEM; + + memcpy(reg->name, name, nlen); + reg->name_len = nlen; + reg->type = type; + reg->data = data; + reg->length = length; + + nvkm_debug(&gsp->subdev, "adding GSP-RM registry '%s=%u'\n", name, data); + list_add_tail(®->list, &gsp->registry_list); + gsp->registry_rpc_size += sizeof(PACKED_REGISTRY_ENTRY) + nlen; + + return 0; +} + +static int add_registry_num(struct nvkm_gsp *gsp, const char *name, u32 value) +{ + return add_registry(gsp, name, 1, value, sizeof(u32)); +} + +/** + * build_registry -- create the registry RPC data + * @gsp: gsp pointer + * @registry: pointer to the RPC payload to fill + * + * After all registry key/value pairs have been added, call this function to + * build the RPC. + * + * The registry RPC looks like this: + * + * +-----------------+ + * |NvU32 size; | + * |NvU32 numEntries;| + * +-----------------+ + * +---------------------+ + * |PACKED_REGISTRY_ENTRY| + * +---------------------+ + * |PACKED_REGISTRY_ENTRY| + * +---------------------+ + * ... (one copy for each entry) + * + * +----------------------------------+ + * |Null-terminated string for entry 0| + * +----------------------------------+ + * |Null-terminated string for entry 1| + * +----------------------------------+ + * ... (one copy for each entry) + * + * All memory allocated by add_registry() is released. + */ +static void build_registry(struct nvkm_gsp *gsp, PACKED_REGISTRY_TABLE *registry) +{ + struct registry_list_entry *reg, *n; + size_t str_offset; + unsigned int i = 0; + + registry->numEntries = list_count_nodes(&gsp->registry_list); + str_offset = struct_size(registry, entries, registry->numEntries); + + list_for_each_entry_safe(reg, n, &gsp->registry_list, list) { + registry->entries[i].type = reg->type; + registry->entries[i].data = reg->data; + registry->entries[i].length = reg->length; + registry->entries[i].nameOffset = str_offset; + memcpy((void *)registry + str_offset, reg->name, reg->name_len); + str_offset += reg->name_len; + i++; + + list_del(®->list); + kfree(reg); + } + + /* Double-check that we calculated the sizes correctly */ + WARN_ON(gsp->registry_rpc_size != str_offset); + + registry->size = gsp->registry_rpc_size; +} + +/** + * clean_registry -- clean up registry memory in case of error + * @gsp: gsp pointer + * + * Call this function to clean up all memory allocated by add_registry() + * in case of error and build_registry() is not called. + */ +static void clean_registry(struct nvkm_gsp *gsp) +{ + struct registry_list_entry *reg, *n; + + list_for_each_entry_safe(reg, n, &gsp->registry_list, list) { + list_del(®->list); + kfree(reg); + } + + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE); +} + +MODULE_PARM_DESC(NVreg_RegistryDwords, + "A semicolon-separated list of key=integer pairs of GSP-RM registry keys"); +static char *NVreg_RegistryDwords; +module_param(NVreg_RegistryDwords, charp, 0400); + /* dword only */ struct nv_gsp_registry_entries { const char *name; u32 value; }; +/** + * r535_registry_entries - required registry entries for GSP-RM + * + * This array lists registry entries that are required for GSP-RM to + * function correctly. + * + * RMSecBusResetEnable - enables PCI secondary bus reset + * RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration + * registers on any PCI reset. + */ static const struct nv_gsp_registry_entries r535_registry_entries[] = { { "RMSecBusResetEnable", 1 }, { "RMForcePcieConfigSave", 1 }, }; #define NV_GSP_REG_NUM_ENTRIES ARRAY_SIZE(r535_registry_entries) +/** + * strip - strips all characters in 'reject' from 's' + * @s: string to strip + * @reject: string of characters to remove + * + * 's' is modified. + * + * Returns the length of the new string. + */ +static size_t strip(char *s, const char *reject) +{ + char *p = s, *p2 = s; + size_t length = 0; + char c; + + do { + while ((c = *p2) && strchr(reject, c)) + p2++; + + *p++ = c = *p2++; + length++; + } while (c); + + return length; +} + +/** + * r535_gsp_rpc_set_registry - build registry RPC and call GSP-RM + * @gsp: gsp pointer + * + * The GSP-RM registry is a set of key/value pairs that configure some aspects + * of GSP-RM. The keys are strings, and the values are 32-bit integers. + * + * The registry is built from a combination of a static hard-coded list (see + * above) and entries passed on the driver's command line. + */ static int r535_gsp_rpc_set_registry(struct nvkm_gsp *gsp) { PACKED_REGISTRY_TABLE *rpc; - char *strings; - int str_offset; - int i; - size_t rpc_size = struct_size(rpc, entries, NV_GSP_REG_NUM_ENTRIES); + unsigned int i; + int ret; - /* add strings + null terminator */ - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) - rpc_size += strlen(r535_registry_entries[i].name) + 1; + INIT_LIST_HEAD(&gsp->registry_list); + gsp->registry_rpc_size = sizeof(PACKED_REGISTRY_TABLE); - rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, rpc_size); - if (IS_ERR(rpc)) - return PTR_ERR(rpc); + /* Add the required registry entries first */ + for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { + ret = add_registry_num(gsp, r535_registry_entries[i].name, + r535_registry_entries[i].value); + if (ret) { + clean_registry(gsp); + return ret; + } + } + + /* + * The NVreg_RegistryDwords parameter is a string of key=value + * pairs separated by semicolons. We need to extract and trim each + * substring, and then parse the substring to extract the key and + * value. + */ + if (NVreg_RegistryDwords) { + char *p = kstrdup(NVreg_RegistryDwords, GFP_KERNEL); + char *start, *next = p, *equal; + size_t length; + + /* Remove any whitespace from the parameter string */ + length = strip(p, " \t\n"); + + while ((start = strsep(&next, ";"))) { + long value; + + equal = strchr(start, '='); + if (!equal || (equal == start) || !isdigit(equal[1])) { + nvkm_error(&gsp->subdev, + "ignoring invalid registry string '%s'\n", start); + continue; + } - rpc->numEntries = NV_GSP_REG_NUM_ENTRIES; + ret = kstrtol(equal + 1, 0, &value); + if (ret) { + nvkm_error(&gsp->subdev, + "ignoring invalid registry value in '%s'\n", start); + continue; + } - str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]); - strings = (char *)&rpc->entries[NV_GSP_REG_NUM_ENTRIES]; - for (i = 0; i < NV_GSP_REG_NUM_ENTRIES; i++) { - int name_len = strlen(r535_registry_entries[i].name) + 1; - - rpc->entries[i].nameOffset = str_offset; - rpc->entries[i].type = 1; - rpc->entries[i].data = r535_registry_entries[i].value; - rpc->entries[i].length = 4; - memcpy(strings, r535_registry_entries[i].name, name_len); - strings += name_len; - str_offset += name_len; + /* Truncate the key=value string to just key */ + *equal = 0; + + ret = add_registry_num(gsp, start, value); + if (ret) { + nvkm_error(&gsp->subdev, + "ignoring invalid registry key/value '%s=%lu'\n", + start, value); + continue; + } + } + + kfree(p); } - rpc->size = str_offset; + + rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_SET_REGISTRY, gsp->registry_rpc_size); + if (IS_ERR(rpc)) { + clean_registry(gsp); + return PTR_ERR(rpc); + } + + build_registry(gsp, rpc); return nvkm_gsp_rpc_wr(gsp, rpc, false); }
Add the NVreg_RegistryDwords command line parameter, which allows specifying additional registry keys to be sent to GSP-RM. This allows additional configuration, debugging, and experimentation with GSP-RM, which uses these keys to alter its behavior. Note that these keys are passed as-is to GSP-RM, and Nouveau does not parse them. This is in contrast to the Nvidia driver, which may parse some of the keys to configure some functionality in concert with GSP-RM. Therefore, any keys which also require action by the driver may not function correctly when passed by Nouveau. Caveat emptor. The name and format of NVreg_RegistryDwords is the same as used by the Nvidia driver, to maintain compatibility. Signed-off-by: Timur Tabi <ttabi@nvidia.com> --- v3: rebased to drm-next .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 6 + .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 279 ++++++++++++++++-- 2 files changed, 261 insertions(+), 24 deletions(-)