Hi,

this patch targets drm-misc-next but depends on commit 838ae9f45c4e 
("nouveau/gsp:
Avoid addressing beyond end of rpc->entries") which is only in drm-misc-fixes.

Please let me know if you want to backmerge directly, let me hold the patch back
until or anything else.

- Danilo

On 4/17/24 23:53, 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 <tt...@nvidia.com>
---
v7:
   rebase onto drm-misc-next
   rename vlen to alloc_size

  .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |   6 +
  .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 355 ++++++++++++++++--
  2 files changed, 337 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 9858c1438aa7..1b5d5b02c640 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
@@ -1080,51 +1082,356 @@ r535_gsp_rpc_unloading_guest_driver(struct nvkm_gsp 
*gsp, bool suspend)
        return nvkm_gsp_rpc_wr(gsp, rpc, true);
  }
+enum registry_type {
+       REGISTRY_TABLE_ENTRY_TYPE_DWORD  = 1, /* 32-bit unsigned integer */
+       REGISTRY_TABLE_ENTRY_TYPE_BINARY = 2, /* Binary blob */
+       REGISTRY_TABLE_ENTRY_TYPE_STRING = 3, /* Null-terminated string */
+};
+
+/* An arbitrary limit to the length of a registry key */
+#define REGISTRY_MAX_KEY_LENGTH                64
+
+/**
+ * registry_list_entry - linked list member for a registry key/value
+ * @head: list_head struct
+ * @type: dword, binary, or string
+ * @klen: the length of name of the key
+ * @vlen: the length of the value
+ * @key: the key name
+ * @dword: the data, if REGISTRY_TABLE_ENTRY_TYPE_DWORD
+ * @binary: the data, if TYPE_BINARY or TYPE_STRING
+ *
+ * Every registry key/value is represented internally by this struct.
+ *
+ * Type DWORD is a simple 32-bit unsigned integer, and its value is stored in
+ * @dword.
+ *
+ * Types BINARY and STRING are variable-length binary blobs.  The only real
+ * difference between BINARY and STRING is that STRING is null-terminated and
+ * is expected to contain only printable characters.
+ *
+ * Note: it is technically possible to have multiple keys with the same name
+ * but different types, but this is not useful since GSP-RM expects keys to
+ * have only one specific type.
+ */
+struct registry_list_entry {
+       struct list_head head;
+       enum registry_type type;
+       size_t klen;
+       char key[REGISTRY_MAX_KEY_LENGTH];
+       size_t vlen;
+       u32 dword;                      /* TYPE_DWORD */
+       u8 binary[] __counted_by(vlen); /* TYPE_BINARY or TYPE_STRING */
+};
+
+/**
+ * add_registry -- adds a registry entry
+ * @gsp: gsp pointer
+ * @key: name of the registry key
+ * @type: type of data
+ * @data: pointer to value
+ * @length: size of data, in bytes
+ *
+ * Adds a registry key/value pair to the registry database.
+ *
+ * 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 *key,
+                       enum registry_type type, const void *data, size_t 
length)
+{
+       struct registry_list_entry *reg;
+       const size_t nlen = strnlen(key, REGISTRY_MAX_KEY_LENGTH) + 1;
+       size_t alloc_size; /* extra bytes to alloc for binary or string value */
+
+       if (nlen > REGISTRY_MAX_KEY_LENGTH)
+               return -EINVAL;
+
+       alloc_size = (type == REGISTRY_TABLE_ENTRY_TYPE_DWORD) ? 0 : length;
+
+       reg = kmalloc(sizeof(*reg) + alloc_size, GFP_KERNEL);
+       if (!reg)
+               return -ENOMEM;
+
+       switch (type) {
+       case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
+               reg->dword = *(const u32 *)(data);
+               break;
+       case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
+       case REGISTRY_TABLE_ENTRY_TYPE_STRING:
+               memcpy(reg->binary, data, alloc_size);
+               break;
+       default:
+               nvkm_error(&gsp->subdev, "unrecognized registry type %u for 
'%s'\n",
+                          type, key);
+               kfree(reg);
+               return -EINVAL;
+       }
+
+       memcpy(reg->key, key, nlen);
+       reg->klen = nlen;
+       reg->vlen = length;
+       reg->type = type;
+
+       list_add_tail(&reg->head, &gsp->registry_list);
+       gsp->registry_rpc_size += sizeof(PACKED_REGISTRY_ENTRY) + nlen + 
alloc_size;
+
+       return 0;
+}
+
+static int add_registry_num(struct nvkm_gsp *gsp, const char *key, u32 value)
+{
+       return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_DWORD,
+                           &value, sizeof(u32));
+}
+
+static int add_registry_string(struct nvkm_gsp *gsp, const char *key, const 
char *value)
+{
+       return add_registry(gsp, key, REGISTRY_TABLE_ENTRY_TYPE_STRING,
+                           value, strlen(value) + 1);
+}
+
+/**
+ * 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                   |
+ * +----------------------------------------+
+ * |Null-terminated key (string) for entry 0|
+ * +----------------------------------------+
+ * |Binary/string data value for entry 0    | (only if necessary)
+ * +----------------------------------------+
+ *
+ * +----------------------------------------+
+ * |PACKED_REGISTRY_ENTRY                   |
+ * +----------------------------------------+
+ * |Null-terminated key (string) for entry 1|
+ * +----------------------------------------+
+ * |Binary/string data value for entry 1    | (only if necessary)
+ * +----------------------------------------+
+ * ... (and so on, one copy for each entry)
+ *
+ *
+ * The 'data' field of an entry is either a 32-bit integer (for type DWORD)
+ * or an offset into the PACKED_REGISTRY_TABLE (for types BINARY and STRING).
+ *
+ * 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, head) {
+               registry->entries[i].type = reg->type;
+               registry->entries[i].length = reg->vlen;
+
+               /* Append the key name to the table */
+               registry->entries[i].nameOffset = str_offset;
+               memcpy((void *)registry + str_offset, reg->key, reg->klen);
+               str_offset += reg->klen;
+
+               switch (reg->type) {
+               case REGISTRY_TABLE_ENTRY_TYPE_DWORD:
+                       registry->entries[i].data = reg->dword;
+                       break;
+               case REGISTRY_TABLE_ENTRY_TYPE_BINARY:
+               case REGISTRY_TABLE_ENTRY_TYPE_STRING:
+                       /* If the type is binary or string, also append the 
value */
+                       memcpy((void *)registry + str_offset, reg->binary, 
reg->vlen);
+                       registry->entries[i].data = str_offset;
+                       str_offset += reg->vlen;
+                       break;
+               default:
+               }
+
+               i++;
+               list_del(&reg->head);
+               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, head) {
+               list_del(&reg->head);
+               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);
+       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;
+               }
+       }
- rpc->numEntries = NV_GSP_REG_NUM_ENTRIES;
+       /*
+        * 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 || equal[1] == 0) {
+                               nvkm_error(&gsp->subdev,
+                                          "ignoring invalid registry string 
'%s'\n",
+                                          start);
+                               continue;
+                       }
- str_offset = offsetof(typeof(*rpc), entries[NV_GSP_REG_NUM_ENTRIES]);
-       strings = (char *)rpc + str_offset;
-       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 = kstrtol(equal + 1, 0, &value);
+                       if (!ret) {
+                               ret = add_registry_num(gsp, start, value);
+                       } else {
+                               /* Not a number, so treat it as a string */
+                               ret = add_registry_string(gsp, start, equal + 
1);
+                       }
+
+                       if (ret) {
+                               nvkm_error(&gsp->subdev,
+                                          "ignoring invalid registry key/value 
'%s=%s'\n",
+                                          start, equal + 1);
+                               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);
  }

base-commit: f7ad2ce5fd89ab5d146da8f486a310746df5dc9e
prerequisite-patch-id: 9bb653b6a53dcba4171d653e24a242461374f9fe
prerequisite-patch-id: 7093a9db84053e43f6f278bf1d159a25d14ceebf

Reply via email to