| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250602151506.GA779285@joelnvbox>
Date: Mon, 2 Jun 2025 11:15:06 -0400
From: Joel Fernandes <joelagnelf@...dia.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Alexandre Courbot <acourbot@...dia.com>,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
John Hubbard <jhubbard@...dia.com>, Ben Skeggs <bskeggs@...dia.com>,
Timur Tabi <ttabi@...dia.com>, Alistair Popple <apopple@...dia.com>,
linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
Shirish Baskaran <sbaskaran@...dia.com>
Subject: Re: [PATCH v4 16/20] nova-core: Add support for VBIOS ucode
extraction for boot
On Mon, Jun 02, 2025 at 03:33:56PM +0200, Danilo Krummrich wrote:
> On Wed, May 21, 2025 at 03:45:11PM +0900, Alexandre Courbot wrote:
> > +impl Vbios {
>
> <snip>
>
> > + pub(crate) fn fwsec_header(&self, pdev: &device::Device) -> Result<&FalconUCodeDescV3> {
> > + self.fwsec_image.fwsec_header(pdev)
> > + }
> > +
> > + pub(crate) fn fwsec_ucode(&self, pdev: &device::Device) -> Result<&[u8]> {
> > + self.fwsec_image.fwsec_ucode(pdev, self.fwsec_header(pdev)?)
> > + }
> > +
> > + pub(crate) fn fwsec_sigs(&self, pdev: &device::Device) -> Result<&[u8]> {
> > + self.fwsec_image.fwsec_sigs(pdev, self.fwsec_header(pdev)?)
> > + }
>
> Can't we just implement Deref here? Why do we need this indirection?
We could, but it seems weird to deref a Vbios struct to an FwsecBiosImage
struct. Conceptually a Vbios is a collection of things and it could have
future extensions to its struct.
The win with using Deref is also not that much, just 2 lines fewer since the
deleted functions are replaced by the the impl Deref block. But I am Ok with
it either way, here is the diff on top of this patch.
Or did I miss something about the suggestion? Will respond to the other
comments, soon, Thanks.
---8<-----------------------
diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 346d48c4820c..ccf83b206758 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -6,6 +6,7 @@
use crate::firmware::fwsec::Bcrt30Rsa3kSignature;
use crate::firmware::FalconUCodeDescV3;
use core::convert::TryFrom;
+use core::ops::Deref;
use kernel::device;
use kernel::error::Result;
use kernel::num::NumExt;
@@ -247,17 +248,13 @@ pub(crate) fn new(pdev: &pci::Device, bar0: &Bar0) -> Result<Vbios> {
Err(EINVAL)
}
}
+}
- pub(crate) fn fwsec_header(&self, pdev: &device::Device) -> Result<&FalconUCodeDescV3> {
- self.fwsec_image.fwsec_header(pdev)
- }
-
- pub(crate) fn fwsec_ucode(&self, pdev: &device::Device) -> Result<&[u8]> {
- self.fwsec_image.fwsec_ucode(pdev, self.fwsec_header(pdev)?)
- }
+impl Deref for Vbios {
+ type Target = FwSecBiosImage;
- pub(crate) fn fwsec_sigs(&self, pdev: &device::Device) -> Result<&[Bcrt30Rsa3kSignature]> {
- self.fwsec_image.fwsec_sigs(pdev, self.fwsec_header(pdev)?)
+ fn deref(&self) -> &Self::Target {
+ &self.fwsec_image
}
}
@@ -735,7 +732,7 @@ struct FwSecBiosPartial {
falcon_ucode_offset: Option<usize>,
}
-struct FwSecBiosImage {
+pub(crate) struct FwSecBiosImage {
base: BiosImageBase,
// The offset of the Falcon ucode
falcon_ucode_offset: usize,
@@ -1091,7 +1088,7 @@ fn new(pdev: &pci::Device, data: FwSecBiosPartial) -> Result<Self> {
}
/// Get the FwSec header (FalconUCodeDescV3)
- fn fwsec_header(&self, dev: &device::Device) -> Result<&FalconUCodeDescV3> {
+ pub(crate) fn fwsec_header(&self, dev: &device::Device) -> Result<&FalconUCodeDescV3> {
// Get the falcon ucode offset that was found in setup_falcon_data
let falcon_ucode_offset = self.falcon_ucode_offset;
@@ -1119,9 +1116,11 @@ fn fwsec_header(&self, dev: &device::Device) -> Result<&FalconUCodeDescV3> {
&*(self.base.data.as_ptr().add(falcon_ucode_offset) as *const FalconUCodeDescV3)
})
}
+
/// Get the ucode data as a byte slice
- fn fwsec_ucode(&self, dev: &device::Device, desc: &FalconUCodeDescV3) -> Result<&[u8]> {
+ pub(crate) fn fwsec_ucode(&self, dev: &device::Device) -> Result<&[u8]> {
let falcon_ucode_offset = self.falcon_ucode_offset;
+ let desc = self.fwsec_header(dev)?;
// The ucode data follows the descriptor
let ucode_data_offset = falcon_ucode_offset + desc.size();
@@ -1136,17 +1135,17 @@ fn fwsec_ucode(&self, dev: &device::Device, desc: &FalconUCodeDescV3) -> Result<
}
/// Get the FWSEC signatures.
- fn fwsec_sigs(
+ pub(crate) fn fwsec_sigs(
&self,
dev: &device::Device,
- v3_desc: &FalconUCodeDescV3,
) -> Result<&[Bcrt30Rsa3kSignature]> {
let falcon_ucode_offset = self.falcon_ucode_offset;
+ let desc = self.fwsec_header(dev)?;
// The signatures data follows the descriptor
let sigs_data_offset = falcon_ucode_offset + core::mem::size_of::<FalconUCodeDescV3>();
let sigs_size =
- v3_desc.signature_count as usize * core::mem::size_of::<Bcrt30Rsa3kSignature>();
+ desc.signature_count as usize * core::mem::size_of::<Bcrt30Rsa3kSignature>();
// Make sure the data is within bounds
if sigs_data_offset + sigs_size > self.base.data.len() {
@@ -1166,9 +1165,8 @@ fn fwsec_sigs(
.as_ptr()
.add(sigs_data_offset)
.cast::<Bcrt30Rsa3kSignature>(),
- v3_desc.signature_count as usize,
+ desc.signature_count as usize,
)
})
}
-}
-
+}
\ No newline at end of file
Powered by blists - more mailing lists