On Sun Sep 27, 2026 at 10:46 PM JST, Alexandre Courbot wrote: > `wait_for_msg` mixes two layers: the transport layer which polls the > queue, extracts the element header and validates the checksum, and the > RPC layer which reads the RPC header and trims the payload slices to the > length advertised by the RPC header. > > Move the transport layer into `wait_for_element`, and introduce > `consume_element`, a transport-level method which runs a closure on the > next element before advancing the CPU read pointer past it, and > `parse_rpc_message`, which validates the RPC layer. This sets things up > for moving the RPC code into its own module, leaving the transport > agnostic of the message type. > > Signed-off-by: Alexandre Courbot <[email protected]> > --- > drivers/gpu/nova-core/gsp/cmdq.rs | 91 > ++++++++++++++++++++++++++++----------- > 1 file changed, 65 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs > b/drivers/gpu/nova-core/gsp/cmdq.rs > index 640afe2e29cb..169ef0865339 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -411,7 +411,7 @@ struct GspCommand<'a> { > > /// A message ready to be processed from the message queue. > /// > -/// This is the type returned by [`CmdqInner::wait_for_msg`]. > +/// This is the type returned by [`CmdqInner::wait_for_element`]. > struct GspMessage<'a> { > // Reference to the header of the message. > header: &'a GspMsgElement, > @@ -566,7 +566,7 @@ fn send_command_element( > Ok(()) > } > > - /// Wait for a message to become available on the message queue. > + /// Wait for the next element to become available on the message queue. > /// > /// This works purely at the transport layer and does not interpret or > validate the message > /// beyond the advertised length in its [`GspMsgElement`]. > @@ -584,7 +584,7 @@ fn send_command_element( > /// message queue. > /// > /// Error codes returned by the message constructor are propagated as-is. > - fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { > + fn wait_for_element(&self, timeout: Delta) -> Result<GspMessage<'_>> { > // Wait for a message to arrive from the GSP. > let (slice_1, slice_2) = read_poll_timeout( > || Ok(self.gsp_mem.driver_read_area()), > @@ -606,18 +606,65 @@ fn wait_for_msg(&self, timeout: Delta) -> > Result<GspMessage<'_>> { > return Err(EIO); > } > > + Ok(GspMessage { > + header, > + contents: (slice_1, slice_2), > + }) > + } > + > + /// Wait for the next element on the message queue, pass it to > `process_element`, and advances > + /// the read pointer past it. > + /// > + /// The read pointer advances regardless of whether `process_element` > succeeds or not. > + /// > + /// # Errors > + /// > + /// Errors from [`Self::wait_for_element`] and from `process_element` > are propagated as-is. > + fn consume_element<R>( > + &mut self, > + timeout: Delta, > + process_element: impl FnOnce(GspMessage<'_>) -> Result<R>, > + ) -> Result<R> { > + let (elem_count, result) = { > + let message = self.wait_for_element(timeout)?; > + > + ( > + > u32::try_from(message.header.length().div_ceil(GSP_PAGE_SIZE))?, > + process_element(message), > + ) > + }; > + > + self.gsp_mem.advance_cpu_read_ptr(elem_count); > + > + result > + } > + > + /// Validate the RPC layer of `element` and trim its contents down to > the RPC payload. > + /// > + /// # Errors > + /// > + /// - `EIO` if the element is shorter than the payload length advertised > by the RPC header. > + fn parse_rpc_message<'a>( > + dev: &device::Device, > + element: GspMessage<'a>, > + ) -> Result<GspMessage<'a>> {
Since this trims the payload, we should return a different type (`RpcMessage`?) with a field referring to the RPC header, otherwise we risk getting confused as to which headers are in the payload or not. Not re-sending just for that, but wanted to flag this for v3.
