Zhi Wang
2025-Dec-06 12:42 UTC
[RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled
GSP firmware needs to know the VF BAR offsets to correctly calculate the
VF events.
The VF BAR information is stored in GSP_VF_INFO, which needs to be
initialized and uploaded together with the GSP_SYSTEM_INFO.
Populate GSP_VF_INFO when nova-core uploads the GSP_SYSTEM_INFO if NVIDIA
vGPU is enabled.
Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 2 +-
drivers/gpu/nova-core/gsp.rs | 8 ++-
drivers/gpu/nova-core/gsp/boot.rs | 6 +-
drivers/gpu/nova-core/gsp/commands.rs | 8 ++-
drivers/gpu/nova-core/gsp/fw.rs | 75 ++++++++++++++++++++++++
drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++-
6 files changed, 102 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 10c5ae07a891..08a41e7bd982 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -285,7 +285,7 @@ pub(crate) fn new<'a>(
sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?,
- gsp <- Gsp::new(pdev)?,
+ gsp <- Gsp::new(pdev, vgpu.vgpu_support)?,
_: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? },
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index fb6f74797178..2d9352740c28 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -115,11 +115,16 @@ pub(crate) struct Gsp {
pub(crate) cmdq: Cmdq,
/// RM arguments.
rmargs: CoherentAllocation<GspArgumentsCached>,
+ /// Support vGPU.
+ vgpu_support: bool,
}
impl Gsp {
// Creates an in-place initializer for a `Gsp` manager for `pdev`.
- pub(crate) fn new(pdev: &pci::Device<device::Bound>) ->
Result<impl PinInit<Self, Error>> {
+ pub(crate) fn new(
+ pdev: &pci::Device<device::Bound>,
+ vgpu_support: bool,
+ ) -> Result<impl PinInit<Self, Error>> {
let dev = pdev.as_ref();
let libos =
CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
dev,
@@ -156,6 +161,7 @@ pub(crate) fn new(pdev:
&pci::Device<device::Bound>) -> Result<impl PinInit<Self
logrm,
rmargs,
cmdq,
+ vgpu_support,
}))
}
}
diff --git a/drivers/gpu/nova-core/gsp/boot.rs
b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..5016c630cec3 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -33,6 +33,7 @@
gpu::Chipset,
gsp::{
commands,
+ fw::GspVfInfo,
sequencer::{
GspSequencer,
GspSequencerParams, //
@@ -136,6 +137,7 @@ pub(crate) fn boot(
sec2_falcon: &Falcon<Sec2>,
) -> Result {
let dev = pdev.as_ref();
+ let vgpu_support = self.vgpu_support;
let bios = Vbios::new(dev, bar)?;
@@ -162,8 +164,10 @@ pub(crate) fn boot(
CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1,
GFP_KERNEL | __GFP_ZERO)?;
dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw,
&fb_layout))?;
+ let vf_info = GspVfInfo::new(pdev, bar, vgpu_support)?;
+
self.cmdq
- .send_command(bar, commands::SetSystemInfo::new(pdev))?;
+ .send_command(bar, commands::SetSystemInfo::new(pdev, vf_info))?;
self.cmdq.send_command(bar, commands::SetRegistry::new())?;
gsp_falcon.reset(bar)?;
diff --git a/drivers/gpu/nova-core/gsp/commands.rs
b/drivers/gpu/nova-core/gsp/commands.rs
index 0425c65b5d6f..1d519c4ed232 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -26,6 +26,7 @@
},
fw::{
commands::*,
+ GspVfInfo,
MsgFunction, //
},
},
@@ -36,12 +37,13 @@
/// The `GspSetSystemInfo` command.
pub(crate) struct SetSystemInfo<'a> {
pdev: &'a pci::Device<device::Bound>,
+ vf_info: GspVfInfo,
}
impl<'a> SetSystemInfo<'a> {
/// Creates a new `GspSetSystemInfo` command using the parameters of
`pdev`.
- pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) ->
Self {
- Self { pdev }
+ pub(crate) fn new(pdev: &'a pci::Device<device::Bound>,
vf_info: GspVfInfo) -> Self {
+ Self { pdev, vf_info }
}
}
@@ -51,7 +53,7 @@ impl<'a> CommandToGsp for
SetSystemInfo<'a> {
type InitError = Error;
fn init(&self) -> impl Init<Self::Command, Self::InitError> {
- GspSetSystemInfo::init(self.pdev)
+ GspSetSystemInfo::init(self.pdev, self.vf_info)
}
}
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index abffd6beec65..a0581ac34586 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -9,8 +9,10 @@
use core::ops::Range;
use kernel::{
+ device,
dma::CoherentAllocation,
fmt,
+ pci,
prelude::*,
ptr::{
Alignable,
@@ -27,6 +29,7 @@
};
use crate::{
+ driver::Bar0,
fb::FbLayout,
firmware::gsp::GspFirmware,
gpu::Chipset,
@@ -926,3 +929,75 @@ fn new(cmdq: &Cmdq) -> Self {
})
}
}
+
+/// VF information - gspVFInfo in SetSystemInfo.
+#[derive(Clone, Copy, Zeroable)]
+#[repr(transparent)]
+pub(crate) struct GspVfInfo {
+ inner: bindings::GSP_VF_INFO,
+}
+
+impl GspVfInfo {
+ /// Creates a new GspVfInfo structure.
+ pub(crate) fn new<'a>(
+ pdev: &'a pci::Device<device::Bound>,
+ bar: &Bar0,
+ vgpu_support: bool,
+ ) -> Result<GspVfInfo> {
+ let mut vf_info = GspVfInfo::zeroed();
+ let info = &mut vf_info.inner;
+
+ if vgpu_support {
+ let val = pdev.sriov_get_totalvfs()?;
+ info.totalVFs = u32::try_from(val)?;
+
+ let pos = pdev
+ .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as
i32)
+ .ok_or(ENODEV)?;
+
+ let val = pdev.config_read_word(
+ i32::from(pos) +
i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32),
+ )?;
+ info.firstVFOffset = u32::from(val);
+
+ let val = pdev.config_read_dword(
+ i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as
i32),
+ )?;
+ info.FirstVFBar0Address = u64::from(val);
+
+ let bar1_lo = pdev.config_read_dword(
+ i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as
i32 + 4),
+ )?;
+ let bar1_hi = pdev.config_read_dword(
+ i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as
i32 + 8),
+ )?;
+
+ let addr_mask =
u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?;
+
+ info.FirstVFBar1Address + (u64::from(bar1_hi)
<< 32) | ((u64::from(bar1_lo)) & addr_mask);
+
+ let bar2_lo = pdev.config_read_dword(
+ i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as
i32 + 12),
+ )?;
+ let bar2_hi = pdev.config_read_dword(
+ i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as
i32 + 16),
+ )?;
+
+ info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) |
(u64::from(bar2_lo) & addr_mask);
+
+ let val = bar.read32(0x88000 + 0xbf4);
+ info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004);
+
+ let val = bar.read32(0x88000 + 0xbfc);
+ info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);
+ }
+ Ok(vf_info)
+ }
+}
+
+// SAFETY: Padding is explicit and does not contain uninitialized data.
+unsafe impl AsBytes for GspVfInfo {}
+
+// SAFETY: This struct only contains integer types for which all bit patterns
are valid.
+unsafe impl FromBytes for GspVfInfo {}
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs
b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 21be44199693..3b5c05704b2d 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -4,7 +4,10 @@
use kernel::transmute::{AsBytes, FromBytes};
use kernel::{device, pci};
-use crate::gsp::GSP_PAGE_SIZE;
+use crate::gsp::{
+ fw::GspVfInfo,
+ GSP_PAGE_SIZE, //
+};
use super::bindings;
@@ -18,7 +21,10 @@ pub(crate) struct GspSetSystemInfo {
impl GspSetSystemInfo {
/// Returns an in-place initializer for the `GspSetSystemInfo` command.
#[allow(non_snake_case)]
- pub(crate) fn init<'a>(dev: &'a
pci::Device<device::Bound>) -> impl Init<Self, Error> + 'a {
+ pub(crate) fn init<'a>(
+ dev: &'a pci::Device<device::Bound>,
+ info: GspVfInfo,
+ ) -> impl Init<Self, Error> + 'a {
type InnerGspSystemInfo = bindings::GspSystemInfo;
let init_inner = try_init!(InnerGspSystemInfo {
gpuPhysAddr: dev.resource_start(0)?,
@@ -38,6 +44,7 @@ pub(crate) fn init<'a>(dev: &'a
pci::Device<device::Bound>) -> impl Init<Self, E
PCIRevisionID: u32::from(dev.revision_id()),
bIsPrimary: 0,
bPreserveVideoMemoryAllocations: 0,
+ gspVFInfo: info.inner,
..Zeroable::init_zeroed()
});
--
2.51.0
Joel Fernandes
2025-Dec-07 02:32 UTC
[RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled
Hi Zhi, On 12/6/2025 7:42 AM, Zhi Wang wrote: [...]> +/// VF information - gspVFInfo in SetSystemInfo. > +#[derive(Clone, Copy, Zeroable)] > +#[repr(transparent)] > +pub(crate) struct GspVfInfo { > + inner: bindings::GSP_VF_INFO, > +} > + > +impl GspVfInfo { > + /// Creates a new GspVfInfo structure. > + pub(crate) fn new<'a>( > + pdev: &'a pci::Device<device::Bound>, > + bar: &Bar0, > + vgpu_support: bool, > + ) -> Result<GspVfInfo> { > + let mut vf_info = GspVfInfo::zeroed(); > + let info = &mut vf_info.inner; > + > + if vgpu_support { > + let val = pdev.sriov_get_totalvfs()?; > + info.totalVFs = u32::try_from(val)?; > + > + let pos = pdev > + .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as i32) > + .ok_or(ENODEV)?; > + > + let val = pdev.config_read_word( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32), > + )?; > + info.firstVFOffset = u32::from(val); > + > + let val = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32), > + )?; > + info.FirstVFBar0Address = u64::from(val); > + > + let bar1_lo = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 4), > + )?; > + let bar1_hi = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 8), > + )?; > + > + let addr_mask = u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?; > + > + info.FirstVFBar1Address > + (u64::from(bar1_hi) << 32) | ((u64::from(bar1_lo)) & addr_mask); > + > + let bar2_lo = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 12), > + )?; > + let bar2_hi = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 16), > + )?; > + > + info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) | (u64::from(bar2_lo) & addr_mask); > + > + let val = bar.read32(0x88000 + 0xbf4); > + info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004); > + > + let val = bar.read32(0x88000 + 0xbfc); > + info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);Please no magic numbers, please use proper named constants with documentation comments explaining the values. Also BAR reads here need proper register macro definitions/access. Also the above code is lacking in comments. All the steps above need proper comments IMO. General philosophy of Nova is it is a well documented, cleanly written driver with minimal/no magic numbers and abundant comments. :) Thanks.
Alexandre Courbot
2025-Dec-10 14:27 UTC
[RFC 4/7] gpu: nova-core: populate GSP_VF_INFO when vGPU is enabled
On Sat Dec 6, 2025 at 9:42 PM JST, Zhi Wang wrote:> GSP firmware needs to know the VF BAR offsets to correctly calculate the > VF events. > > The VF BAR information is stored in GSP_VF_INFO, which needs to be > initialized and uploaded together with the GSP_SYSTEM_INFO. > > Populate GSP_VF_INFO when nova-core uploads the GSP_SYSTEM_INFO if NVIDIA > vGPU is enabled. > > Signed-off-by: Zhi Wang <zhiw at nvidia.com> > --- > drivers/gpu/nova-core/gpu.rs | 2 +- > drivers/gpu/nova-core/gsp.rs | 8 ++- > drivers/gpu/nova-core/gsp/boot.rs | 6 +- > drivers/gpu/nova-core/gsp/commands.rs | 8 ++- > drivers/gpu/nova-core/gsp/fw.rs | 75 ++++++++++++++++++++++++ > drivers/gpu/nova-core/gsp/fw/commands.rs | 11 +++- > 6 files changed, 102 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs > index 10c5ae07a891..08a41e7bd982 100644 > --- a/drivers/gpu/nova-core/gpu.rs > +++ b/drivers/gpu/nova-core/gpu.rs > @@ -285,7 +285,7 @@ pub(crate) fn new<'a>( > > sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset)?, > > - gsp <- Gsp::new(pdev)?, > + gsp <- Gsp::new(pdev, vgpu.vgpu_support)?,This seems like it is making the whole `Vgpu` structure introduced in the previous patch superfluous, since its sole purpose is to pass the `vgpu_support` value to `Gsp::new` - we could just extract that value in `Gsp::new`, or better in `Gsp::boot`, where we actually use it, and avoid storing it in 3 different places.> > _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? }, > > diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs > index fb6f74797178..2d9352740c28 100644 > --- a/drivers/gpu/nova-core/gsp.rs > +++ b/drivers/gpu/nova-core/gsp.rs > @@ -115,11 +115,16 @@ pub(crate) struct Gsp { > pub(crate) cmdq: Cmdq, > /// RM arguments. > rmargs: CoherentAllocation<GspArgumentsCached>, > + /// Support vGPU. > + vgpu_support: bool, > } > > impl Gsp { > // Creates an in-place initializer for a `Gsp` manager for `pdev`. > - pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self, Error>> { > + pub(crate) fn new( > + pdev: &pci::Device<device::Bound>, > + vgpu_support: bool, > + ) -> Result<impl PinInit<Self, Error>> { > let dev = pdev.as_ref(); > let libos = CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent( > dev, > @@ -156,6 +161,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self > logrm, > rmargs, > cmdq, > + vgpu_support, > })) > } > } > diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs > index 54937606b5b0..5016c630cec3 100644 > --- a/drivers/gpu/nova-core/gsp/boot.rs > +++ b/drivers/gpu/nova-core/gsp/boot.rs > @@ -33,6 +33,7 @@ > gpu::Chipset, > gsp::{ > commands, > + fw::GspVfInfo, > sequencer::{ > GspSequencer, > GspSequencerParams, // > @@ -136,6 +137,7 @@ pub(crate) fn boot( > sec2_falcon: &Falcon<Sec2>, > ) -> Result { > let dev = pdev.as_ref(); > + let vgpu_support = self.vgpu_support; > > let bios = Vbios::new(dev, bar)?; > > @@ -162,8 +164,10 @@ pub(crate) fn boot( > CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?; > dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?; > > + let vf_info = GspVfInfo::new(pdev, bar, vgpu_support)?;This is a strange constructor. It initializes the `GspVfInfo` if `vgpu_support` is true, and returns a zeroed structure otherwise. I'd rather have an unconditional constructor that is only called if `vgpu_support` is true, and store the result in an `Option`: let vf_info = if vgpu_support { Some(GspVfInfo::new(pdev, bar)?) } else { None }; It will become clearer later why this is a better design.> + > self.cmdq > - .send_command(bar, commands::SetSystemInfo::new(pdev))?; > + .send_command(bar, commands::SetSystemInfo::new(pdev, vf_info))?;As a result of the previous comment, `SetSystemInfo::new` now takes an `Option<GspVfInfo>`...> self.cmdq.send_command(bar, commands::SetRegistry::new())?; > > gsp_falcon.reset(bar)?; > diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs > index 0425c65b5d6f..1d519c4ed232 100644 > --- a/drivers/gpu/nova-core/gsp/commands.rs > +++ b/drivers/gpu/nova-core/gsp/commands.rs > @@ -26,6 +26,7 @@ > }, > fw::{ > commands::*, > + GspVfInfo, > MsgFunction, // > }, > }, > @@ -36,12 +37,13 @@ > /// The `GspSetSystemInfo` command. > pub(crate) struct SetSystemInfo<'a> { > pdev: &'a pci::Device<device::Bound>, > + vf_info: GspVfInfo,... and this becomes an `Option` as well.> } > > impl<'a> SetSystemInfo<'a> { > /// Creates a new `GspSetSystemInfo` command using the parameters of `pdev`. > - pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) -> Self { > - Self { pdev } > + pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, vf_info: GspVfInfo) -> Self { > + Self { pdev, vf_info } > } > } > > @@ -51,7 +53,7 @@ impl<'a> CommandToGsp for SetSystemInfo<'a> { > type InitError = Error; > > fn init(&self) -> impl Init<Self::Command, Self::InitError> { > - GspSetSystemInfo::init(self.pdev) > + GspSetSystemInfo::init(self.pdev, self.vf_info)And here things become interesting: you can leave the constructor if `GspSetSystemInfo` unchanged, since vgpu support is not strictly required to produce a valid value. But you can also chain the initializer to add the vgpu information when relevant: GspSetSystemInfo::init(self.pdev).chain(|info| { if let Some(vf_info) = &self.vf_info { info.set_vf_info(vf_info); } Ok(()) }) (more on `set_vf_info` below) This results in less code overall, and better conveys the fact that vgpu support is technically optional.> } > } > > diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs > index abffd6beec65..a0581ac34586 100644 > --- a/drivers/gpu/nova-core/gsp/fw.rs > +++ b/drivers/gpu/nova-core/gsp/fw.rs > @@ -9,8 +9,10 @@ > use core::ops::Range; > > use kernel::{ > + device, > dma::CoherentAllocation, > fmt, > + pci, > prelude::*, > ptr::{ > Alignable, > @@ -27,6 +29,7 @@ > }; > > use crate::{ > + driver::Bar0, > fb::FbLayout, > firmware::gsp::GspFirmware, > gpu::Chipset, > @@ -926,3 +929,75 @@ fn new(cmdq: &Cmdq) -> Self { > }) > } > } > + > +/// VF information - gspVFInfo in SetSystemInfo. > +#[derive(Clone, Copy, Zeroable)] > +#[repr(transparent)] > +pub(crate) struct GspVfInfo { > + inner: bindings::GSP_VF_INFO, > +}You can use a tuple struct here (i.e. `struct GspVfInfo(bindings::GSP_VF_INFO`). Also none of the derives should be needed eventually.> + > +impl GspVfInfo { > + /// Creates a new GspVfInfo structure. > + pub(crate) fn new<'a>( > + pdev: &'a pci::Device<device::Bound>, > + bar: &Bar0, > + vgpu_support: bool,As mentioned before, we can drop this bool argument.> + ) -> Result<GspVfInfo> { > + let mut vf_info = GspVfInfo::zeroed(); > + let info = &mut vf_info.inner;It is generally considered better practice to avoid mutating values we initialize. By starting with a zeroed state and then initializing the members, you are at risk of forgetting to initialize some. What you should do is initialize your `GspVfInfo` in its entirety in a final statement like the other command structures do, storing intermediate values in temporary variables if needed.> + > + if vgpu_support { > + let val = pdev.sriov_get_totalvfs()?; > + info.totalVFs = u32::try_from(val)?;We should be able to avoid this `try_from` once `sriov_get_totalvfs` returns the correct `u16` type as suggested on the first patch. :)> + > + let pos = pdev > + .find_ext_capability(kernel::bindings::PCI_EXT_CAP_ID_SRIOV as i32) > + .ok_or(ENODEV)?;`pos` also seems to be the wrong type - it seems to me that it should be unsigned...> + > + let val = pdev.config_read_word( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_VF_OFFSET as i32), > + )?;... but I guess this comes from here - I understand that `config_read_word` takes an `int` for its offset parameter, but when I read the C code I also see checks that return errors if that offset is bigger than 4095. What does the PCI spec says? It seems we can go with a `u16` for the offset, which would simplify this code quite a bit. Also please don't use `as` whenever possible, there are utility functions in `crate::num` to do the conversions infallibly. You will probably be interested in `u32_into_u16` for this method.> + info.firstVFOffset = u32::from(val); > + > + let val = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32), > + )?; > + info.FirstVFBar0Address = u64::from(val); > + > + let bar1_lo = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 4), > + )?; > + let bar1_hi = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 8), > + )?; > + > + let addr_mask = u64::try_from(kernel::bindings::PCI_BASE_ADDRESS_MEM_MASK)?;This `try_from` will always fail as `PCI_BASE_ADDRESS_MEM_MASK` is negative. This is a case for a legit use of `as` (with a CAST: comment), although this is also a case for us generating better bindings. :)> + > + info.FirstVFBar1Address > + (u64::from(bar1_hi) << 32) | ((u64::from(bar1_lo)) & addr_mask); > + > + let bar2_lo = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 12), > + )?; > + let bar2_hi = pdev.config_read_dword( > + i32::from(pos) + i32::from(kernel::bindings::PCI_SRIOV_BAR as i32 + 16), > + )?; > + > + info.FirstVFBar2Address = (u64::from(bar2_hi) << 32) | (u64::from(bar2_lo) & addr_mask); > + > + let val = bar.read32(0x88000 + 0xbf4); > + info.b64bitBar1 = u8::from((val & 0x00000006) == 0x00000004); > + > + let val = bar.read32(0x88000 + 0xbfc); > + info.b64bitBar2 = u8::from((val & 0x00000006) == 0x00000004);Magic numbers baaaaaaad. Let's define these as proper registers.