Re: [PATCH 26/27] gpu: nova-core: gsp: remove the retired system-info and static-info RPCs
Zhi Wang <[email protected]>
| Newsgroups | dev.linux.lists.nova-gpu,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260820215956.35ec589b@inno-dell> |
On Tue, 18 Aug 2026 20:52:19 -0700 John Hubbard <[email protected]> wrote: We can remove SetRegistry in gsp/commands.rs and gsp/fw/commands.rs as well. > The r000 boot path folds the system information and the static GPU > configuration into the GSP_INIT request, whose reply carries that > configuration back to the driver. > > The set-system-info and get-static-info commands stayed behind after > the switch, unreachable and marked dead, along with the two payload > structures that encoded them. > > Remove both commands and their payloads, and keep the decoded static > configuration that the GSP_INIT reply fills in. The two > message-function entries stay, because that table catalogs the wire > protocol rather than what the driver implements, and already names > many functions nova-core never sends. > > Assisted-by: Cursor:claude-opus-5 > Signed-off-by: John Hubbard <[email protected]> > --- > drivers/gpu/nova-core/gsp.rs | 8 -- > drivers/gpu/nova-core/gsp/commands.rs | 65 +------------ > drivers/gpu/nova-core/gsp/fw/commands.rs | 115 > +---------------------- 3 files changed, 3 insertions(+), 185 > deletions(-) > > diff --git a/drivers/gpu/nova-core/gsp.rs > b/drivers/gpu/nova-core/gsp.rs index 8e912eaf8e2e..b128153fda86 100644 > --- a/drivers/gpu/nova-core/gsp.rs > +++ b/drivers/gpu/nova-core/gsp.rs > @@ -407,14 +407,6 @@ pub(crate) fn new( > }) > } > > - /// Query the GSP for the static GPU information. > - /// > - /// The r000 boot path gets the same information from the > `GSP_INIT` reply instead. > - #[expect(dead_code)] > - pub(crate) fn get_static_info(&self, bar: Bar0<'_>) -> > Result<commands::GetGspStaticInfoReply> { > - self.cmdq.send_command(bar, commands::GetGspStaticInfo) > - } > - > /// Returns a shared handle to the GSP command queue. > pub(crate) fn cmdq(&self) -> Arc<Cmdq> { > self.cmdq.clone() > diff --git a/drivers/gpu/nova-core/gsp/commands.rs > b/drivers/gpu/nova-core/gsp/commands.rs index > 26ea07dc4a28..9079393da0b3 100644 --- > a/drivers/gpu/nova-core/gsp/commands.rs +++ > b/drivers/gpu/nova-core/gsp/commands.rs @@ -49,34 +49,6 @@ > vgpu::VgpuState, // > }; > > -/// The `GspSetSystemInfo` command. > -/// > -/// The r000 boot path folds this into the `GSP_INIT` payload > instead. -pub(crate) struct SetSystemInfo<'a> { > - pdev: &'a pci::Device<device::Bound>, > - chipset: Chipset, > -} > - > -#[expect(dead_code)] > -impl<'a> SetSystemInfo<'a> { > - /// Creates a new `GspSetSystemInfo` command using the > parameters of `pdev`. > - pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, chipset: > Chipset) -> Self { > - Self { pdev, chipset } > - } > -} > - > -impl<'a> CommandToGsp for SetSystemInfo<'a> { > - const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo; > - const IS_ASYNC: bool = true; > - type Command = fw::commands::GspSetSystemInfo; > - type Reply = NoReply; > - type InitError = Error; > - > - fn init(&self) -> impl Init<Self::Command, Self::InitError> { > - Self::Command::init(self.pdev, self.chipset) > - } > -} > - > struct RegistryEntry { > key: &'static str, > value: u32, > @@ -186,48 +158,13 @@ fn init_variable_payload( > } > } > > -/// The `GetGspStaticInfo` command. > -pub(crate) struct GetGspStaticInfo; > - > -impl CommandToGsp for GetGspStaticInfo { > - const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo; > - type Command = fw::commands::GspStaticConfigInfo; > - type Reply = GetGspStaticInfoReply; > - type InitError = Infallible; > - > - fn init(&self) -> impl Init<Self::Command, Self::InitError> { > - Self::Command::init_zeroed() > - } > -} > - > -/// The reply from the GSP to the [`GetGspStaticInfo`] command. > +/// The static GPU configuration, as decoded from the `GSP_INIT` > reply. pub(crate) struct GetGspStaticInfoReply { > gpu_name: [u8; 64], > /// Usable FB (VRAM) regions for driver memory allocation. > pub(crate) usable_fb_regions: KVec<Range<u64>>, > } > > -impl MessageFromGsp for GetGspStaticInfoReply { > - const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo; > - type Message = fw::commands::GspStaticConfigInfo; > - type InitError = Error; > - > - fn read( > - msg: &Self::Message, > - _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>, > - ) -> Result<Self, Self::InitError> { > - let mut usable_fb_regions = KVec::new(); > - for region in msg.usable_fb_regions() { > - usable_fb_regions.push(region, GFP_KERNEL)?; > - } > - > - Ok(GetGspStaticInfoReply { > - gpu_name: msg.gpu_name_str(), > - usable_fb_regions, > - }) > - } > -} > - > /// Error type for [`GetGspStaticInfoReply::gpu_name`]. > #[derive(Debug)] > pub(crate) enum GpuNameError { > diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs > b/drivers/gpu/nova-core/gsp/fw/commands.rs index > 00e40a435053..e49234f35d0f 100644 --- > a/drivers/gpu/nova-core/gsp/fw/commands.rs +++ > b/drivers/gpu/nova-core/gsp/fw/commands.rs @@ -14,11 +14,7 @@ > }, // > }; > > -use crate::{ > - gpu::Chipset, > - gsp::GSP_PAGE_SIZE, > - num::IntoSafeCast, // > -}; > +use crate::gpu::Chipset; > > use crate::gsp::nvkv::{ > nvkv_decode, > @@ -37,56 +33,6 @@ > > use super::bindings; > > -/// Payload of the `GspSetSystemInfo` command. > -#[repr(transparent)] > -pub(crate) struct GspSetSystemInfo { > - inner: bindings::GspSystemInfo, > -} > -static_assert!(size_of::<GspSetSystemInfo>() < GSP_PAGE_SIZE); > - > -impl GspSetSystemInfo { > - /// Returns an in-place initializer for the `GspSetSystemInfo` > command. > - pub(crate) fn init<'a>( > - dev: &'a pci::Device<device::Bound>, > - chipset: Chipset, > - ) -> impl Init<Self, Error> + 'a { > - type InnerGspSystemInfo = bindings::GspSystemInfo; > - let pci_config_mirror_range = > chipset.pci_config_mirror_range(); > - let init_inner = try_init!(InnerGspSystemInfo { > - gpuPhysAddr: dev.resource_start(0)?, > - gpuPhysFbAddr: dev.resource_start(1)?, > - gpuPhysInstAddr: dev.resource_start(3)?, > - nvDomainBusDeviceFunc: u64::from(dev.dev_id()), > - > - // Using TASK_SIZE in r535_gsp_rpc_set_system_info() > seems wrong because > - // TASK_SIZE is per-task. That's probably a design issue > in GSP-RM though. > - maxUserVa: (1 << 47) - 4096, > - pciConfigMirrorBase: pci_config_mirror_range.start, > - pciConfigMirrorSize: pci_config_mirror_range.end - > pci_config_mirror_range.start, - > - PCIDeviceID: (u32::from(dev.device_id()) << 16) | > u32::from(dev.vendor_id().as_raw()), > - PCISubDeviceID: (u32::from(dev.subsystem_device_id()) << > 16) > - | u32::from(dev.subsystem_vendor_id()), > - PCIRevisionID: u32::from(dev.revision_id()), > - bIsPrimary: 0, > - bPreserveVideoMemoryAllocations: 0, > - ..Zeroable::init_zeroed() > - }); > - > - try_init!(GspSetSystemInfo { > - inner <- init_inner, > - }) > - } > -} > - > -// SAFETY: These structs don't meet the no-padding requirements of > AsBytes but -// that is not a problem because they are not > used outside the kernel. -unsafe impl AsBytes for GspSetSystemInfo {} > - > -// SAFETY: These structs don't meet the no-padding requirements of > FromBytes but -// that is not a problem because they are not > used outside the kernel. -unsafe impl FromBytes for GspSetSystemInfo > {} - > #[repr(transparent)] > pub(crate) struct > PackedRegistryEntry(bindings::PACKED_REGISTRY_ENTRY); > @@ -136,60 +82,6 @@ unsafe impl AsBytes for PackedRegistryTable {} > // are valid. > unsafe impl FromBytes for PackedRegistryTable {} > > -/// Payload of the `GetGspStaticInfo` command and message. > -#[repr(transparent)] > -#[derive(Zeroable)] > -pub(crate) struct > GspStaticConfigInfo(bindings::GspStaticConfigInfo_t); - > -impl GspStaticConfigInfo { > - /// Returns a bytes array containing the (hopefully) > zero-terminated name of this GPU. > - pub(crate) fn gpu_name_str(&self) -> [u8; 64] { > - self.0.gpuNameString > - } > - > - /// Returns an iterator over valid FB regions from GSP firmware > data. > - fn fb_regions( > - &self, > - ) -> impl Iterator<Item = > &bindings::NV2080_CTRL_CMD_FB_GET_FB_REGION_FB_REGION_INFO> { > - let fb_info = &self.0.fbRegionInfoParams; > - fb_info > - .fbRegion > - .iter() > - .take(fb_info.numFBRegions.into_safe_cast()) > - .filter(|reg| reg.limit >= reg.base) > - } > - > - /// Iterates over usable FB regions from GSP firmware data. > - /// > - /// Each yielded region is a [`Range<u64>`] suitable for driver > memory allocation. > - /// Usable regions are those that satisfy all the following > properties: > - /// - Are not reserved for firmware internal use. > - /// - Are not protected (hardware-enforced access restrictions). > - /// - Support compression (can use GPU memory compression for > bandwidth). > - /// - Support ISO (isochronous memory for display requiring > guaranteed bandwidth). > - pub(crate) fn usable_fb_regions(&self) -> impl Iterator<Item = > Range<u64>> + '_ { > - self.fb_regions().filter_map(|reg| { > - // Filter: not reserved, not protected, supports > compression and ISO. > - if reg.reserved == 0 > - && reg.bProtected == 0 > - && reg.supportCompressed != 0 > - && reg.supportISO != 0 > - { > - reg.limit.checked_add(1).map(|end| reg.base..end) > - } else { > - None > - } > - }) > - } > -} > - > -// SAFETY: Padding is explicit and will not contain uninitialized > data. -unsafe impl AsBytes for GspStaticConfigInfo {} > - > -// SAFETY: This struct only contains integer types for which all bit > patterns -// are valid. > -unsafe impl FromBytes for GspStaticConfigInfo {} > - > /// Power level requested to the [`UnloadingGuestDriver`] command. > #[derive(Clone, Copy, Debug, PartialEq, Eq)] > #[repr(u32)] > @@ -369,8 +261,6 @@ impl GspInitRequest { > const NV_DOMAIN_BUS_DEVICE_FUNC_KEY: KeyId = 0x1020; > > /// Describes `dev` to GSP-RM and asks it to apply `regkeys`. > - /// > - /// The same identifiers reach GSP-RM through > [`GspSetSystemInfo::init`] on the RPC path. pub(crate) fn new( > dev: &pci::Device<device::Bound>, > chipset: Chipset, > @@ -442,8 +332,7 @@ pub(crate) fn gpu_name(&self) -> &[u8] { > /// Iterates over the FB regions the driver may allocate from. > /// > /// A region qualifies when it is untagged, unprotected, and > supports both compression and > - /// isochronous access, which is the same set the RPC path > selects from > - /// [`GspStaticConfigInfo::usable_fb_regions`]. > + /// isochronous access. > pub(crate) fn usable_fb_regions(&self) -> impl Iterator<Item = > Range<u64>> + '_ { self.fb_regions.iter().filter_map(|region| { > if region.limit >= region.base