Alexandre Courbot
2025-Aug-08 02:46 UTC
[PATCH 0/2] gpu: nova-core: vbios: simplify device use
This small cleanup series simplifies the use of `Device` in vbios
methods.
The device is used for logging purposes only; thus we don't need a
pci::Device, neither do we need it to be bound. This latter property
allows us to store an `ARef` to it into structures that require logging
instead of having all their methods take an extra `dev` argument.
Removing this argument streamlines the code a bit.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
Alexandre Courbot (2):
gpu: nova-core: vbios: replace pci::Device with device::Device
gpu: nova-core: vbios: store reference to Device where relevant
drivers/gpu/nova-core/firmware/fwsec.rs | 8 +-
drivers/gpu/nova-core/gpu.rs | 2 +-
drivers/gpu/nova-core/vbios.rs | 168 +++++++++++++++-----------------
3 files changed, 85 insertions(+), 93 deletions(-)
---
base-commit: 14ae91a81ec8fa0bc23170d4aa16dd2a20d54105
change-id: 20250808-vbios_device-b0a912aff149
Best regards,
--
Alexandre Courbot <acourbot at nvidia.com>
Alexandre Courbot
2025-Aug-08 02:46 UTC
[PATCH 1/2] gpu: nova-core: vbios: replace pci::Device with device::Device
The passed pci::Device is exclusively used for logging purposes, so it
can be replaced by a regular device::Device, which allows us to remove
the `as_ref()` indirections at each logging site.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 2 +-
drivers/gpu/nova-core/vbios.rs | 135 +++++++++++++++++------------------------
2 files changed, 57 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index
72d40b0124f0c1a2a381484172c289af523511df..33082ac45873ee4cf91d7d8af499efa984af4ba9
100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -298,7 +298,7 @@ pub(crate) fn new(
let fb_layout = FbLayout::new(spec.chipset, bar)?;
dev_dbg!(pdev.as_ref(), "{:#x?}\n", fb_layout);
- let bios = Vbios::new(pdev, bar)?;
+ let bios = Vbios::new(pdev.as_ref(), bar)?;
Self::run_fwsec_frts(pdev.as_ref(), &gsp_falcon, bar, &bios,
&fb_layout)?;
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index
5b5d9f38cbb3a6b1c374c1e0eee2509eb8d5660c..b5564b4d3e4758e77178aa403635e4780f0378cc
100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -8,7 +8,6 @@
use core::convert::TryFrom;
use kernel::device;
use kernel::error::Result;
-use kernel::pci;
use kernel::prelude::*;
/// The offset of the VBIOS ROM in the BAR0 space.
@@ -31,7 +30,7 @@
/// Vbios Reader for constructing the VBIOS data.
struct VbiosIterator<'a> {
- pdev: &'a pci::Device,
+ dev: &'a device::Device,
bar0: &'a Bar0,
/// VBIOS data vector: As BIOS images are scanned, they are added to this
vector for reference
/// or copying into other data structures. It is the entire scanned
contents of the VBIOS which
@@ -46,9 +45,9 @@ struct VbiosIterator<'a> {
}
impl<'a> VbiosIterator<'a> {
- fn new(pdev: &'a pci::Device, bar0: &'a Bar0) ->
Result<Self> {
+ fn new(dev: &'a device::Device, bar0: &'a Bar0) ->
Result<Self> {
Ok(Self {
- pdev,
+ dev,
bar0,
data: KVec::new(),
current_offset: 0,
@@ -64,7 +63,7 @@ fn read_more(&mut self, len: usize) -> Result {
// Ensure length is a multiple of 4 for 32-bit reads
if len % core::mem::size_of::<u32>() != 0 {
dev_err!(
- self.pdev.as_ref(),
+ self.dev,
"VBIOS read length {} is not a multiple of 4\n",
len
);
@@ -89,7 +88,7 @@ fn read_more(&mut self, len: usize) -> Result {
/// Read bytes at a specific offset, filling any gap.
fn read_more_at_offset(&mut self, offset: usize, len: usize) ->
Result {
if offset > BIOS_MAX_SCAN_LEN {
- dev_err!(self.pdev.as_ref(), "Error: exceeded BIOS scan
limit.\n");
+ dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
return Err(EINVAL);
}
@@ -115,7 +114,7 @@ fn read_bios_image_at_offset(
if offset + len > data_len {
self.read_more_at_offset(offset, len).inspect_err(|e| {
dev_err!(
- self.pdev.as_ref(),
+ self.dev,
"Failed to read more at offset {:#x}: {:?}\n",
offset,
e
@@ -123,9 +122,9 @@ fn read_bios_image_at_offset(
})?;
}
- BiosImage::new(self.pdev, &self.data[offset..offset +
len]).inspect_err(|err| {
+ BiosImage::new(self.dev, &self.data[offset..offset +
len]).inspect_err(|err| {
dev_err!(
- self.pdev.as_ref(),
+ self.dev,
"Failed to {} at offset {:#x}: {:?}\n",
context,
offset,
@@ -146,10 +145,7 @@ fn next(&mut self) -> Option<Self::Item> {
}
if self.current_offset > BIOS_MAX_SCAN_LEN {
- dev_err!(
- self.pdev.as_ref(),
- "Error: exceeded BIOS scan limit, stopping scan\n"
- );
+ dev_err!(self.dev, "Error: exceeded BIOS scan limit, stopping
scan\n");
return None;
}
@@ -192,18 +188,18 @@ impl Vbios {
/// Probe for VBIOS extraction.
///
/// Once the VBIOS object is built, `bar0` is not read for [`Vbios`]
purposes anymore.
- pub(crate) fn new(pdev: &pci::Device, bar0: &Bar0) ->
Result<Vbios> {
+ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) ->
Result<Vbios> {
// Images to extract from iteration
let mut pci_at_image: Option<PciAtBiosImage> = None;
let mut first_fwsec_image: Option<FwSecBiosBuilder> = None;
let mut second_fwsec_image: Option<FwSecBiosBuilder> = None;
// Parse all VBIOS images in the ROM
- for image_result in VbiosIterator::new(pdev, bar0)? {
+ for image_result in VbiosIterator::new(dev, bar0)? {
let full_image = image_result?;
dev_dbg!(
- pdev.as_ref(),
+ dev,
"Found BIOS image: size: {:#x}, type: {}, last:
{}\n",
full_image.image_size_bytes(),
full_image.image_type_str(),
@@ -234,14 +230,14 @@ pub(crate) fn new(pdev: &pci::Device, bar0: &Bar0)
-> Result<Vbios> {
(second_fwsec_image, first_fwsec_image, pci_at_image)
{
second
- .setup_falcon_data(pdev, &pci_at, &first)
- .inspect_err(|e| dev_err!(pdev.as_ref(), "Falcon data
setup failed: {:?}\n", e))?;
+ .setup_falcon_data(dev, &pci_at, &first)
+ .inspect_err(|e| dev_err!(dev, "Falcon data setup failed:
{:?}\n", e))?;
Ok(Vbios {
- fwsec_image: second.build(pdev)?,
+ fwsec_image: second.build(dev)?,
})
} else {
dev_err!(
- pdev.as_ref(),
+ dev,
"Missing required images for falcon data setup,
skipping\n"
);
Err(EINVAL)
@@ -284,9 +280,9 @@ struct PcirStruct {
}
impl PcirStruct {
- fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
+ fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
if data.len() < core::mem::size_of::<PcirStruct>() {
- dev_err!(pdev.as_ref(), "Not enough data for
PcirStruct\n");
+ dev_err!(dev, "Not enough data for PcirStruct\n");
return Err(EINVAL);
}
@@ -295,11 +291,7 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
// Signature should be "PCIR" (0x52494350) or
"NPDS" (0x5344504e).
if &signature != b"PCIR" && &signature !=
b"NPDS" {
- dev_err!(
- pdev.as_ref(),
- "Invalid signature for PcirStruct: {:?}\n",
- signature
- );
+ dev_err!(dev, "Invalid signature for PcirStruct: {:?}\n",
signature);
return Err(EINVAL);
}
@@ -308,7 +300,7 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
let image_len = u16::from_le_bytes([data[16], data[17]]);
if image_len == 0 {
- dev_err!(pdev.as_ref(), "Invalid image length: 0\n");
+ dev_err!(dev, "Invalid image length: 0\n");
return Err(EINVAL);
}
@@ -467,7 +459,7 @@ struct PciRomHeader {
}
impl PciRomHeader {
- fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
+ fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
if data.len() < 26 {
// Need at least 26 bytes to read pciDataStrucPtr and sizeOfBlock.
return Err(EINVAL);
@@ -479,7 +471,7 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
match signature {
0xAA55 | 0xBB77 | 0x4E56 => {}
_ => {
- dev_err!(pdev.as_ref(), "ROM signature unknown
{:#x}\n", signature);
+ dev_err!(dev, "ROM signature unknown {:#x}\n",
signature);
return Err(EINVAL);
}
}
@@ -538,9 +530,9 @@ struct NpdeStruct {
}
impl NpdeStruct {
- fn new(pdev: &pci::Device, data: &[u8]) -> Option<Self> {
+ fn new(dev: &device::Device, data: &[u8]) -> Option<Self>
{
if data.len() < core::mem::size_of::<Self>() {
- dev_dbg!(pdev.as_ref(), "Not enough data for
NpdeStruct\n");
+ dev_dbg!(dev, "Not enough data for NpdeStruct\n");
return None;
}
@@ -549,17 +541,13 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Option<Self> {
// Signature should be "NPDE" (0x4544504E).
if &signature != b"NPDE" {
- dev_dbg!(
- pdev.as_ref(),
- "Invalid signature for NpdeStruct: {:?}\n",
- signature
- );
+ dev_dbg!(dev, "Invalid signature for NpdeStruct: {:?}\n",
signature);
return None;
}
let subimage_len = u16::from_le_bytes([data[8], data[9]]);
if subimage_len == 0 {
- dev_dbg!(pdev.as_ref(), "Invalid subimage length: 0\n");
+ dev_dbg!(dev, "Invalid subimage length: 0\n");
return None;
}
@@ -584,7 +572,7 @@ fn image_size_bytes(&self) -> usize {
/// Try to find NPDE in the data, the NPDE is right after the PCIR.
fn find_in_data(
- pdev: &pci::Device,
+ dev: &device::Device,
data: &[u8],
rom_header: &PciRomHeader,
pcir: &PcirStruct,
@@ -596,12 +584,12 @@ fn find_in_data(
// Check if we have enough data
if npde_start + core::mem::size_of::<Self>() > data.len() {
- dev_dbg!(pdev.as_ref(), "Not enough data for NPDE\n");
+ dev_dbg!(dev, "Not enough data for NPDE\n");
return None;
}
// Try to create NPDE from the data
- NpdeStruct::new(pdev, &data[npde_start..])
+ NpdeStruct::new(dev, &data[npde_start..])
}
}
@@ -669,10 +657,10 @@ fn image_size_bytes(&self) -> usize {
/// Create a [`BiosImageBase`] from a byte slice and convert it to a
[`BiosImage`] which
/// triggers the constructor of the specific BiosImage enum variant.
- fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
- let base = BiosImageBase::new(pdev, data)?;
+ fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
+ let base = BiosImageBase::new(dev, data)?;
let image = base.into_image().inspect_err(|e| {
- dev_err!(pdev.as_ref(), "Failed to create BiosImage:
{:?}\n", e);
+ dev_err!(dev, "Failed to create BiosImage: {:?}\n", e);
})?;
Ok(image)
@@ -773,16 +761,16 @@ fn into_image(self) -> Result<BiosImage> {
}
/// Creates a new BiosImageBase from raw byte data.
- fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
+ fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
// Ensure we have enough data for the ROM header.
if data.len() < 26 {
- dev_err!(pdev.as_ref(), "Not enough data for ROM
header\n");
+ dev_err!(dev, "Not enough data for ROM header\n");
return Err(EINVAL);
}
// Parse the ROM header.
- let rom_header = PciRomHeader::new(pdev, &data[0..26])
- .inspect_err(|e| dev_err!(pdev.as_ref(), "Failed to create
PciRomHeader: {:?}\n", e))?;
+ let rom_header = PciRomHeader::new(dev, &data[0..26])
+ .inspect_err(|e| dev_err!(dev, "Failed to create PciRomHeader:
{:?}\n", e))?;
// Get the PCI Data Structure using the pointer from the ROM header.
let pcir_offset = rom_header.pci_data_struct_offset as usize;
@@ -791,22 +779,22 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
.ok_or(EINVAL)
.inspect_err(|_| {
dev_err!(
- pdev.as_ref(),
+ dev,
"PCIR offset {:#x} out of bounds (data length:
{})\n",
pcir_offset,
data.len()
);
dev_err!(
- pdev.as_ref(),
+ dev,
"Consider reading more data for construction of
BiosImage\n"
);
})?;
- let pcir = PcirStruct::new(pdev, pcir_data)
- .inspect_err(|e| dev_err!(pdev.as_ref(), "Failed to create
PcirStruct: {:?}\n", e))?;
+ let pcir = PcirStruct::new(dev, pcir_data)
+ .inspect_err(|e| dev_err!(dev, "Failed to create PcirStruct:
{:?}\n", e))?;
// Look for NPDE structure if this is not an NBSI image (type != 0x70).
- let npde = NpdeStruct::find_in_data(pdev, data, &rom_header,
&pcir);
+ let npde = NpdeStruct::find_in_data(dev, data, &rom_header,
&pcir);
// Create a copy of the data.
let mut data_copy = KVec::new();
@@ -848,7 +836,7 @@ fn get_bit_token(&self, token_id: u8) ->
Result<BitToken> {
///
/// This is just a 4 byte structure that contains a pointer to the Falcon
data in the FWSEC
/// image.
- fn falcon_data_ptr(&self, pdev: &pci::Device) ->
Result<u32> {
+ fn falcon_data_ptr(&self, dev: &device::Device) ->
Result<u32> {
let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?;
// Make sure we don't go out of bounds
@@ -859,14 +847,14 @@ fn falcon_data_ptr(&self, pdev: &pci::Device)
-> Result<u32> {
// read the 4 bytes at the offset specified in the token
let offset = token.data_offset as usize;
let bytes: [u8; 4] = self.base.data[offset..offset +
4].try_into().map_err(|_| {
- dev_err!(pdev.as_ref(), "Failed to convert data slice to
array");
+ dev_err!(dev, "Failed to convert data slice to array");
EINVAL
})?;
let data_ptr = u32::from_le_bytes(bytes);
if (data_ptr as usize) < self.base.data.len() {
- dev_err!(pdev.as_ref(), "Falcon data pointer out of
bounds\n");
+ dev_err!(dev, "Falcon data pointer out of bounds\n");
return Err(EINVAL);
}
@@ -928,7 +916,7 @@ struct PmuLookupTable {
}
impl PmuLookupTable {
- fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
+ fn new(dev: &device::Device, data: &[u8]) -> Result<Self>
{
if data.len() < 4 {
return Err(EINVAL);
}
@@ -940,10 +928,7 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
let required_bytes = header_len + (entry_count * entry_len);
if data.len() < required_bytes {
- dev_err!(
- pdev.as_ref(),
- "PmuLookupTable data length less than required\n"
- );
+ dev_err!(dev, "PmuLookupTable data length less than
required\n");
return Err(EINVAL);
}
@@ -956,11 +941,7 @@ fn new(pdev: &pci::Device, data: &[u8]) ->
Result<Self> {
// Debug logging of entries (dumps the table data to dmesg)
for i in (header_len..required_bytes).step_by(entry_len) {
- dev_dbg!(
- pdev.as_ref(),
- "PMU entry: {:02x?}\n",
- &data[i..][..entry_len]
- );
+ dev_dbg!(dev, "PMU entry: {:02x?}\n",
&data[i..][..entry_len]);
}
Ok(PmuLookupTable {
@@ -997,11 +978,11 @@ fn find_entry_by_type(&self, entry_type: u8) ->
Result<PmuLookupTableEntry> {
impl FwSecBiosBuilder {
fn setup_falcon_data(
&mut self,
- pdev: &pci::Device,
+ dev: &device::Device,
pci_at_image: &PciAtBiosImage,
first_fwsec: &FwSecBiosBuilder,
) -> Result {
- let mut offset = pci_at_image.falcon_data_ptr(pdev)? as usize;
+ let mut offset = pci_at_image.falcon_data_ptr(dev)? as usize;
let mut pmu_in_first_fwsec = false;
// The falcon data pointer assumes that the PciAt and FWSEC images
@@ -1025,9 +1006,9 @@ fn setup_falcon_data(
if pmu_in_first_fwsec {
self.pmu_lookup_table -
Some(PmuLookupTable::new(pdev, &first_fwsec.base.data[offset..])?);
+ Some(PmuLookupTable::new(dev,
&first_fwsec.base.data[offset..])?);
} else {
- self.pmu_lookup_table = Some(PmuLookupTable::new(pdev,
&self.base.data[offset..])?);
+ self.pmu_lookup_table = Some(PmuLookupTable::new(dev,
&self.base.data[offset..])?);
}
match self
@@ -1040,18 +1021,14 @@ fn setup_falcon_data(
let mut ucode_offset = entry.data as usize;
ucode_offset -= pci_at_image.base.data.len();
if ucode_offset < first_fwsec.base.data.len() {
- dev_err!(pdev.as_ref(), "Falcon Ucode offset not in
second Fwsec.\n");
+ dev_err!(dev, "Falcon Ucode offset not in second
Fwsec.\n");
return Err(EINVAL);
}
ucode_offset -= first_fwsec.base.data.len();
self.falcon_ucode_offset = Some(ucode_offset);
}
Err(e) => {
- dev_err!(
- pdev.as_ref(),
- "PmuLookupTableEntry not found, error: {:?}\n",
- e
- );
+ dev_err!(dev, "PmuLookupTableEntry not found, error:
{:?}\n", e);
return Err(EINVAL);
}
}
@@ -1059,7 +1036,7 @@ fn setup_falcon_data(
}
/// Build the final FwSecBiosImage from this builder
- fn build(self, pdev: &pci::Device) -> Result<FwSecBiosImage> {
+ fn build(self, dev: &device::Device) -> Result<FwSecBiosImage>
{
let ret = FwSecBiosImage {
base: self.base,
falcon_ucode_offset: self.falcon_ucode_offset.ok_or(EINVAL)?,
@@ -1067,8 +1044,8 @@ fn build(self, pdev: &pci::Device) ->
Result<FwSecBiosImage> {
if cfg!(debug_assertions) {
// Print the desc header for debugging
- let desc = ret.header(pdev.as_ref())?;
- dev_dbg!(pdev.as_ref(), "PmuLookupTableEntry desc:
{:#?}\n", desc);
+ let desc = ret.header(dev)?;
+ dev_dbg!(dev, "PmuLookupTableEntry desc: {:#?}\n", desc);
}
Ok(ret)
--
2.50.1
Alexandre Courbot
2025-Aug-08 02:46 UTC
[PATCH 2/2] gpu: nova-core: vbios: store reference to Device where relevant
Now that the vbios code uses a non-bound `Device` instance, store an
`ARef` to it at construction time so we can use it for logging without
having to carry an extra argument on every method for that sole purpose.
Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>
---
drivers/gpu/nova-core/firmware/fwsec.rs | 8 ++--
drivers/gpu/nova-core/vbios.rs | 69 ++++++++++++++++++++-------------
2 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
b/drivers/gpu/nova-core/firmware/fwsec.rs
index
0dff3cfa90afee0cd4c3348023c8bfd7edccdb29..d9b9d1f92880cbcd36dac84b9e86a84e6465cf5d
100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -253,8 +253,8 @@ impl FalconFirmware for FwsecFirmware {
impl FirmwareDmaObject<FwsecFirmware, Unsigned> {
fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd:
FwsecCommand) -> Result<Self> {
- let desc = bios.fwsec_image().header(dev)?;
- let ucode = bios.fwsec_image().ucode(dev, desc)?;
+ let desc = bios.fwsec_image().header()?;
+ let ucode = bios.fwsec_image().ucode(desc)?;
let mut dma_object = DmaObject::from_data(dev, ucode)?;
let hdr_offset = (desc.imem_load_size + desc.interface_offset) as
usize;
@@ -343,7 +343,7 @@ pub(crate) fn new(
let ucode_dma = FirmwareDmaObject::<Self, _>::new_fwsec(dev,
bios, cmd)?;
// Patch signature if needed.
- let desc = bios.fwsec_image().header(dev)?;
+ let desc = bios.fwsec_image().header()?;
let ucode_signed = if desc.signature_count != 0 {
let sig_base_img = (desc.imem_load_size + desc.pkc_data_offset) as
usize;
let desc_sig_versions = u32::from(desc.signature_versions);
@@ -382,7 +382,7 @@ pub(crate) fn new(
dev_dbg!(dev, "patching signature with index {}\n",
signature_idx);
let signature = bios
.fwsec_image()
- .sigs(dev, desc)
+ .sigs(desc)
.and_then(|sigs| sigs.get(signature_idx).ok_or(EINVAL))?;
ucode_dma.patch_signature(signature, sig_base_img)?
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index
b5564b4d3e4758e77178aa403635e4780f0378cc..6fc06b1b83655a7dec00308880dbdfc32d7105ce
100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -9,6 +9,7 @@
use kernel::device;
use kernel::error::Result;
use kernel::prelude::*;
+use kernel::types::ARef;
/// The offset of the VBIOS ROM in the BAR0 space.
const ROM_OFFSET: usize = 0x300000;
@@ -230,10 +231,10 @@ pub(crate) fn new(dev: &device::Device, bar0:
&Bar0) -> Result<Vbios> {
(second_fwsec_image, first_fwsec_image, pci_at_image)
{
second
- .setup_falcon_data(dev, &pci_at, &first)
+ .setup_falcon_data(&pci_at, &first)
.inspect_err(|e| dev_err!(dev, "Falcon data setup failed:
{:?}\n", e))?;
Ok(Vbios {
- fwsec_image: second.build(dev)?,
+ fwsec_image: second.build()?,
})
} else {
dev_err!(
@@ -742,9 +743,10 @@ fn try_from(base: BiosImageBase) -> Result<Self> {
///
/// Each BiosImage type has a BiosImageBase type along with other
image-specific fields. Note that
/// Rust favors composition of types over inheritance.
-#[derive(Debug)]
#[expect(dead_code)]
struct BiosImageBase {
+ /// Used for logging.
+ dev: ARef<device::Device>,
/// PCI ROM Expansion Header
rom_header: PciRomHeader,
/// PCI Data Structure
@@ -801,6 +803,7 @@ fn new(dev: &device::Device, data: &[u8]) ->
Result<Self> {
data_copy.extend_from_slice(data, GFP_KERNEL)?;
Ok(BiosImageBase {
+ dev: dev.into(),
rom_header,
pcir,
npde,
@@ -836,7 +839,7 @@ fn get_bit_token(&self, token_id: u8) ->
Result<BitToken> {
///
/// This is just a 4 byte structure that contains a pointer to the Falcon
data in the FWSEC
/// image.
- fn falcon_data_ptr(&self, dev: &device::Device) ->
Result<u32> {
+ fn falcon_data_ptr(&self) -> Result<u32> {
let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?;
// Make sure we don't go out of bounds
@@ -847,14 +850,14 @@ fn falcon_data_ptr(&self, dev: &device::Device)
-> Result<u32> {
// read the 4 bytes at the offset specified in the token
let offset = token.data_offset as usize;
let bytes: [u8; 4] = self.base.data[offset..offset +
4].try_into().map_err(|_| {
- dev_err!(dev, "Failed to convert data slice to array");
+ dev_err!(self.base.dev, "Failed to convert data slice to
array");
EINVAL
})?;
let data_ptr = u32::from_le_bytes(bytes);
if (data_ptr as usize) < self.base.data.len() {
- dev_err!(dev, "Falcon data pointer out of bounds\n");
+ dev_err!(self.base.dev, "Falcon data pointer out of
bounds\n");
return Err(EINVAL);
}
@@ -978,11 +981,10 @@ fn find_entry_by_type(&self, entry_type: u8) ->
Result<PmuLookupTableEntry> {
impl FwSecBiosBuilder {
fn setup_falcon_data(
&mut self,
- dev: &device::Device,
pci_at_image: &PciAtBiosImage,
first_fwsec: &FwSecBiosBuilder,
) -> Result {
- let mut offset = pci_at_image.falcon_data_ptr(dev)? as usize;
+ let mut offset = pci_at_image.falcon_data_ptr()? as usize;
let mut pmu_in_first_fwsec = false;
// The falcon data pointer assumes that the PciAt and FWSEC images
@@ -1005,10 +1007,15 @@ fn setup_falcon_data(
self.falcon_data_offset = Some(offset);
if pmu_in_first_fwsec {
- self.pmu_lookup_table -
Some(PmuLookupTable::new(dev, &first_fwsec.base.data[offset..])?);
+ self.pmu_lookup_table = Some(PmuLookupTable::new(
+ &self.base.dev,
+ &first_fwsec.base.data[offset..],
+ )?);
} else {
- self.pmu_lookup_table = Some(PmuLookupTable::new(dev,
&self.base.data[offset..])?);
+ self.pmu_lookup_table = Some(PmuLookupTable::new(
+ &self.base.dev,
+ &self.base.data[offset..],
+ )?);
}
match self
@@ -1021,14 +1028,18 @@ fn setup_falcon_data(
let mut ucode_offset = entry.data as usize;
ucode_offset -= pci_at_image.base.data.len();
if ucode_offset < first_fwsec.base.data.len() {
- dev_err!(dev, "Falcon Ucode offset not in second
Fwsec.\n");
+ dev_err!(self.base.dev, "Falcon Ucode offset not in
second Fwsec.\n");
return Err(EINVAL);
}
ucode_offset -= first_fwsec.base.data.len();
self.falcon_ucode_offset = Some(ucode_offset);
}
Err(e) => {
- dev_err!(dev, "PmuLookupTableEntry not found, error:
{:?}\n", e);
+ dev_err!(
+ self.base.dev,
+ "PmuLookupTableEntry not found, error: {:?}\n",
+ e
+ );
return Err(EINVAL);
}
}
@@ -1036,7 +1047,7 @@ fn setup_falcon_data(
}
/// Build the final FwSecBiosImage from this builder
- fn build(self, dev: &device::Device) -> Result<FwSecBiosImage>
{
+ fn build(self) -> Result<FwSecBiosImage> {
let ret = FwSecBiosImage {
base: self.base,
falcon_ucode_offset: self.falcon_ucode_offset.ok_or(EINVAL)?,
@@ -1044,8 +1055,8 @@ fn build(self, dev: &device::Device) ->
Result<FwSecBiosImage> {
if cfg!(debug_assertions) {
// Print the desc header for debugging
- let desc = ret.header(dev)?;
- dev_dbg!(dev, "PmuLookupTableEntry desc: {:#?}\n", desc);
+ let desc = ret.header()?;
+ dev_dbg!(ret.base.dev, "PmuLookupTableEntry desc:
{:#?}\n", desc);
}
Ok(ret)
@@ -1054,13 +1065,16 @@ fn build(self, dev: &device::Device) ->
Result<FwSecBiosImage> {
impl FwSecBiosImage {
/// Get the FwSec header ([`FalconUCodeDescV3`]).
- pub(crate) fn header(&self, dev: &device::Device) ->
Result<&FalconUCodeDescV3> {
+ pub(crate) fn header(&self) -> Result<&FalconUCodeDescV3>
{
// Get the falcon ucode offset that was found in setup_falcon_data.
let falcon_ucode_offset = self.falcon_ucode_offset;
// Make sure the offset is within the data bounds.
if falcon_ucode_offset +
core::mem::size_of::<FalconUCodeDescV3>() > self.base.data.len() {
- dev_err!(dev, "fwsec-frts header not contained within BIOS
bounds\n");
+ dev_err!(
+ self.base.dev,
+ "fwsec-frts header not contained within BIOS
bounds\n"
+ );
return Err(ERANGE);
}
@@ -1072,7 +1086,7 @@ pub(crate) fn header(&self, dev: &device::Device)
-> Result<&FalconUCodeDescV3>
let ver = (hdr & 0xff00) >> 8;
if ver != 3 {
- dev_err!(dev, "invalid fwsec firmware version: {:?}\n",
ver);
+ dev_err!(self.base.dev, "invalid fwsec firmware version:
{:?}\n", ver);
return Err(EINVAL);
}
@@ -1092,7 +1106,7 @@ pub(crate) fn header(&self, dev: &device::Device)
-> Result<&FalconUCodeDescV3>
}
/// Get the ucode data as a byte slice
- pub(crate) fn ucode(&self, dev: &device::Device, desc:
&FalconUCodeDescV3) -> Result<&[u8]> {
+ pub(crate) fn ucode(&self, desc: &FalconUCodeDescV3) ->
Result<&[u8]> {
let falcon_ucode_offset = self.falcon_ucode_offset;
// The ucode data follows the descriptor.
@@ -1104,15 +1118,16 @@ pub(crate) fn ucode(&self, dev: &device::Device,
desc: &FalconUCodeDescV3) -> Re
.data
.get(ucode_data_offset..ucode_data_offset + size)
.ok_or(ERANGE)
- .inspect_err(|_| dev_err!(dev, "fwsec ucode data not contained
within BIOS bounds\n"))
+ .inspect_err(|_| {
+ dev_err!(
+ self.base.dev,
+ "fwsec ucode data not contained within BIOS
bounds\n"
+ )
+ })
}
/// Get the signatures as a byte slice
- pub(crate) fn sigs(
- &self,
- dev: &device::Device,
- desc: &FalconUCodeDescV3,
- ) -> Result<&[Bcrt30Rsa3kSignature]> {
+ pub(crate) fn sigs(&self, desc: &FalconUCodeDescV3) ->
Result<&[Bcrt30Rsa3kSignature]> {
// The signatures data follows the descriptor.
let sigs_data_offset = self.falcon_ucode_offset +
core::mem::size_of::<FalconUCodeDescV3>();
let sigs_size @@ -1121,7 +1136,7 @@ pub(crate) fn sigs(
// Make sure the data is within bounds.
if sigs_data_offset + sigs_size > self.base.data.len() {
dev_err!(
- dev,
+ self.base.dev,
"fwsec signatures data not contained within BIOS
bounds\n"
);
return Err(ERANGE);
--
2.50.1
Danilo Krummrich
2025-Sep-01 10:37 UTC
[PATCH 0/2] gpu: nova-core: vbios: simplify device use
On 8/8/25 4:46 AM, Alexandre Courbot wrote:> This small cleanup series simplifies the use of `Device` in vbios > methods. > > The device is used for logging purposes only; thus we don't need a > pci::Device, neither do we need it to be bound. This latter property > allows us to store an `ARef` to it into structures that require logging > instead of having all their methods take an extra `dev` argument. > Removing this argument streamlines the code a bit. > > Signed-off-by: Alexandre Courbot <acourbot at nvidia.com>Acked-by: Danilo Krummrich <dakr at kernel.org>