thr3ads.net - Nouveau - [PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes [Jan 2025]

If this information is useful, please help other people find it:
Share via:

Zhi Wang

2025-Jan-24 18:29 UTC

[PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

Hi folks:

Here is another re-spin of handling the large GSP message return.

Besides the support of the large GSP message, kernel doc and many cleanups
are introduced according to the comments in the previous spin [1].

For testing, I tried the following without any problem:

- run Uniengine Heaven [2] on my RTX 4060 for 8 hours
- the GL CTS runner [3] (commandline: ./cts-runner --type-gl40) from
Khronos
- with the vGPU RFC patches [4], boot up a ubuntu VM with a vGPU on a L40
and run glmark2.

v4:
- Introduce a documentation in Documentation/gpu/nouveau.rst.
- Fix the reference link of NVIDIA SDK headers in the document.
- Replace the "decoders" term with "terminology".
- Explain the adding offset in r535_gsp_msgq_get_entry().
- Replace the magic number 0x1000 with GSP_PAGE_SIZE in the re-factor.
- Introduce a DOC to explain the GSP message receiving flow.

v3:

- Add a kernel doc and chart to introduce the GSP message and denote the
memory layout.
- Add a decoder section in the kernel doc to explain the terms in the code.
- Refine the many confusing variable names to align with the terms in the
kernel doc.
- Introduce the continaution records in the kernel doc.
- Re-factor the complicated function r535_gsp_msgq_wait().
- Other minor cleanups.

v2:

- Remove the Fixes: tags as the vanilla nouveau aren't going to hit these
bugs. (Danilo)
- Test the patchset on VK-GL-CTS. (Danilo)
- Remove the ambiguous empty line in PATCH 2. (Danilo)
- Rename the r535_gsp_msgq_wait to gsp_msgq_recv. (Danilo)
- Introduce a data structure to hold the params of gsp_msgq_recv(). (Danilo)
- Document the params and the states they are related to. (Danilo)

[1] https://lore.kernel.org/nouveau/20241031085250.2941482-1-zhiw at nvidia.com/
[2] https://benchmark.unigine.com/heaven
[3] https://github.com/KhronosGroup/VK-GL-CTS
[4] https://lore.kernel.org/kvm/20240922124951.1946072-1-zhiw at nvidia.com/T/#t


Zhi Wang (15):
  drm/nouveau: add a kernel doc to introduce the GSP RPC
  drm/nouveau: rename "repc" to "gsp_rpc_len" on the GSP
message recv
    path
  drm/nouveau: rename "argv" to what it represents on the GSP message
    send path
  drm/nouveau: remove unused param repc in *rm_alloc_push()
  drm/nouveau: rename "argv" to what it represents in *rm_{alloc,
    ctrl}_*()
  drm/nouveau: rename "argc" to what it represents in GSP RPC routines
  drm/nouveau: fix the broken marco GSP_MSG_MAX_SIZE
  drm/nouveau: remove the magic number in r535_gsp_rpc_push()
  drm/nouveau: refine the variable names in r535_gsp_rpc_push()
  drm/nouveau: refine the variable names in r535_gsp_msg_recv()
  drm/nouveau: rename the variable "cmd" to "msg" in
r535_gsp_cmdq_{get,
    push}()
  drm/nouveau: factor out r535_gsp_msgq_peek()
  drm/nouveau: factor out r535_gsp_msgq_recv_one_elem()
  drm/nouveau: support handling the return of large GSP message
  drm/nouveau: consume the return of large GSP message

 Documentation/gpu/drivers.rst                 |   1 +
 Documentation/gpu/nouveau.rst                 |  27 +
 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |   8 +-
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 526 +++++++++++++-----
 4 files changed, 409 insertions(+), 153 deletions(-)
 create mode 100644 Documentation/gpu/nouveau.rst

-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 01/15] drm/nouveau: add a kernel doc to introduce the GSP RPC

In order to explain the name clean-ups in GSP RPC routines, a kernel
doc to explain the memory layout and terms is required.

Add a kernel doc to introduce the GSP RPC.

Cc: Danilo Krummrich <dakr at kernel.org>
Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 Documentation/gpu/drivers.rst                 |  1 +
 Documentation/gpu/nouveau.rst                 | 27 +++++++++++
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 45 +++++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 Documentation/gpu/nouveau.rst

diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index 1f17ad0790d7..74fc2cbf1b6f 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -10,6 +10,7 @@ GPU Driver Documentation
    imagination/index
    mcde
    meson
+   nouveau
    pl111
    tegra
    tve200
diff --git a/Documentation/gpu/nouveau.rst b/Documentation/gpu/nouveau.rst
new file mode 100644
index 000000000000..127e15c92b74
--- /dev/null
+++ b/Documentation/gpu/nouveau.rst
@@ -0,0 +1,27 @@
+==============================+ drm/nouveau NVIDIA GPU Driver
+==============================+
+The drm/nouveau driver provides support for a wide range of NVIDIA GPUs,
+covering GeForce, Quadro, and Tesla series, from the NV04 architecture up
+to the latest Turing, Ampere, Ada families.
+
+NVKM: NVIDIA Kernel Manager
+==========================+
+The NVKM component serves as the core abstraction layer within the nouveau
+driver, responsible for managing NVIDIA GPU hardware at the kernel level.
+NVKM provides a unified interface for handling various GPU  architectures.
+
+It enables resource management, power control, memory handling, and command
+submission required for the proper functioning of NVIDIA GPUs under the
+nouveau driver.
+
+NVKM plays a critical role in abstracting hardware complexities and
+providing a consistent API to upper layers of the driver stack.
+
+GSP Support
+------------------------
+
+.. kernel-doc:: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+   :doc: GSP message queue element
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 58502102926b..2a315fe55857 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -63,6 +63,51 @@ extern struct dentry *nouveau_debugfs_root;
 #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
 #define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
 
+/**
+ * DOC: GSP message queue element
+ *
+ *
https://github.com/NVIDIA/open-gpu-kernel-modules/blob/535/src/nvidia/inc/kernel/gpu/gsp/message_queue_priv.h
+ *
+ * The GSP command queue and status queue are message queues for the
+ * communication between software and GSP. The software submits the GSP
+ * RPC via the GSP command queue, GSP writes the status of the submitted
+ * RPC in the status queue.
+ *
+ * A GSP message queue element consists of three parts:
+ *
+ * - message element header (struct r535_gsp_msg), which mostly maintains
+ * the metadata for queuing the element.
+ *
+ * - RPC message header (struct nvfw_gsp_rpc), which maintains the info
+ * of the RPC. E.g., the RPC function number.
+ *
+ * - The payload, where the RPC message stays. E.g. the params of a
+ * specific RPC function. Some RPC functions also have their headers
+ * in the payload. E.g. rm_alloc, rm_control.
+ *
+ * The memory layout of a GSP message element can be illustrated below::
+ *
+ *    +------------------------+
+ *    | Message Element Header |
+ *    |    (r535_gsp_msg)      |
+ *    |                        |
+ *    | (r535_gsp_msg.data)    |
+ *    |          |             |
+ *    |----------V-------------|
+ *    |    GSP RPC Header      |
+ *    |    (nvfw_gsp_rpc)      |
+ *    |                        |
+ *    | (nvfw_gsp_rpc.data)    |
+ *    |          |             |
+ *    |----------V-------------|
+ *    |       Payload          |
+ *    |                        |
+ *    |   header(optional)     |
+ *    |        params          |
+ *    +------------------------+
+ *
+ */
+
 struct r535_gsp_msg {
 	u8 auth_tag_buffer[16];
 	u8 aad_buffer[16];
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 02/15] drm/nouveau: rename "repc" to "gsp_rpc_len" on the GSP message recv path

The name "repc" has different meanings in different contexts.

To improve the readability, it's better to refine it to a name that
reflects what it actually represents.

Rename "repc" to "gsp_rpc_len" in the GSP message recv path.
Add an
section in the doc to explain the terms.

No functional change is intended.

Cc: Danilo Krummrich <dakr at kernel.org>
Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 46 +++++++++++--------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 2a315fe55857..3189d5c475f7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -106,6 +106,9 @@ extern struct dentry *nouveau_debugfs_root;
  *    |        params          |
  *    +------------------------+
  *
+ * Terminology:
+ *
+ * - gsp_rpc_len: size of (GSP RPC header + payload)
  */
 
 struct r535_gsp_msg {
@@ -135,7 +138,8 @@ r535_rpc_status_to_errno(uint32_t rpc_status)
 }
 
 static void *
-r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32 *prepc, int *ptime)
+r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, u32 *prepc,
+		   int *ptime)
 {
 	struct r535_gsp_msg *mqe;
 	u32 size, rptr = *gsp->msgq.rptr;
@@ -143,7 +147,8 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 	u8 *msg;
 	u32 len;
 
-	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + repc, GSP_PAGE_SIZE);
+	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + gsp_rpc_len,
+			    GSP_PAGE_SIZE);
 	if (WARN_ON(!size || size >= gsp->msgq.cnt))
 		return ERR_PTR(-EINVAL);
 
@@ -169,21 +174,21 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 		return mqe->data;
 	}
 
-	size = ALIGN(repc + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
+	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
-	msg = kvmalloc(repc, GFP_KERNEL);
+	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
 	if (!msg)
 		return ERR_PTR(-ENOMEM);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
-	len = min_t(u32, repc, len);
+	len = min_t(u32, gsp_rpc_len, len);
 	memcpy(msg, mqe->data, len);
 
-	repc -= len;
+	gsp_rpc_len -= len;
 
-	if (repc) {
+	if (gsp_rpc_len) {
 		mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
-		memcpy(msg + len, mqe, repc);
+		memcpy(msg + len, mqe, gsp_rpc_len);
 	}
 
 	rptr = (rptr + DIV_ROUND_UP(size, GSP_PAGE_SIZE)) % gsp->msgq.cnt;
@@ -194,9 +199,9 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 }
 
 static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 repc, int *ptime)
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
 {
-	return r535_gsp_msgq_wait(gsp, repc, NULL, ptime);
+	return r535_gsp_msgq_wait(gsp, gsp_rpc_len, NULL, ptime);
 }
 
 static int
@@ -317,7 +322,7 @@ r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc
*msg, int lvl)
 }
 
 static struct nvfw_gsp_rpc *
-r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 repc)
+r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
 	struct nvfw_gsp_rpc *msg;
@@ -342,10 +347,11 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 repc)
 	r535_gsp_msg_dump(gsp, msg, NV_DBG_TRACE);
 
 	if (fn && msg->function == fn) {
-		if (repc) {
-			if (msg->length < sizeof(*msg) + repc) {
+		if (gsp_rpc_len) {
+			if (msg->length < sizeof(*msg) + gsp_rpc_len) {
 				nvkm_error(subdev, "msg len %d < %zd\n",
-					   msg->length, sizeof(*msg) + repc);
+					   msg->length, sizeof(*msg) +
+					   gsp_rpc_len);
 				r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
 				r535_gsp_msg_done(gsp, msg);
 				return ERR_PTR(-EIO);
@@ -414,7 +420,8 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 }
 
 static void *
-r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait, u32 repc)
+r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait,
+		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
 	struct nvfw_gsp_rpc *msg;
@@ -434,7 +441,7 @@ r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 		return ERR_PTR(ret);
 
 	if (wait) {
-		msg = r535_gsp_msg_recv(gsp, fn, repc);
+		msg = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
 		if (!IS_ERR_OR_NULL(msg))
 			repv = msg->data;
 		else
@@ -770,7 +777,8 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 }
 
 static void *
-r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait, u32 repc)
+r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait,
+		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
 	struct r535_gsp_msg *cmd = container_of((void *)rpc, typeof(*cmd), data);
@@ -817,7 +825,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 
 		/* Wait for reply. */
 		if (wait) {
-			rpc = r535_gsp_msg_recv(gsp, fn, repc);
+			rpc = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
 			if (!IS_ERR_OR_NULL(rpc))
 				repv = rpc->data;
 			else
@@ -826,7 +834,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 			repv = NULL;
 		}
 	} else {
-		repv = r535_gsp_rpc_send(gsp, argv, wait, repc);
+		repv = r535_gsp_rpc_send(gsp, argv, wait, gsp_rpc_len);
 	}
 
 done:
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 03/15] drm/nouveau: rename "argv" to what it represents on the GSP message send path

The name "argv" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "repc" to what it represents in the GSP message send path.
Wrap the long container_of() into to_gsp_hdr() to make checkpatch.pl
happy.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 3189d5c475f7..cf19afd6bf44 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -123,6 +123,9 @@ struct r535_gsp_msg {
 
 #define GSP_MSG_HDR_SIZE offsetof(struct r535_gsp_msg, data)
 
+#define to_gsp_hdr(p, header) \
+	container_of((void *)p, typeof(*header), data)
+
 static int
 r535_rpc_status_to_errno(uint32_t rpc_status)
 {
@@ -205,9 +208,9 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *ptime)
 }
 
 static int
-r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *argv)
+r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
-	struct r535_gsp_msg *cmd = container_of(argv, typeof(*cmd), data);
+	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	struct r535_gsp_msg *cqe;
 	u32 argc = cmd->checksum;
 	u64 *ptr = (void *)cmd;
@@ -420,10 +423,10 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 }
 
 static void *
-r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait,
+r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload, bool wait,
 		  u32 gsp_rpc_len)
 {
-	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
+	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
 	struct nvfw_gsp_rpc *msg;
 	u32 fn = rpc->function;
 	void *repv = NULL;
@@ -777,11 +780,11 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 }
 
 static void *
-r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait,
+r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool wait,
 		  u32 gsp_rpc_len)
 {
-	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
-	struct r535_gsp_msg *cmd = container_of((void *)rpc, typeof(*cmd), data);
+	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
+	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	const u32 max_msg_size = (16 * 0x1000) - sizeof(struct r535_gsp_msg);
 	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
 	u32 rpc_size = rpc->length - sizeof(*rpc);
@@ -795,11 +798,11 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 		rpc->length = sizeof(*rpc) + max_rpc_size;
 		cmd->checksum = rpc->length;
 
-		repv = r535_gsp_rpc_send(gsp, argv, false, 0);
+		repv = r535_gsp_rpc_send(gsp, payload, false, 0);
 		if (IS_ERR(repv))
 			goto done;
 
-		argv += max_rpc_size;
+		payload += max_rpc_size;
 		rpc_size -= max_rpc_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
@@ -813,13 +816,13 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 				goto done;
 			}
 
-			memcpy(next, argv, size);
+			memcpy(next, payload, size);
 
 			repv = r535_gsp_rpc_send(gsp, next, false, 0);
 			if (IS_ERR(repv))
 				goto done;
 
-			argv += size;
+			payload += size;
 			rpc_size -= size;
 		}
 
@@ -834,7 +837,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 			repv = NULL;
 		}
 	} else {
-		repv = r535_gsp_rpc_send(gsp, argv, wait, gsp_rpc_len);
+		repv = r535_gsp_rpc_send(gsp, payload, wait, gsp_rpc_len);
 	}
 
 done:
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 04/15] drm/nouveau: remove unused param repc in *rm_alloc_push()

The user of *rm_alloc_push() always pass 0 in repc.

Remove unused param repc since no user actually uses it.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 8 ++++----
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 8 +++-----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index 5c5f4607fcc9..746e126c3ecf 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -196,7 +196,7 @@ struct nvkm_gsp {
 		void (*rm_ctrl_done)(struct nvkm_gsp_object *, void *repv);
 
 		void *(*rm_alloc_get)(struct nvkm_gsp_object *, u32 oclass, u32 argc);
-		void *(*rm_alloc_push)(struct nvkm_gsp_object *, void *argv, u32 repc);
+		void *(*rm_alloc_push)(struct nvkm_gsp_object *, void *argv);
 		void (*rm_alloc_done)(struct nvkm_gsp_object *, void *repv);
 
 		int (*rm_free)(struct nvkm_gsp_object *);
@@ -353,9 +353,9 @@ nvkm_gsp_rm_alloc_get(struct nvkm_gsp_object *parent, u32
handle, u32 oclass, u3
 }
 
 static inline void *
-nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
+nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
 {
-	void *repv = object->client->gsp->rm->rm_alloc_push(object, argv,
repc);
+	void *repv = object->client->gsp->rm->rm_alloc_push(object, argv);
 
 	if (IS_ERR(repv))
 		object->client = NULL;
@@ -366,7 +366,7 @@ nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void
*argv, u32 repc)
 static inline int
 nvkm_gsp_rm_alloc_wr(struct nvkm_gsp_object *object, void *argv)
 {
-	void *repv = nvkm_gsp_rm_alloc_push(object, argv, 0);
+	void *repv = nvkm_gsp_rm_alloc_push(object, argv);
 
 	if (IS_ERR(repv))
 		return PTR_ERR(repv);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index cf19afd6bf44..eb82e589381a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -647,13 +647,13 @@ r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object,
void *repv)
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32
repc)
+r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
 {
 	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
 	struct nvkm_gsp *gsp = object->client->gsp;
-	void *ret;
+	void *ret = NULL;
 
-	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, sizeof(*rpc) + repc);
+	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, sizeof(*rpc));
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
@@ -661,8 +661,6 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object,
void *argv, u32 repc)
 		ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
 		if (PTR_ERR(ret) != -EAGAIN && PTR_ERR(ret) != -EBUSY)
 			nvkm_error(&gsp->subdev, "RM_ALLOC: 0x%x\n",
rpc->status);
-	} else {
-		ret = repc ? rpc->params : NULL;
 	}
 
 	nvkm_gsp_rpc_done(gsp, rpc);
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 05/15] drm/nouveau: rename "argv" to what it represents in rm_{alloc, ctrl}_()

The name "argv" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "argv" to what it represents. Wrap the long container_of() into
to_payload_header() to denote a clear meaning and make checkpatch.pl
happy.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index eb82e589381a..3bc5f25f1e8c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -126,6 +126,9 @@ struct r535_gsp_msg {
 #define to_gsp_hdr(p, header) \
 	container_of((void *)p, typeof(*header), data)
 
+#define to_payload_hdr(p, header) \
+	container_of((void *)p, typeof(*header), params)
+
 static int
 r535_rpc_status_to_errno(uint32_t rpc_status)
 {
@@ -639,17 +642,17 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
 }
 
 static void
-r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object, void *repv)
+r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(repv, typeof(*rpc), params);
+	rpc_gsp_rm_alloc_v03_00 *rpc = to_payload_hdr(params, rpc);
 
 	nvkm_gsp_rpc_done(object->client->gsp, rpc);
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
+r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
+	rpc_gsp_rm_alloc_v03_00 *rpc = to_payload_hdr(params, rpc);
 	struct nvkm_gsp *gsp = object->client->gsp;
 	void *ret = NULL;
 
@@ -692,25 +695,25 @@ r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object,
u32 oclass, u32 argc)
 }
 
 static void
-r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *repv)
+r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_control_v03_00 *rpc = container_of(repv, typeof(*rpc), params);
+	rpc_gsp_rm_control_v03_00 *rpc = to_payload_hdr(params, rpc);
 
-	if (!repv)
+	if (!params)
 		return;
 	nvkm_gsp_rpc_done(object->client->gsp, rpc);
 }
 
 static int
-r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **argv, u32
repc)
+r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **params, u32
repc)
 {
-	rpc_gsp_rm_control_v03_00 *rpc = container_of((*argv), typeof(*rpc), params);
+	rpc_gsp_rm_control_v03_00 *rpc = to_payload_hdr((*params), rpc);
 	struct nvkm_gsp *gsp = object->client->gsp;
 	int ret = 0;
 
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, repc);
 	if (IS_ERR_OR_NULL(rpc)) {
-		*argv = NULL;
+		*params = NULL;
 		return PTR_ERR(rpc);
 	}
 
@@ -722,7 +725,7 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object,
void **argv, u32 repc)
 	}
 
 	if (repc)
-		*argv = rpc->params;
+		*params = rpc->params;
 	else
 		nvkm_gsp_rpc_done(gsp, rpc);
 
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 06/15] drm/nouveau: rename "argc" to what it represents in GSP RPC routines

The name "argc" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "argc" to what it represents. Add terms in the decoder section
to
explain their meaning.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 58 +++++++++++--------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 3bc5f25f1e8c..92daed69cd46 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -109,6 +109,8 @@ extern struct dentry *nouveau_debugfs_root;
  * Terminology:
  *
  * - gsp_rpc_len: size of (GSP RPC header + payload)
+ * - params_size: size of params in the payload
+ * - payload_size: size of (header if exists + params) in the payload
  */
 
 struct r535_gsp_msg {
@@ -215,21 +217,21 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
 	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	struct r535_gsp_msg *cqe;
-	u32 argc = cmd->checksum;
+	u32 gsp_rpc_len = cmd->checksum;
 	u64 *ptr = (void *)cmd;
 	u64 *end;
 	u64 csum = 0;
 	int free, time = 1000000;
-	u32 wptr, size, step;
+	u32 wptr, size, step, len;
 	u32 off = 0;
 
-	argc = ALIGN(GSP_MSG_HDR_SIZE + argc, GSP_PAGE_SIZE);
+	len = ALIGN(GSP_MSG_HDR_SIZE + gsp_rpc_len, GSP_PAGE_SIZE);
 
-	end = (u64 *)((char *)ptr + argc);
+	end = (u64 *)((char *)ptr + len);
 	cmd->pad = 0;
 	cmd->checksum = 0;
 	cmd->sequence = gsp->cmdq.seq++;
-	cmd->elem_count = DIV_ROUND_UP(argc, 0x1000);
+	cmd->elem_count = DIV_ROUND_UP(len, 0x1000);
 
 	while (ptr < end)
 		csum ^= *ptr++;
@@ -255,7 +257,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 
 		cqe = (void *)((u8 *)gsp->shm.cmdq.ptr + 0x1000 + wptr * 0x1000);
 		step = min_t(u32, free, (gsp->cmdq.cnt - wptr));
-		size = min_t(u32, argc, step * GSP_PAGE_SIZE);
+		size = min_t(u32, len, step * GSP_PAGE_SIZE);
 
 		memcpy(cqe, (u8 *)cmd + off, size);
 
@@ -264,8 +266,8 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 			wptr = 0;
 
 		off  += size;
-		argc -= size;
-	} while(argc);
+		len -= size;
+	} while (len);
 
 	nvkm_trace(&gsp->subdev, "cmdq: wptr %d\n", wptr);
 	wmb();
@@ -279,17 +281,17 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 }
 
 static void *
-r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 argc)
+r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 {
 	struct r535_gsp_msg *cmd;
-	u32 size = GSP_MSG_HDR_SIZE + argc;
+	u32 size = GSP_MSG_HDR_SIZE + gsp_rpc_len;
 
 	size = ALIGN(size, GSP_MSG_MIN_SIZE);
 	cmd = kvzalloc(size, GFP_KERNEL);
 	if (!cmd)
 		return ERR_PTR(-ENOMEM);
 
-	cmd->checksum = argc;
+	cmd->checksum = gsp_rpc_len;
 	return cmd->data;
 }
 
@@ -672,16 +674,22 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object,
void *params)
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object, u32 oclass, u32 argc)
+r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object, u32 oclass,
+			  u32 params_size)
 {
 	struct nvkm_gsp_client *client = object->client;
 	struct nvkm_gsp *gsp = client->gsp;
 	rpc_gsp_rm_alloc_v03_00 *rpc;
 
-	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x new obj:0x%08x
cls:0x%08x argc:%d\n",
-		   client->object.handle, object->parent->handle, object->handle,
oclass, argc);
+	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x new
obj:0x%08x\n",
+		   client->object.handle, object->parent->handle,
+		   object->handle);
 
-	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_ALLOC, sizeof(*rpc) +
argc);
+	nvkm_debug(&gsp->subdev, "cls:0x%08x params_size:%d\n",
oclass,
+		   params_size);
+
+	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_ALLOC,
+			       sizeof(*rpc) + params_size);
 	if (IS_ERR(rpc))
 		return rpc;
 
@@ -690,7 +698,7 @@ r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object,
u32 oclass, u32 argc)
 	rpc->hObject = object->handle;
 	rpc->hClass = oclass;
 	rpc->status = 0;
-	rpc->paramsSize = argc;
+	rpc->paramsSize = params_size;
 	return rpc->params;
 }
 
@@ -733,16 +741,17 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object,
void **params, u32 rep
 }
 
 static void *
-r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32 argc)
+r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32
params_size)
 {
 	struct nvkm_gsp_client *client = object->client;
 	struct nvkm_gsp *gsp = client->gsp;
 	rpc_gsp_rm_control_v03_00 *rpc;
 
-	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x
argc:%d\n",
-		   client->object.handle, object->handle, cmd, argc);
+	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x
params_size:%d\n",
+		   client->object.handle, object->handle, cmd, params_size);
 
-	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL, sizeof(*rpc)
+ argc);
+	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL,
+			       sizeof(*rpc) + params_size);
 	if (IS_ERR(rpc))
 		return rpc;
 
@@ -750,7 +759,7 @@ r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32
cmd, u32 argc)
 	rpc->hObject    = object->handle;
 	rpc->cmd	= cmd;
 	rpc->status     = 0;
-	rpc->paramsSize = argc;
+	rpc->paramsSize = params_size;
 	return rpc->params;
 }
 
@@ -763,11 +772,12 @@ r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 }
 
 static void *
-r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
+r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 {
 	struct nvfw_gsp_rpc *rpc;
 
-	rpc = r535_gsp_cmdq_get(gsp, ALIGN(sizeof(*rpc) + argc, sizeof(u64)));
+	rpc = r535_gsp_cmdq_get(gsp, ALIGN(sizeof(*rpc) + payload_size,
+				sizeof(u64)));
 	if (IS_ERR(rpc))
 		return ERR_CAST(rpc);
 
@@ -776,7 +786,7 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 	rpc->function = fn;
 	rpc->rpc_result = 0xffffffff;
 	rpc->rpc_result_private = 0xffffffff;
-	rpc->length = sizeof(*rpc) + argc;
+	rpc->length = sizeof(*rpc) + payload_size;
 	return rpc->data;
 }
 
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 07/15] drm/nouveau: fix the broken marco GSP_MSG_MAX_SIZE

The macro GSP_MSG_MAX_SIZE refers to another macro that doesn't exist.
It represents the max GSP message element size.

Fix the broken marco so it can be used to replace some magic numbers in
the code.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 92daed69cd46..dab0117d41fd 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -61,7 +61,7 @@
 extern struct dentry *nouveau_debugfs_root;
 
 #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
-#define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
+#define GSP_MSG_MAX_SIZE (GSP_MSG_MIN_SIZE * 16)
 
 /**
  * DOC: GSP message queue element
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 08/15] drm/nouveau: remove the magic number in r535_gsp_rpc_push()

There has been a GSP_MSG_MAX_SIZE which represents the max size of a GSP
message element header. Use it instead of a magic number.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index dab0117d41fd..442a424d0fed 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -796,7 +796,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 {
 	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
 	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
-	const u32 max_msg_size = (16 * 0x1000) - sizeof(struct r535_gsp_msg);
+	const u32 max_msg_size = GSP_MSG_MAX_SIZE - sizeof(*cmd);
 	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
 	u32 rpc_size = rpc->length - sizeof(*rpc);
 	void *repv;
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 09/15] drm/nouveau: refine the variable names in r535_gsp_rpc_push()

The variable names in r535_gsp_rpc_push() are quite confusing and some
of them are not representing what they really are.

Update the names and explanations in the decoder section of the
kernel doc. Refine the names to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 442a424d0fed..49dfc1475973 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -108,6 +108,9 @@ extern struct dentry *nouveau_debugfs_root;
  *
  * Terminology:
  *
+ * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
+ *   payload)
+ * - gsp_rpc(rpc): GSP RPC (RPC header + payload)
  * - gsp_rpc_len: size of (GSP RPC header + payload)
  * - params_size: size of params in the payload
  * - payload_size: size of (header if exists + params) in the payload
@@ -795,30 +798,30 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
-	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
-	const u32 max_msg_size = GSP_MSG_MAX_SIZE - sizeof(*cmd);
-	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
-	u32 rpc_size = rpc->length - sizeof(*rpc);
+	struct r535_gsp_msg *msg = to_gsp_hdr(rpc, msg);
+	const u32 max_rpc_size = GSP_MSG_MAX_SIZE - sizeof(*msg);
+	const u32 max_payload_size = max_rpc_size - sizeof(*rpc);
+	u32 payload_size = rpc->length - sizeof(*rpc);
 	void *repv;
 
 	mutex_lock(&gsp->cmdq.mutex);
-	if (rpc_size > max_rpc_size) {
+	if (payload_size > max_payload_size) {
 		const u32 fn = rpc->function;
 
 		/* Adjust length, and send initial RPC. */
-		rpc->length = sizeof(*rpc) + max_rpc_size;
-		cmd->checksum = rpc->length;
+		rpc->length = sizeof(*rpc) + max_payload_size;
+		msg->checksum = rpc->length;
 
 		repv = r535_gsp_rpc_send(gsp, payload, false, 0);
 		if (IS_ERR(repv))
 			goto done;
 
-		payload += max_rpc_size;
-		rpc_size -= max_rpc_size;
+		payload += max_payload_size;
+		payload_size -= max_payload_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
-		while (rpc_size) {
-			u32 size = min(rpc_size, max_rpc_size);
+		while (payload_size) {
+			u32 size = min(payload_size, max_payload_size);
 			void *next;
 
 			next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD,
size);
@@ -834,7 +837,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 				goto done;
 
 			payload += size;
-			rpc_size -= size;
+			payload_size -= size;
 		}
 
 		/* Wait for reply. */
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 10/15] drm/nouveau: refine the variable names in r535_gsp_msg_recv()

The variable "msg" in r535_gsp_msg_recv() actually means the GSP RPC.

Refine the names to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 47 ++++++++++---------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 49dfc1475973..3b1c648aad46 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -336,59 +336,60 @@ static struct nvfw_gsp_rpc *
 r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
-	struct nvfw_gsp_rpc *msg;
+	struct nvfw_gsp_rpc *rpc;
 	int time = 4000000, i;
 	u32 size;
 
 retry:
-	msg = r535_gsp_msgq_wait(gsp, sizeof(*msg), &size, &time);
-	if (IS_ERR_OR_NULL(msg))
-		return msg;
+	rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
+	if (IS_ERR_OR_NULL(rpc))
+		return rpc;
 
-	msg = r535_gsp_msgq_recv(gsp, msg->length, &time);
-	if (IS_ERR_OR_NULL(msg))
-		return msg;
+	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &time);
+	if (IS_ERR_OR_NULL(rpc))
+		return rpc;
 
-	if (msg->rpc_result) {
-		r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
-		r535_gsp_msg_done(gsp, msg);
+	if (rpc->rpc_result) {
+		r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+		r535_gsp_msg_done(gsp, rpc);
 		return ERR_PTR(-EINVAL);
 	}
 
-	r535_gsp_msg_dump(gsp, msg, NV_DBG_TRACE);
+	r535_gsp_msg_dump(gsp, rpc, NV_DBG_TRACE);
 
-	if (fn && msg->function == fn) {
+	if (fn && rpc->function == fn) {
 		if (gsp_rpc_len) {
-			if (msg->length < sizeof(*msg) + gsp_rpc_len) {
-				nvkm_error(subdev, "msg len %d < %zd\n",
-					   msg->length, sizeof(*msg) +
+			if (rpc->length < sizeof(*rpc) + gsp_rpc_len) {
+				nvkm_error(subdev, "rpc len %d < %zd\n",
+					   rpc->length, sizeof(*rpc) +
 					   gsp_rpc_len);
-				r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
-				r535_gsp_msg_done(gsp, msg);
+				r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+				r535_gsp_msg_done(gsp, rpc);
 				return ERR_PTR(-EIO);
 			}
 
-			return msg;
+			return rpc;
 		}
 
-		r535_gsp_msg_done(gsp, msg);
+		r535_gsp_msg_done(gsp, rpc);
 		return NULL;
 	}
 
 	for (i = 0; i < gsp->msgq.ntfy_nr; i++) {
 		struct nvkm_gsp_msgq_ntfy *ntfy = &gsp->msgq.ntfy[i];
 
-		if (ntfy->fn == msg->function) {
+		if (ntfy->fn == rpc->function) {
 			if (ntfy->func)
-				ntfy->func(ntfy->priv, ntfy->fn, msg->data, msg->length -
sizeof(*msg));
+				ntfy->func(ntfy->priv, ntfy->fn, rpc->data,
+					   rpc->length - sizeof(*rpc));
 			break;
 		}
 	}
 
 	if (i == gsp->msgq.ntfy_nr)
-		r535_gsp_msg_dump(gsp, msg, NV_DBG_WARN);
+		r535_gsp_msg_dump(gsp, rpc, NV_DBG_WARN);
 
-	r535_gsp_msg_done(gsp, msg);
+	r535_gsp_msg_done(gsp, rpc);
 	if (fn)
 		goto retry;
 
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 11/15] drm/nouveau: rename the variable "cmd" to "msg" in r535_gsp_cmdq_{get, push}()

Refine the name to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 3b1c648aad46..864bacbd0ab9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -218,10 +218,10 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *ptime)
 static int
 r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
-	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
+	struct r535_gsp_msg *msg = to_gsp_hdr(rpc, msg);
 	struct r535_gsp_msg *cqe;
-	u32 gsp_rpc_len = cmd->checksum;
-	u64 *ptr = (void *)cmd;
+	u32 gsp_rpc_len = msg->checksum;
+	u64 *ptr = (void *)msg;
 	u64 *end;
 	u64 csum = 0;
 	int free, time = 1000000;
@@ -231,15 +231,15 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 	len = ALIGN(GSP_MSG_HDR_SIZE + gsp_rpc_len, GSP_PAGE_SIZE);
 
 	end = (u64 *)((char *)ptr + len);
-	cmd->pad = 0;
-	cmd->checksum = 0;
-	cmd->sequence = gsp->cmdq.seq++;
-	cmd->elem_count = DIV_ROUND_UP(len, 0x1000);
+	msg->pad = 0;
+	msg->checksum = 0;
+	msg->sequence = gsp->cmdq.seq++;
+	msg->elem_count = DIV_ROUND_UP(len, 0x1000);
 
 	while (ptr < end)
 		csum ^= *ptr++;
 
-	cmd->checksum = upper_32_bits(csum) ^ lower_32_bits(csum);
+	msg->checksum = upper_32_bits(csum) ^ lower_32_bits(csum);
 
 	wptr = *gsp->cmdq.wptr;
 	do {
@@ -254,7 +254,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 		} while(--time);
 
 		if (WARN_ON(!time)) {
-			kvfree(cmd);
+			kvfree(msg);
 			return -ETIMEDOUT;
 		}
 
@@ -262,7 +262,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 		step = min_t(u32, free, (gsp->cmdq.cnt - wptr));
 		size = min_t(u32, len, step * GSP_PAGE_SIZE);
 
-		memcpy(cqe, (u8 *)cmd + off, size);
+		memcpy(cqe, (u8 *)msg + off, size);
 
 		wptr += DIV_ROUND_UP(size, 0x1000);
 		if (wptr == gsp->cmdq.cnt)
@@ -279,23 +279,23 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 
 	nvkm_falcon_wr32(&gsp->falcon, 0xc00, 0x00000000);
 
-	kvfree(cmd);
+	kvfree(msg);
 	return 0;
 }
 
 static void *
 r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 {
-	struct r535_gsp_msg *cmd;
+	struct r535_gsp_msg *msg;
 	u32 size = GSP_MSG_HDR_SIZE + gsp_rpc_len;
 
 	size = ALIGN(size, GSP_MSG_MIN_SIZE);
-	cmd = kvzalloc(size, GFP_KERNEL);
-	if (!cmd)
+	msg = kvzalloc(size, GFP_KERNEL);
+	if (!msg)
 		return ERR_PTR(-ENOMEM);
 
-	cmd->checksum = gsp_rpc_len;
-	return cmd->data;
+	msg->checksum = gsp_rpc_len;
+	return msg->data;
 }
 
 struct nvfw_gsp_rpc {
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 12/15] drm/nouveau: factor out r535_gsp_msgq_peek()

To receive a GSP message queue element from the GSP status queue, the
driver needs to make sure there are available elements in the queue.

The previous r535_gsp_msgq_wait() consists of three functions, which is
a little too complicated for a single function:
- wait for an available element.
- peek the message element header in the queue.
- recevice the element from the queue.

Factor out r535_gsp_msgq_peek() and divide the functions in
r535_gsp_msgq_wait() into three functions.

No functional change is intended.

Cc: Danilo Krummrich <dakr at kernel.org>
Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 89 ++++++++++++++-----
 1 file changed, 66 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 864bacbd0ab9..f603298448f6 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -148,20 +148,16 @@ r535_rpc_status_to_errno(uint32_t rpc_status)
 	}
 }
 
-static void *
-r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, u32 *prepc,
-		   int *ptime)
+static int
+r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
 {
-	struct r535_gsp_msg *mqe;
 	u32 size, rptr = *gsp->msgq.rptr;
 	int used;
-	u8 *msg;
-	u32 len;
 
 	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + gsp_rpc_len,
 			    GSP_PAGE_SIZE);
 	if (WARN_ON(!size || size >= gsp->msgq.cnt))
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	do {
 		u32 wptr = *gsp->msgq.wptr;
@@ -176,15 +172,69 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
u32 *prepc,
 	} while (--(*ptime));
 
 	if (WARN_ON(!*ptime))
-		return ERR_PTR(-ETIMEDOUT);
+		return -ETIMEDOUT;
 
-	mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + rptr * 0x1000);
+	return used;
+}
 
-	if (prepc) {
-		*prepc = (used * GSP_PAGE_SIZE) - sizeof(*mqe);
-		return mqe->data;
-	}
+static struct r535_gsp_msg *
+r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
+{
+	u32 rptr = *gsp->msgq.rptr;
+
+	/* Skip the first page, which is the message queue info */
+	return (void *)((u8 *)gsp->shm.msgq.ptr + GSP_PAGE_SIZE +
+	       rptr * GSP_PAGE_SIZE);
+}
 
+/**
+ * DOC: Receive a GSP message queue element
+ *
+ * Receiving a GSP message queue element from the message queue consists of
+ * the following steps:
+ *
+ * - Peek the element from the queue: r535_gsp_msgq_peek().
+ *   Peek the first page of the element to determine the total size of the
+ *   message before allocating the proper memory.
+ *
+ * - Allocate memory and receive the message: r535_gsp_msgq_recv().
+ *   Once the total size of the message is determined from the GSP message
+ *   queue element, allocate memory and copy the pages of the message
+ *   into the allocated memory.
+ *
+ * - Free the allocated memory after processing the GSP message.
+ *   The caller is responsible for freeing the memory allocated for the GSP
+ *   message pages after they have been processed.
+ */
+static void *
+r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	struct r535_gsp_msg *mqe;
+	int ret;
+
+	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	mqe = r535_gsp_msgq_get_entry(gsp);
+
+	return mqe->data;
+}
+
+static void *
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	u32 rptr = *gsp->msgq.rptr;
+	struct r535_gsp_msg *mqe;
+	u32 size, len;
+	u8 *msg;
+	int ret;
+
+	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	mqe = r535_gsp_msgq_get_entry(gsp);
 	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
@@ -209,12 +259,6 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
u32 *prepc,
 	return msg;
 }
 
-static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
-{
-	return r535_gsp_msgq_wait(gsp, gsp_rpc_len, NULL, ptime);
-}
-
 static int
 r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
@@ -337,15 +381,14 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
 	struct nvfw_gsp_rpc *rpc;
-	int time = 4000000, i;
-	u32 size;
+	int retries = 4000000, i;
 
 retry:
-	rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
+	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &time);
+	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 13/15] drm/nouveau: factor out r535_gsp_msgq_recv_one_elem()

Prepare for supporting receive the large GSP RPC message.

Factor out r535_gsp_msgq_recv_one_elem(). Fold its params into a data
structure of params. Move the allocation of the GSP RPC message to its
caller. Refine the variable names in the re-factor.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 72 +++++++++++++------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index f603298448f6..d1ff038b0d95 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -111,6 +111,7 @@ extern struct dentry *nouveau_debugfs_root;
  * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
  *   payload)
  * - gsp_rpc(rpc): GSP RPC (RPC header + payload)
+ * - gsp_rpc_buf: buffer for (GSP RPC header + payload)
  * - gsp_rpc_len: size of (GSP RPC header + payload)
  * - params_size: size of params in the payload
  * - payload_size: size of (header if exists + params) in the payload
@@ -197,13 +198,16 @@ r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
  *   Peek the first page of the element to determine the total size of the
  *   message before allocating the proper memory.
  *
- * - Allocate memory and receive the message: r535_gsp_msgq_recv().
+ * - Allocate memory for the message.
  *   Once the total size of the message is determined from the GSP message
- *   queue element, allocate memory and copy the pages of the message
- *   into the allocated memory.
+ *   queue element, the caller of r535_gsp_msgq_recv() allocates the
+ *   required memory.
  *
- * - Free the allocated memory after processing the GSP message.
- *   The caller is responsible for freeing the memory allocated for the GSP
+ * - Receive the message: r535_gsp_msgq_recv().
+ *   Copy the message into the allocated memory. Advance the read pointer.
+ *
+ * - Free the allocated memory: r535_gsp_msg_done().
+ *   The user is responsible for freeing the memory allocated for the GSP
  *   message pages after they have been processed.
  */
 static void *
@@ -221,42 +225,70 @@ r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *retries)
 	return mqe->data;
 }
 
+struct r535_gsp_msg_info {
+	int *retries;
+	u32 gsp_rpc_len;
+	void *gsp_rpc_buf;
+};
+
 static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
+			    struct r535_gsp_msg_info *info)
 {
+	u8 *buf = info->gsp_rpc_buf;
 	u32 rptr = *gsp->msgq.rptr;
 	struct r535_gsp_msg *mqe;
-	u32 size, len;
-	u8 *msg;
+	u32 size, expected, len;
 	int ret;
 
-	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	expected = info->gsp_rpc_len;
+
+	ret = r535_gsp_msgq_wait(gsp, expected, info->retries);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
 	mqe = r535_gsp_msgq_get_entry(gsp);
-	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
-
-	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
-	if (!msg)
-		return ERR_PTR(-ENOMEM);
+	size = ALIGN(expected + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
-	len = min_t(u32, gsp_rpc_len, len);
-	memcpy(msg, mqe->data, len);
+	len = min_t(u32, expected, len);
+	memcpy(buf, mqe->data, len);
 
-	gsp_rpc_len -= len;
+	expected -= len;
 
-	if (gsp_rpc_len) {
+	if (expected) {
 		mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
-		memcpy(msg + len, mqe, gsp_rpc_len);
+		memcpy(buf + len, mqe, expected);
 	}
 
 	rptr = (rptr + DIV_ROUND_UP(size, GSP_PAGE_SIZE)) % gsp->msgq.cnt;
 
 	mb();
 	(*gsp->msgq.rptr) = rptr;
-	return msg;
+	return buf;
+}
+
+static void *
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	struct r535_gsp_msg_info info = {0};
+	void *buf;
+
+	buf = kvmalloc(gsp_rpc_len, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	info.gsp_rpc_buf = buf;
+	info.retries = retries;
+	info.gsp_rpc_len = gsp_rpc_len;
+
+	buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
+	if (IS_ERR(buf)) {
+		kvfree(info.gsp_rpc_buf);
+		info.gsp_rpc_buf = NULL;
+	}
+
+	return buf;
 }
 
 static int
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 14/15] drm/nouveau: support handling the return of large GSP message

The max GSP message element size is 16 pages (including the headers). To
send a message larger than 16 pages, nvkm should split it into multiple
and send them accordingly. The first element has the expected function
number, while the rest are sent with function number as
NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD. GSP consumes the elements from
the cmdq and always writes the result back to the msgq. The result is also
formed as split elements.

However, nvkm is able to split the large GSP message and send them, but
totally not aware of handling the return of the large GSP message, which
are the split elements in the msgq. Thus, it keeps dumping the unknown RPC
messages from msgq, which is actually CONTINUATION_RECORD message,
discard them unexpectedly. Thus, the caller will not be able to consume
the result from GSP.

Introduce the handling of the return of large GSP message on the msgq path.
Slightly re-factor the low-level part of msg receiving routines. Merge the
split elements back into a large element before handling it to the upper
level. Thus, the upper-level of GSP RPC APIs don't need to be heavily
changed.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 116 +++++++++++++++---
 1 file changed, 97 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index d1ff038b0d95..40bc3aeb5111 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -106,6 +106,17 @@ extern struct dentry *nouveau_debugfs_root;
  *    |        params          |
  *    +------------------------+
  *
+ * The max size of a message queue element is 16 pages (including the
+ * headers). When a GSP message to be sent is larger than 16 pages, the
+ * message should be split into multiple elements and sent accordingly.
+ *
+ * In the bunch of the split elements, the first element has the expected
+ * function number, while the rest of the elements are sent with the
+ * function number NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD.
+ *
+ * GSP consumes the elements from the cmdq and always writes the result
+ * back to the msgq. The result is also formed as split elements.
+ *
  * Terminology:
  *
  * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
@@ -127,6 +138,21 @@ struct r535_gsp_msg {
 	u8  data[];
 };
 
+struct nvfw_gsp_rpc {
+	u32 header_version;
+	u32 signature;
+	u32 length;
+	u32 function;
+	u32 rpc_result;
+	u32 rpc_result_private;
+	u32 sequence;
+	union {
+		u32 spare;
+		u32 cpuRmGfid;
+	};
+	u8  data[];
+};
+
 #define GSP_MSG_HDR_SIZE offsetof(struct r535_gsp_msg, data)
 
 #define to_gsp_hdr(p, header) \
@@ -205,6 +231,11 @@ r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
  *
  * - Receive the message: r535_gsp_msgq_recv().
  *   Copy the message into the allocated memory. Advance the read pointer.
+ *   If the message is a large GSP message, r535_gsp_msgq_recv() calls
+ *   r535_gsp_msgq_recv_one_elem() repeatedly to receive continuation parts
+ *   until the complete message is received.
+ *   r535_gsp_msgq_recv() assembles the payloads of cotinuation parts into
+ *   the return of the large GSP message.
  *
  * - Free the allocated memory: r535_gsp_msg_done().
  *   The user is responsible for freeing the memory allocated for the GSP
@@ -229,8 +260,12 @@ struct r535_gsp_msg_info {
 	int *retries;
 	u32 gsp_rpc_len;
 	void *gsp_rpc_buf;
+	bool continuation;
 };
 
+static void
+r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
+
 static void *
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 			    struct r535_gsp_msg_info *info)
@@ -248,11 +283,28 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 		return ERR_PTR(ret);
 
 	mqe = r535_gsp_msgq_get_entry(gsp);
+
+	if (info->continuation) {
+		struct nvfw_gsp_rpc *rpc = (struct nvfw_gsp_rpc *)mqe->data;
+
+		if (rpc->function != NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD) {
+			nvkm_error(&gsp->subdev,
+				   "Not a continuation of a large RPC\n");
+			r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+			return ERR_PTR(-EIO);
+		}
+	}
+
 	size = ALIGN(expected + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
 	len = min_t(u32, expected, len);
-	memcpy(buf, mqe->data, len);
+
+	if (info->continuation)
+		memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
+		       len - sizeof(struct nvfw_gsp_rpc));
+	else
+		memcpy(buf, mqe->data, len);
 
 	expected -= len;
 
@@ -271,16 +323,26 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 static void *
 r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
+	struct r535_gsp_msg *mqe;
+	const u32 max_rpc_size = GSP_MSG_MAX_SIZE - sizeof(*mqe);
+	struct nvfw_gsp_rpc *rpc;
 	struct r535_gsp_msg_info info = {0};
+	u32 expected = gsp_rpc_len;
 	void *buf;
 
-	buf = kvmalloc(gsp_rpc_len, GFP_KERNEL);
+	mqe = r535_gsp_msgq_get_entry(gsp);
+	rpc = (struct nvfw_gsp_rpc *)mqe->data;
+
+	if (WARN_ON(rpc->length > max_rpc_size))
+		return NULL;
+
+	buf = kvmalloc(max_t(u32, rpc->length, expected), GFP_KERNEL);
 	if (!buf)
 		return ERR_PTR(-ENOMEM);
 
 	info.gsp_rpc_buf = buf;
 	info.retries = retries;
-	info.gsp_rpc_len = gsp_rpc_len;
+	info.gsp_rpc_len = rpc->length;
 
 	buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
 	if (IS_ERR(buf)) {
@@ -288,6 +350,37 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *retries)
 		info.gsp_rpc_buf = NULL;
 	}
 
+	if (expected <= max_rpc_size)
+		return buf;
+
+	info.gsp_rpc_buf += info.gsp_rpc_len;
+	expected -= info.gsp_rpc_len;
+
+	while (expected) {
+		u32 size;
+
+		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
+		if (IS_ERR_OR_NULL(rpc)) {
+			kfree(buf);
+			return rpc;
+		}
+
+		info.gsp_rpc_len = rpc->length;
+		info.continuation = true;
+
+		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
+		if (IS_ERR_OR_NULL(rpc)) {
+			kfree(buf);
+			return rpc;
+		}
+
+		size = info.gsp_rpc_len - sizeof(*rpc);
+		expected -= size;
+		info.gsp_rpc_buf += size;
+	}
+
+	rpc = buf;
+	rpc->length = gsp_rpc_len;
 	return buf;
 }
 
@@ -374,21 +467,6 @@ r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 	return msg->data;
 }
 
-struct nvfw_gsp_rpc {
-	u32 header_version;
-	u32 signature;
-	u32 length;
-	u32 function;
-	u32 rpc_result;
-	u32 rpc_result_private;
-	u32 sequence;
-	union {
-		u32 spare;
-		u32 cpuRmGfid;
-	};
-	u8  data[];
-};
-
 static void
 r535_gsp_msg_done(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg)
 {
@@ -420,7 +498,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &retries);
+	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-- 
2.34.1

Zhi Wang

2025-Jan-24 18:29 UTC

head link

[PATCH v4 15/15] drm/nouveau: consume the return of large GSP message

As the GSP message recv path is able to handle the return of large GSP
message, consume the return of large GSP message in the sending path.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 32 +++++++++++--------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 40bc3aeb5111..db978ffb3993 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -512,10 +512,9 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 
 	if (fn && rpc->function == fn) {
 		if (gsp_rpc_len) {
-			if (rpc->length < sizeof(*rpc) + gsp_rpc_len) {
-				nvkm_error(subdev, "rpc len %d < %zd\n",
-					   rpc->length, sizeof(*rpc) +
-					   gsp_rpc_len);
+			if (rpc->length < gsp_rpc_len) {
+				nvkm_error(subdev, "rpc len %d < %d\n",
+					   rpc->length, gsp_rpc_len);
 				r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
 				r535_gsp_msg_done(gsp, rpc);
 				return ERR_PTR(-EIO);
@@ -961,6 +960,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 	mutex_lock(&gsp->cmdq.mutex);
 	if (payload_size > max_payload_size) {
 		const u32 fn = rpc->function;
+		u32 remain_payload_size = payload_size;
 
 		/* Adjust length, and send initial RPC. */
 		rpc->length = sizeof(*rpc) + max_payload_size;
@@ -971,11 +971,12 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 			goto done;
 
 		payload += max_payload_size;
-		payload_size -= max_payload_size;
+		remain_payload_size -= max_payload_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
-		while (payload_size) {
-			u32 size = min(payload_size, max_payload_size);
+		while (remain_payload_size) {
+			u32 size = min(remain_payload_size,
+				       max_payload_size);
 			void *next;
 
 			next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD,
size);
@@ -991,18 +992,21 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 				goto done;
 
 			payload += size;
-			payload_size -= size;
+			remain_payload_size -= size;
 		}
 
 		/* Wait for reply. */
-		if (wait) {
-			rpc = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
-			if (!IS_ERR_OR_NULL(rpc))
+		rpc = r535_gsp_msg_recv(gsp, fn, payload_size +
+					sizeof(*rpc));
+		if (!IS_ERR_OR_NULL(rpc)) {
+			if (wait) {
 				repv = rpc->data;
-			else
-				repv = rpc;
+			} else {
+				nvkm_gsp_rpc_done(gsp, rpc);
+				repv = NULL;
+			}
 		} else {
-			repv = NULL;
+			repv = wait ? rpc : NULL;
 		}
 	} else {
 		repv = r535_gsp_rpc_send(gsp, payload, wait, gsp_rpc_len);
-- 
2.34.1

Danilo Krummrich

2025-Jan-24 23:57 UTC

head link

[PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

Hi Zhi,

On Fri, Jan 24, 2025 at 10:29:43AM -0800, Zhi Wang
wrote:> Hi folks:
> 
> Here is another re-spin of handling the large GSP message return.
> 
> Besides the support of the large GSP message, kernel doc and many cleanups
> are introduced according to the comments in the previous spin [1].
> 
> For testing, I tried the following without any problem:
> 
> - run Uniengine Heaven [2] on my RTX 4060 for 8 hours
> - the GL CTS runner [3] (commandline: ./cts-runner --type-gl40) from
> Khronos
> - with the vGPU RFC patches [4], boot up a ubuntu VM with a vGPU on a L40
> and run glmark2.
Applied to drm-misc-next.

Thanks a lot for this series and the iterations -- those are really good
improvements.

I fixed up the first commit "drm/nouveau: add a kernel doc to introduce the
GSP
RPC", which had some indentation issues in the bullet list and the new file
was
missing the SPDX-License-Identifier.

When adding documentation, please make sure to check the result after running
the htmldocs make target.

I also fixed up one minor indentation issue in one of the other patches.

Again, thanks for the series!

- Danilo
> 
> v4:
> - Introduce a documentation in Documentation/gpu/nouveau.rst.
> - Fix the reference link of NVIDIA SDK headers in the document.
> - Replace the "decoders" term with "terminology".
> - Explain the adding offset in r535_gsp_msgq_get_entry().
> - Replace the magic number 0x1000 with GSP_PAGE_SIZE in the re-factor.
> - Introduce a DOC to explain the GSP message receiving flow.
> 
> v3:
> 
> - Add a kernel doc and chart to introduce the GSP message and denote the
> memory layout.
> - Add a decoder section in the kernel doc to explain the terms in the code.
> - Refine the many confusing variable names to align with the terms in the
> kernel doc.
> - Introduce the continaution records in the kernel doc.
> - Re-factor the complicated function r535_gsp_msgq_wait().
> - Other minor cleanups.
> 
> v2:
> 
> - Remove the Fixes: tags as the vanilla nouveau aren't going to hit
these bugs. (Danilo)
> - Test the patchset on VK-GL-CTS. (Danilo)
> - Remove the ambiguous empty line in PATCH 2. (Danilo)
> - Rename the r535_gsp_msgq_wait to gsp_msgq_recv. (Danilo)
> - Introduce a data structure to hold the params of gsp_msgq_recv().
(Danilo)
> - Document the params and the states they are related to. (Danilo)
> 
> [1] https://lore.kernel.org/nouveau/20241031085250.2941482-1-zhiw at
nvidia.com/
> [2] https://benchmark.unigine.com/heaven
> [3] https://github.com/KhronosGroup/VK-GL-CTS
> [4] https://lore.kernel.org/kvm/20240922124951.1946072-1-zhiw at
nvidia.com/T/#t
> 
> 
> Zhi Wang (15):
>   drm/nouveau: add a kernel doc to introduce the GSP RPC
>   drm/nouveau: rename "repc" to "gsp_rpc_len" on the
GSP message recv
>     path
>   drm/nouveau: rename "argv" to what it represents on the GSP
message
>     send path
>   drm/nouveau: remove unused param repc in *rm_alloc_push()
>   drm/nouveau: rename "argv" to what it represents in *rm_{alloc,
>     ctrl}_*()
>   drm/nouveau: rename "argc" to what it represents in GSP RPC
routines
>   drm/nouveau: fix the broken marco GSP_MSG_MAX_SIZE
>   drm/nouveau: remove the magic number in r535_gsp_rpc_push()
>   drm/nouveau: refine the variable names in r535_gsp_rpc_push()
>   drm/nouveau: refine the variable names in r535_gsp_msg_recv()
>   drm/nouveau: rename the variable "cmd" to "msg" in
r535_gsp_cmdq_{get,
>     push}()
>   drm/nouveau: factor out r535_gsp_msgq_peek()
>   drm/nouveau: factor out r535_gsp_msgq_recv_one_elem()
>   drm/nouveau: support handling the return of large GSP message
>   drm/nouveau: consume the return of large GSP message
> 
>  Documentation/gpu/drivers.rst                 |   1 +
>  Documentation/gpu/nouveau.rst                 |  27 +
>  .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |   8 +-
>  .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 526 +++++++++++++-----
>  4 files changed, 409 insertions(+), 153 deletions(-)
>  create mode 100644 Documentation/gpu/nouveau.rst
> 
> -- 
> 2.34.1
>

Nouveau - Jan 2025 - [PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

[PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

[PATCH v4 01/15] drm/nouveau: add a kernel doc to introduce the GSP RPC

[PATCH v4 02/15] drm/nouveau: rename "repc" to "gsp_rpc_len" on the GSP message recv path

[PATCH v4 03/15] drm/nouveau: rename "argv" to what it represents on the GSP message send path

[PATCH v4 04/15] drm/nouveau: remove unused param repc in *rm_alloc_push()

[PATCH v4 05/15] drm/nouveau: rename "argv" to what it represents in *rm_{alloc, ctrl}_*()

[PATCH v4 06/15] drm/nouveau: rename "argc" to what it represents in GSP RPC routines

[PATCH v4 07/15] drm/nouveau: fix the broken marco GSP_MSG_MAX_SIZE

[PATCH v4 08/15] drm/nouveau: remove the magic number in r535_gsp_rpc_push()

[PATCH v4 09/15] drm/nouveau: refine the variable names in r535_gsp_rpc_push()

[PATCH v4 10/15] drm/nouveau: refine the variable names in r535_gsp_msg_recv()

[PATCH v4 11/15] drm/nouveau: rename the variable "cmd" to "msg" in r535_gsp_cmdq_{get, push}()

[PATCH v4 12/15] drm/nouveau: factor out r535_gsp_msgq_peek()

[PATCH v4 13/15] drm/nouveau: factor out r535_gsp_msgq_recv_one_elem()

[PATCH v4 14/15] drm/nouveau: support handling the return of large GSP message

[PATCH v4 15/15] drm/nouveau: consume the return of large GSP message

[PATCH v4 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

[PATCH v4 05/15] drm/nouveau: rename "argv" to what it represents in rm_{alloc, ctrl}_()