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_command` to make sure that it doesn't get the reply of some other command that could have been sent just beforehand. Reviewed-by: Zhi Wang <[email protected]> Tested-by: Zhi Wang <[email protected]> Signed-off-by: Eliot Courtney <[email protected]> --- drivers/gpu/nova-core/gsp/boot.rs | 8 +- drivers/gpu/nova-core/gsp/cmdq.rs | 170 +++++++++++++++++++-------------- drivers/gpu/nova-core/gsp/commands.rs | 4 +- drivers/gpu/nova-core/gsp/sequencer.rs | 2 +- 4 files changed, 107 insertions(+), 77 deletions(-) diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs index ffc478b33640..5e73bd769dcc 100644 --- a/drivers/gpu/nova-core/gsp/boot.rs +++ b/drivers/gpu/nova-core/gsp/boot.rs @@ -137,7 +137,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, @@ -223,13 +223,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, "GPU name: {}\n", name), Err(e) => dev_warn!(pdev, "GPU name unavailable: {:?}\n", e), diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs index 86ff9a3d1732..d36a62ba1c60 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs +++ b/drivers/gpu/nova-core/gsp/cmdq.rs @@ -12,8 +12,12 @@ }, dma_write, io::poll::read_poll_timeout, + new_mutex, prelude::*, - sync::aref::ARef, + sync::{ + aref::ARef, + Mutex, // + }, time::Delta, transmute::{ AsBytes, @@ -448,12 +452,9 @@ struct GspMessage<'a> { /// area. #[pin_data] pub(crate) struct Cmdq { - /// Device this command queue belongs to. - dev: ARef<device::Device>, - /// Current command sequence number. - seq: u32, - /// Memory area shared with the GSP for communicating commands and messages. - gsp_mem: DmaGspMem, + /// Inner mutex-protected state. + #[pin] + inner: Mutex<CmdqInner>, } impl Cmdq { @@ -473,18 +474,17 @@ impl Cmdq { /// Number of page table entries for the GSP shared region. pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT; - /// Timeout for waiting for space on the command queue. - const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1); - /// Default timeout for receiving a message from the GSP. pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(5); /// 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, + inner <- new_mutex!(CmdqInner { + dev: dev.into(), + gsp_mem: DmaGspMem::new(dev)?, + seq: 0, + }), }) } @@ -508,6 +508,89 @@ fn notify_gsp(bar: &Bar0) { .write(bar); } + /// Sends `command` to the GSP and waits for the reply. + /// + /// Messages with non-matching function codes are silently consumed until the expected reply + /// arrives. + /// + /// The queue is locked for the entire send+receive cycle to ensure that no other command can + /// be interleaved. + /// + /// # 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_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(bar, command)?; + + loop { + match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) { + 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_command_no_wait<M>(&self, bar: &Bar0, command: M) -> Result + where + M: CommandToGsp<Reply = NoReply>, + Error: From<M::InitError>, + { + self.inner.lock().send_command(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(timeout) + } + + /// Returns the DMA handle of the command queue's shared memory region. + pub(crate) fn dma_handle(&self) -> DmaAddress { + self.inner.lock().gsp_mem.0.dma_handle() + } +} + +/// Inner mutex protected state of [`Cmdq`]. +struct CmdqInner { + /// Device this command queue belongs to. + dev: ARef<device::Device>, + /// Current command sequence number. + seq: u32, + /// Memory area shared with the GSP for communicating commands and messages. + gsp_mem: DmaGspMem, +} + +impl CmdqInner { + /// Timeout for waiting for space on the command queue. + const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1); + /// Sends `command` to the GSP, without splitting it. /// /// # Errors @@ -588,7 +671,7 @@ 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_internal<M>(&mut self, bar: &Bar0, command: M) -> Result + fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result where M: CommandToGsp, Error: From<M::InitError>, @@ -608,54 +691,6 @@ fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result } } - /// Sends `command` to the GSP and waits for the reply. - /// - /// 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_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_internal(bar, command)?; - - loop { - match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) { - 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_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result - where - M: CommandToGsp<Reply = NoReply>, - Error: From<M::InitError>, - { - self.send_command_internal(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 @@ -691,7 +726,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, + &self.dev, "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n", header.sequence(), header.function(), @@ -726,7 +761,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> { ])) != 0 { dev_err!( - self.dev, + &self.dev, "GSP RPC: receive: Call {} - bad checksum\n", header.sequence() ); @@ -755,7 +790,7 @@ 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, timeout: Delta) -> Result<M> where // This allows all error types, including `Infallible`, to be used for `M::InitError`. Error: From<M::InitError>, @@ -791,9 +826,4 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul result } - - /// 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() - } } diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs index 77054c92fcc2..c89c7b57a751 100644 --- a/drivers/gpu/nova-core/gsp/commands.rs +++ b/drivers/gpu/nova-core/gsp/commands.rs @@ -165,7 +165,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>(Cmdq::RECEIVE_TIMEOUT) { Ok(_) => break Ok(()), @@ -234,6 +234,6 @@ 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_command(bar, GetGspStaticInfo) } diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs index ce2b3bb05d22..474e4c8021db 100644 --- a/drivers/gpu/nova-core/gsp/sequencer.rs +++ b/drivers/gpu/nova-core/gsp/sequencer.rs @@ -356,7 +356,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>(Cmdq::RECEIVE_TIMEOUT) { Ok(seq_info) => break seq_info, -- 2.53.0
