`receive_msg` currently handles both the transport and RPC layers. Retire it and split its logic between the new `consume_element`, a transport-level method which passes the parsed message contents to a closure before advancing the CPU read pointer past it, and the already-existing `await_msg`, which calls `consume_element` with a closure matching the RPC message.
This introduces a small behavioral change: a matched RPC message that is too short to contain the message component itself no longer stalls the queue. This behavior is justified because it fixes an inconsistency: unmatched RPC messages were always skipped, whereas matched RPC messages could block the queue if they were too short. While a misconstructed RPC message is a problem, it is not a fatal queue error as long as the transport framing and checksum are valid. Signed-off-by: Alexandre Courbot <[email protected]> --- Documentation/gpu/nova/core/interrupts.rst | 7 +-- drivers/gpu/nova-core/gsp/cmdq.rs | 93 +++++++++++++++++------------- 2 files changed, 56 insertions(+), 44 deletions(-) diff --git a/Documentation/gpu/nova/core/interrupts.rst b/Documentation/gpu/nova/core/interrupts.rst index e42286915711..13a087b960c0 100644 --- a/Documentation/gpu/nova/core/interrupts.rst +++ b/Documentation/gpu/nova/core/interrupts.rst @@ -583,13 +583,12 @@ the command's sequence number on every reply. On r570 the reply to ``UnloadingGuestDriver`` carries sequence 0. The read pointer advances past every message that passes framing and checksum -validation, whether it matched or was an event. A matched message that is too -short to decode is the exception, and it stays at the queue head. +validation, whether it matched or was an event. A message's length is inside the region that the checksum covers, so once the framing or the checksum fails there is no trustworthy length with which to skip -the message. Such a message also stays at the queue head, and every later -receive fails on it with ``EIO`` until the device is reset. +the message. Such a message stays at the queue head, and every later receive +fails on it with ``EIO`` until the device is reset. The polling path and the IRQ thread both read the queue under the command-queue mutex. Replies and events share one queue and one read pointer, so one lock is diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 4efc96eba4dd..58761a5f609f 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -645,6 +645,12 @@ fn log(&self, dev: &device::Device) { } } +/// A message whose protocol layer has been parsed and validated. +enum GspParsedMessage<'a> { + /// A [RPC message](RpcMessage). + Rpc(RpcMessage<'a>), +} + /// GSP command queue. /// /// Provides the ability to send commands and receive messages from the GSP using a shared memory @@ -930,57 +936,39 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { }) } - /// Receive a message from the GSP. + /// Wait for the next element on the message queue, pass it to `f`, advance the read pointer + /// past the message, and returns the result of `f`. /// - /// A message whose function code is `M::FUNCTION` is decoded and returned. Any other message - /// is logged as an event. - /// - /// The read pointer is always advanced past the message, regardless of whether it matched. + /// The read pointer advances regardless of whether `f` succeeds or not. /// /// # Errors /// /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available. - /// - `EIO` if a message fails framing or checksum validation. - /// - `ENOMSG` if the message was not the awaited reply. + /// - `EIO` if a message fails framing or checksum validation. In this case, the read pointer + /// does not advance and the queue is stalled. This is a fatal error. /// - /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is. - fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M> - where - // This allows all error types, including `Infallible`, to be used for `M::InitError`. - Error: From<M::InitError>, - { + /// If an error is returned, then `f` has not been called. + fn consume_element<R>( + &mut self, + timeout: Delta, + f: impl FnOnce(GspParsedMessage<'_>) -> R, + ) -> Result<R> { let message = self.wait_for_msg(timeout)?; - 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(); + let elem_count = u32::try_from(message.header.msg_length().div_ceil(GSP_PAGE_SIZE)) + .map_err(|_| { + dev_err!(&self.dev, "GSP receive: message length overflow\n"); + EIO + })?; - // 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(rpc_message.contents.0).ok_or(EIO)?; - let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]); + // RPC is the only type of message supported for now. + let message = GspParsedMessage::Rpc(RpcMessage::parse(self.dev, message)?); - M::read(cmd, &mut sbuffer) - .map_err(|e| e.into()) - .inspect(|_| { - if !sbuffer.is_empty() { - dev_warn!( - &self.dev, - "GSP message {:?} has unprocessed data\n", - M::FUNCTION - ); - } - }) - } else { - rpc_message.log(self.dev); - - Err(ENOMSG) - }; + let res = f(message); // Advance the read pointer past this message. self.gsp_mem.advance_cpu_read_ptr(elem_count); - result + Ok(res) } /// Receives a message of type `M`, waiting up to [`Cmdq::RECEIVE_TIMEOUT`] from the call. @@ -1000,14 +988,39 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M> // This allows all error types, including `Infallible`, to be used for `M::InitError`. Error: From<M::InitError>, { + let f = |message: GspParsedMessage<'_>| { + let GspParsedMessage::Rpc(rpc_message) = message; + let function = rpc_message.header.function(); + if matches!(function, Ok(f) if f == M::FUNCTION) { + 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()) + .inspect(|_| { + if !sbuffer.is_empty() { + dev_warn!( + self.dev, + "GSP message {:?} has unprocessed data\n", + M::FUNCTION + ); + } + }) + } else { + rpc_message.log(self.dev); + + Err(ENOMSG) + } + }; let deadline = Instant::<Monotonic>::now() + Cmdq::RECEIVE_TIMEOUT; loop { let remaining = deadline - Instant::<Monotonic>::now(); if remaining.is_negative() { break Err(ETIMEDOUT); } - match self.receive_msg::<M>(remaining) { - Ok(msg) => break Ok(msg), + match self.consume_element(remaining, f)? { + Ok(m) => break Ok(m), Err(ENOMSG) => continue, Err(e) => break Err(e), } -- 2.56.0
