The RPC parsing should not be done in the same method as the transport header parsing. Split it out and introduce a `RpcMessage` type to indicate messages that have been successfully parsed as RPC. This makes the code easier to move around into its new destination.
Signed-off-by: Alexandre Courbot <[email protected]> --- drivers/gpu/nova-core/gsp/cmdq.rs | 112 ++++++++++++++++++++++++-------------- 1 file changed, 70 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index dee2520d3124..2ffb8f4b2ef4 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -554,11 +554,69 @@ struct GspCommand<'a> { struct GspMessage<'a> { // Reference to the header of the message. header: &'a GspMsgElement, - // Slices to the contents of the message. The second slice is zero unless the message loops + // Slices to the contents of the message. The second slice is empty unless the message loops // over the message queue. contents: (&'a [u8], &'a [u8]), } +/// A RPC message ready to be processed from the message queue. +struct RpcMessage<'a> { + // Reference to the RPC header of the message. + header: &'a RpcMessageHeader, + // Slices to the contents of the message. The second slice is empty unless the message loops + // over the message queue. + contents: (&'a [u8], &'a [u8]), +} + +impl<'a> RpcMessage<'a> { + /// Validate the RPC layer of `element` and returns its RPC header and its contents trimmed down + /// to the RPC payload. + /// + /// # Errors + /// + /// - `EIO` if the element is shorter than the payload length advertised by the RPC header. + fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<RpcMessage<'a>> { + let GspMessage { + header, + contents: (slice_1, slice_2), + } = element; + + let rpc_header = header.rpc_header(); + let payload_length = rpc_header.rpc_length(); + + dev_dbg!( + dev, + "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n", + rpc_header.sequence(), + rpc_header.function(), + payload_length, + ); + + // Check that the driver read area is large enough for the message. + if slice_1.len() + slice_2.len() < payload_length { + return Err(EIO); + } + + // Cut the message slices down to the actual length of the message. + let (slice_1, slice_2) = if slice_1.len() > payload_length { + // PANIC: we checked above that `slice_1` is at least as long as `payload_length`. + (slice_1.split_at(payload_length).0, &slice_2[0..0]) + } else { + ( + slice_1, + // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as + // large as `payload_length`. + slice_2.split_at(payload_length - slice_1.len()).0, + ) + }; + + Ok(RpcMessage { + header: rpc_header, + contents: (slice_1, slice_2), + }) + } +} + /// GSP command queue. /// /// Provides the ability to send commands and receive messages from the GSP using a shared memory @@ -841,35 +899,6 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { return Err(EIO); } - let rpc_header = header.rpc_header(); - let payload_length = rpc_header.rpc_length(); - - dev_dbg!( - &self.dev, - "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n", - rpc_header.sequence(), - rpc_header.function(), - payload_length, - ); - - // Check that the driver read area is large enough for the message. - if slice_1.len() + slice_2.len() < payload_length { - return Err(EIO); - } - - // Cut the message slices down to the actual length of the message. - let (slice_1, slice_2) = if slice_1.len() > payload_length { - // PANIC: we checked above that `slice_1` is at least as long as `payload_length`. - (slice_1.split_at(payload_length).0, &slice_2[0..0]) - } else { - ( - slice_1, - // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as - // large as `payload_length`. - slice_2.split_at(payload_length - slice_1.len()).0, - ) - }; - Ok(GspMessage { header, contents: (slice_1, slice_2), @@ -897,14 +926,15 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M> Error: From<M::InitError>, { let message = self.wait_for_msg(timeout)?; - let rpc_header = message.header.rpc_header(); - let function = rpc_header.function(); - let seq = rpc_header.sequence(); + let elem_count = u32::try_from(message.header.msg_length().div_ceil(GSP_PAGE_SIZE))?; + let rpc_message = RpcMessage::parse(self.dev, message)?; + let function = rpc_message.header.function(); // An early return here would leave the read pointer on this message. let result = if matches!(function, Ok(f) if f == M::FUNCTION) { - let (cmd, contents_1) = M::Message::from_bytes_prefix(message.contents.0).ok_or(EIO)?; - let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]); + let (cmd, contents_1) = + M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?; + let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]); M::read(cmd, &mut sbuffer) .map_err(|e| e.into()) @@ -918,15 +948,13 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M> } }) } else { - self.log_event(function, seq); + self.log_event(function, rpc_message.header.sequence()); Err(ENOMSG) }; // Advance the read pointer past this message. - self.gsp_mem.advance_cpu_read_ptr(u32::try_from( - message.header.msg_length().div_ceil(GSP_PAGE_SIZE), - )?); + self.gsp_mem.advance_cpu_read_ptr(elem_count); result } @@ -1008,9 +1036,9 @@ fn drain(&mut self) -> Result { dev_err!(&self.dev, "GSP drain: message length overflow\n"); EIO })?; - let rpc_header = msg.header.rpc_header(); - let function = rpc_header.function(); - let seq = rpc_header.sequence(); + let rpc_msg = RpcMessage::parse(self.dev, msg)?; + let function = rpc_msg.header.function(); + let seq = rpc_msg.header.sequence(); self.gsp_mem.advance_cpu_read_ptr(pages); self.log_event(function, seq); -- 2.56.0
