On Wed, 25 Feb 2026 22:41:51 +0900 Eliot Courtney <[email protected]> wrote:
Looks good to me. Reviewed-by: Zhi Wang <[email protected]> > Wrap `Cmdq`'s mutable state in a new struct `CmdqInner` and wrap that > in a Mutex. This lets `Cmdq` methods take &self instead of &mut self, > which lets required commands be sent e.g. while unloading the driver. > > The mutex is held over both send and receive in `send_sync_command` to > make sure that it doesn't get the reply of some other command that > could have been sent just beforehand. > > Signed-off-by: Eliot Courtney <[email protected]> > --- > drivers/gpu/nova-core/gsp/boot.rs | 8 +- > drivers/gpu/nova-core/gsp/cmdq.rs | 260 > ++++++++++++++++++--------------- > drivers/gpu/nova-core/gsp/commands.rs | 4 +- > drivers/gpu/nova-core/gsp/sequencer.rs | 2 +- 4 files changed, 152 > insertions(+), 122 deletions(-) > > diff --git a/drivers/gpu/nova-core/gsp/boot.rs > b/drivers/gpu/nova-core/gsp/boot.rs index 1cb21da855b9..cb583f57666a > 100644 --- a/drivers/gpu/nova-core/gsp/boot.rs > +++ b/drivers/gpu/nova-core/gsp/boot.rs > @@ -128,7 +128,7 @@ fn run_fwsec_frts( > /// > /// Upon return, the GSP is up and running, and its runtime > object given as return value. pub(crate) fn boot( > - mut self: Pin<&mut Self>, > + self: Pin<&mut Self>, > pdev: &pci::Device<device::Bound>, > bar: &Bar0, > chipset: Chipset, > @@ -232,13 +232,13 @@ pub(crate) fn boot( > dev: pdev.as_ref().into(), > bar, > }; > - GspSequencer::run(&mut self.cmdq, seq_params)?; > + GspSequencer::run(&self.cmdq, seq_params)?; > > // Wait until GSP is fully initialized. > - commands::wait_gsp_init_done(&mut self.cmdq)?; > + commands::wait_gsp_init_done(&self.cmdq)?; > > // Obtain and display basic GPU information. > - let info = commands::get_gsp_info(&mut self.cmdq, bar)?; > + let info = commands::get_gsp_info(&self.cmdq, bar)?; > match info.gpu_name() { > Ok(name) => dev_info!(pdev.as_ref(), "GPU name: {}\n", > name), Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: > {:?}\n", e), diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs > b/drivers/gpu/nova-core/gsp/cmdq.rs index 44c3e960c965..faf1e9d5072b > 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs > @@ -16,8 +16,12 @@ > }, > dma_write, > io::poll::read_poll_timeout, > + new_mutex, > prelude::*, > - sync::aref::ARef, > + sync::{ > + aref::ARef, > + Mutex, // > + }, > time::Delta, > transmute::{ > AsBytes, > @@ -54,8 +58,8 @@ > > /// Trait implemented by types representing a command to send to the > GSP. /// > -/// The main purpose of this trait is to provide > [`Cmdq::send_command`] with the information it -/// needs to send a > given command. +/// The main purpose of this trait is to provide > [`Cmdq`] with the information it needs to send +/// a given command. > /// > /// [`CommandToGsp::init`] in particular is responsible for > initializing the command directly /// into the space reserved for it > in the command queue buffer. @@ -470,66 +474,15 @@ pub(crate) fn > command_size<M>(command: &M) -> usize size_of::<M::Command>() + > command.variable_payload_len() } > > -/// GSP command queue. > -/// > -/// Provides the ability to send commands and receive messages from > the GSP using a shared memory -/// area. > -#[pin_data] > -pub(crate) struct Cmdq { > - /// Device this command queue belongs to. > - dev: ARef<device::Device>, > +/// Inner mutex protected state of [`Cmdq`]. > +struct CmdqInner { > /// Current command sequence number. > seq: u32, > /// Memory area shared with the GSP for communicating commands > and messages. gsp_mem: DmaGspMem, > } > > -impl Cmdq { > - /// Offset of the data after the PTEs. > - const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, > cpuq); - > - /// Offset of command queue ring buffer. > - pub(crate) const CMDQ_OFFSET: usize = > core::mem::offset_of!(GspMem, cpuq) > - + core::mem::offset_of!(Msgq, msgq) > - - Self::POST_PTE_OFFSET; > - > - /// Offset of message queue ring buffer. > - pub(crate) const STATQ_OFFSET: usize = > core::mem::offset_of!(GspMem, gspq) > - + core::mem::offset_of!(Msgq, msgq) > - - Self::POST_PTE_OFFSET; > - > - /// Number of page table entries for the GSP shared region. > - pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> > GSP_PAGE_SHIFT; - > - /// Creates a new command queue for `dev`. > - pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl > PinInit<Self, Error> + '_ { > - try_pin_init!(Self { > - gsp_mem: DmaGspMem::new(dev)?, > - dev: dev.into(), > - seq: 0, > - }) > - } > - > - /// Computes the checksum for the message pointed to by `it`. > - /// > - /// A message is made of several parts, so `it` is an iterator > over byte slices representing > - /// these parts. > - fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 { > - let sum64 = it > - .enumerate() > - .map(|(idx, byte)| (((idx % 8) * 8) as u32, byte)) > - .fold(0, |acc, (rol, byte)| acc ^ > u64::from(byte).rotate_left(rol)); - > - ((sum64 >> 32) as u32) ^ (sum64 as u32) > - } > - > - /// Notifies the GSP that we have updated the command queue > pointers. > - fn notify_gsp(bar: &Bar0) { > - regs::NV_PGSP_QUEUE_HEAD::default() > - .set_address(0) > - .write(bar); > - } > - > +impl CmdqInner { > /// Sends `command` to the GSP, without splitting it. > /// > /// # Errors > @@ -540,7 +493,7 @@ fn notify_gsp(bar: &Bar0) { > /// written to by its [`CommandToGsp::init_variable_payload`] > method. /// > /// Error codes returned by the command initializers are > propagated as-is. > - fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> > Result > + fn send_single_command<M>(&mut self, dev: &device::Device, bar: > &Bar0, command: M) -> Result where > M: CommandToGsp, > // This allows all error types, including `Infallible`, to > be used for `M::InitError`. @@ -583,7 +536,7 @@ fn > send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result > ]))); > dev_dbg!( > - &self.dev, > + dev, > "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n", > self.seq, > M::FUNCTION, > @@ -610,73 +563,27 @@ fn send_single_command<M>(&mut self, bar: > &Bar0, command: M) -> Result /// written to by its > [`CommandToGsp::init_variable_payload`] method. /// > /// Error codes returned by the command initializers are > propagated as-is. > - fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result > + fn send_command<M>(&mut self, dev: &device::Device, bar: &Bar0, > command: M) -> Result where > M: CommandToGsp, > Error: From<M::InitError>, > { > let mut state = SplitState::new(&command)?; > - > - self.send_single_command(bar, state.command(command))?; > + self.send_single_command(dev, bar, state.command(command))?; > > while let Some(continuation) = > state.next_continuation_record() { dev_dbg!( > - &self.dev, > + dev, > "GSP RPC: send continuation: size=0x{:x}\n", > command_size(&continuation), > ); > // Turbofish needed because the compiler cannot infer M > here. > - self.send_single_command::<ContinuationRecord<'_>>(bar, > continuation)?; > + self.send_single_command::<ContinuationRecord<'_>>(dev, > bar, continuation)?; } > > Ok(()) > } > > - /// Sends `command` to the GSP and waits for the reply. > - /// > - /// # Errors > - /// > - /// - `ETIMEDOUT` if space does not become available to send the > command, or if the reply is > - /// not received within the timeout. > - /// - `EIO` if the variable payload requested by the command has > not been entirely > - /// written to by its [`CommandToGsp::init_variable_payload`] > method. > - /// > - /// Error codes returned by the command and reply initializers > are propagated as-is. > - pub(crate) fn send_sync_command<M>(&mut self, bar: &Bar0, > command: M) -> Result<M::Reply> > - where > - M: CommandToGsp, > - M::Reply: MessageFromGsp, > - Error: From<M::InitError>, > - Error: From<<M::Reply as MessageFromGsp>::InitError>, > - { > - self.send_command(bar, command)?; > - > - loop { > - match self.receive_msg::<M::Reply>(Delta::from_secs(10)) > { > - Ok(reply) => break Ok(reply), > - Err(ERANGE) => continue, > - Err(e) => break Err(e), > - } > - } > - } > - > - /// Sends `command` to the GSP without waiting for a reply. > - /// > - /// # Errors > - /// > - /// - `ETIMEDOUT` if space does not become available within the > timeout. > - /// - `EIO` if the variable payload requested by the command has > not been entirely > - /// written to by its [`CommandToGsp::init_variable_payload`] > method. > - /// > - /// Error codes returned by the command initializers are > propagated as-is. > - pub(crate) fn send_async_command<M>(&mut self, bar: &Bar0, > command: M) -> Result > - where > - M: CommandToGsp<Reply = NoReply>, > - Error: From<M::InitError>, > - { > - self.send_command(bar, command) > - } > - > /// Wait for a message to become available on the message queue. > /// > /// This works purely at the transport layer and does not > interpret or validate the message @@ -695,7 +602,7 @@ pub(crate) fn > send_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result > /// 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_msg(&self, dev: &device::Device, 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()), > @@ -712,7 +619,7 @@ fn wait_for_msg(&self, timeout: Delta) -> > Result<GspMessage<'_>> { let (header, slice_1) = > GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?; > dev_dbg!( > - self.dev, > + dev, > "GSP RPC: receive: seq# {}, function={:?}, > length=0x{:x}\n", header.sequence(), > header.function(), > @@ -747,7 +654,7 @@ fn wait_for_msg(&self, timeout: Delta) -> > Result<GspMessage<'_>> { ])) != 0 > { > dev_err!( > - self.dev, > + dev, > "GSP RPC: receive: Call {} - bad checksum\n", > header.sequence() > ); > @@ -776,12 +683,12 @@ fn wait_for_msg(&self, timeout: Delta) -> > Result<GspMessage<'_>> { /// - `ERANGE` if the message had a > recognized but non-matching function code. /// > /// Error codes returned by [`MessageFromGsp::read`] are > propagated as-is. > - pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: > Delta) -> Result<M> > + fn receive_msg<M: MessageFromGsp>(&mut self, dev: > &device::Device, timeout: Delta) -> Result<M> where > // This allows all error types, including `Infallible`, to > be used for `M::InitError`. Error: From<M::InitError>, > { > - let message = self.wait_for_msg(timeout)?; > + let message = self.wait_for_msg(dev, timeout)?; > let function = message.header.function().map_err(|_| > EINVAL)?; > // Extract the message. Store the result as we want to > advance the read pointer even in @@ -802,9 +709,132 @@ pub(crate) fn > receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul > result > } > +} > + > +/// GSP command queue. > +/// > +/// Provides the ability to send commands and receive messages from > the GSP using a shared memory +/// area. > +#[pin_data] > +pub(crate) struct Cmdq { > + /// Device this command queue belongs to. > + dev: ARef<device::Device>, > + /// Inner mutex-protected state. > + #[pin] > + inner: Mutex<CmdqInner>, > +} > + > +impl Cmdq { > + /// Offset of the data after the PTEs. > + const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, > cpuq); + > + /// Offset of command queue ring buffer. > + pub(crate) const CMDQ_OFFSET: usize = > core::mem::offset_of!(GspMem, cpuq) > + + core::mem::offset_of!(Msgq, msgq) > + - Self::POST_PTE_OFFSET; > + > + /// Offset of message queue ring buffer. > + pub(crate) const STATQ_OFFSET: usize = > core::mem::offset_of!(GspMem, gspq) > + + core::mem::offset_of!(Msgq, msgq) > + - Self::POST_PTE_OFFSET; > + > + /// Number of page table entries for the GSP shared region. > + pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> > GSP_PAGE_SHIFT; + > + /// Creates a new command queue for `dev`. > + pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl > PinInit<Self, Error> + '_ { > + try_pin_init!(Self { > + inner <- new_mutex!(CmdqInner { > + gsp_mem: DmaGspMem::new(dev)?, > + seq: 0, > + }), > + dev: dev.into(), > + }) > + } > + > + /// Computes the checksum for the message pointed to by `it`. > + /// > + /// A message is made of several parts, so `it` is an iterator > over byte slices representing > + /// these parts. > + fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 { > + let sum64 = it > + .enumerate() > + .map(|(idx, byte)| (((idx % 8) * 8) as u32, byte)) > + .fold(0, |acc, (rol, byte)| acc ^ > u64::from(byte).rotate_left(rol)); + > + ((sum64 >> 32) as u32) ^ (sum64 as u32) > + } > + > + /// Notifies the GSP that we have updated the command queue > pointers. > + fn notify_gsp(bar: &Bar0) { > + regs::NV_PGSP_QUEUE_HEAD::default() > + .set_address(0) > + .write(bar); > + } > + > + /// Sends `command` to the GSP and waits for the reply. > + /// > + /// The mutex is held for the entire send+receive cycle to > ensure that no other command can > + /// be interleaved. Messages with non-matching function codes > are silently consumed until the > + /// expected reply arrives. > + /// > + /// # Errors > + /// > + /// - `ETIMEDOUT` if space does not become available to send the > command, or if the reply is > + /// not received within the timeout. > + /// - `EIO` if the variable payload requested by the command has > not been entirely > + /// written to by its [`CommandToGsp::init_variable_payload`] > method. > + /// > + /// Error codes returned by the command and reply initializers > are propagated as-is. > + pub(crate) fn send_sync_command<M>(&self, bar: &Bar0, command: > M) -> Result<M::Reply> > + where > + M: CommandToGsp, > + M::Reply: MessageFromGsp, > + Error: From<M::InitError>, > + Error: From<<M::Reply as MessageFromGsp>::InitError>, > + { > + let mut inner = self.inner.lock(); > + inner.send_command(&self.dev, bar, command)?; > + > + loop { > + match inner.receive_msg::<M::Reply>(&self.dev, > Delta::from_secs(10)) { > + Ok(reply) => break Ok(reply), > + Err(ERANGE) => continue, > + Err(e) => break Err(e), > + } > + } > + } > + > + /// Sends `command` to the GSP without waiting for a reply. > + /// > + /// # Errors > + /// > + /// - `ETIMEDOUT` if space does not become available within the > timeout. > + /// - `EIO` if the variable payload requested by the command has > not been entirely > + /// written to by its [`CommandToGsp::init_variable_payload`] > method. > + /// > + /// Error codes returned by the command initializers are > propagated as-is. > + pub(crate) fn send_async_command<M>(&self, bar: &Bar0, command: > M) -> Result > + where > + M: CommandToGsp<Reply = NoReply>, > + Error: From<M::InitError>, > + { > + self.inner.lock().send_command(&self.dev, bar, command) > + } > + > + /// Receive a message from the GSP. > + /// > + /// See [`CmdqInner::receive_msg`] for details. > + pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: > Delta) -> Result<M> > + where > + // This allows all error types, including `Infallible`, to > be used for `M::InitError`. > + Error: From<M::InitError>, > + { > + self.inner.lock().receive_msg(&self.dev, timeout) > + } > > /// Returns the DMA handle of the command queue's shared memory > region. pub(crate) fn dma_handle(&self) -> DmaAddress { > - self.gsp_mem.0.dma_handle() > + self.inner.lock().gsp_mem.0.dma_handle() > } > } > diff --git a/drivers/gpu/nova-core/gsp/commands.rs > b/drivers/gpu/nova-core/gsp/commands.rs index > b42e32dcc55c..7ceff2e6bd63 100644 --- > a/drivers/gpu/nova-core/gsp/commands.rs +++ > b/drivers/gpu/nova-core/gsp/commands.rs @@ -170,7 +170,7 @@ fn read( > } > > /// Waits for GSP initialization to complete. > -pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result { > +pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq) -> Result { > loop { > match cmdq.receive_msg::<GspInitDone>(Delta::from_secs(10)) { > Ok(_) => break Ok(()), > @@ -239,7 +239,7 @@ pub(crate) fn gpu_name(&self) -> > core::result::Result<&str, GpuNameError> { } > > /// Send the [`GetGspInfo`] command and awaits for its reply. > -pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> > Result<GetGspStaticInfoReply> { +pub(crate) fn get_gsp_info(cmdq: > &Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> { > cmdq.send_sync_command(bar, GetGspStaticInfo) } > > diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs > b/drivers/gpu/nova-core/gsp/sequencer.rs index > 2fa2a0792cec..bc94b8567c6a 100644 --- > a/drivers/gpu/nova-core/gsp/sequencer.rs +++ > b/drivers/gpu/nova-core/gsp/sequencer.rs @@ -366,7 +366,7 @@ > pub(crate) struct GspSequencerParams<'a> { } > > impl<'a> GspSequencer<'a> { > - pub(crate) fn run(cmdq: &mut Cmdq, params: > GspSequencerParams<'a>) -> Result { > + pub(crate) fn run(cmdq: &Cmdq, params: GspSequencerParams<'a>) > -> Result { let seq_info = loop { > match > cmdq.receive_msg::<GspSequence>(Delta::from_secs(10)) { Ok(seq_info) > => break seq_info, >
