Add tests exercising the PCI configuration space helpers.
Suggested-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Zhi Wang <zhiw@nvidia.com>
---
samples/rust/rust_driver_pci.rs | 46 +++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
index 528e672b6b89..6f0324b6bdf6 100644
--- a/samples/rust/rust_driver_pci.rs
+++ b/samples/rust/rust_driver_pci.rs
@@ -58,6 +58,50 @@ fn testdev(index: &TestIndex, bar: &Bar0) -> Result<u32> {
Ok(bar.read32(Regs::COUNT))
}
+
+ fn config_space(pdev: &pci::Device<Core>) -> Result {
+ let config = pdev.config_space()?;
+
+ // TODO: use the register!() macro for defining PCI configuration space registers once it
+ // has been move out of nova-core.
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space read8 rev ID: {:x}\n",
+ config.read8(0x8)
+ );
+
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space read16 vendor ID: {:x}\n",
+ config.read16(0)
+ );
+
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space read32 BAR 0: {:x}\n",
+ config.read32(0x10)
+ );
+
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space try_read8 rev ID: {:x}\n",
+ config.try_read8(0x8)?
+ );
+
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space try_read16 vendor ID: {:x}\n",
+ config.try_read16(0)?
+ );
+
+ dev_info!(
+ pdev.as_ref(),
+ "pci-testdev config space try_read32 BAR 0: {:x}\n",
+ config.try_read32(0x10)?
+ );
+
+ Ok(())
+ }
}
impl pci::Driver for SampleDriver {
@@ -93,6 +137,8 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> Result<Pin<KBox<Self>
Self::testdev(info, bar)?
);
+ Self::config_space(pdev)?;
+
Ok(drvdata)
}
--
2.51.0On Tue Nov 4, 2025 at 3:27 PM CET, Zhi Wang wrote:
> + fn config_space(pdev: &pci::Device<Core>) -> Result {
> + let config = pdev.config_space()?;
> +
> + // TODO: use the register!() macro for defining PCI configuration space registers once it
> + // has been move out of nova-core.
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space read8 rev ID: {:x}\n",
> + config.read8(0x8)
> + );
> +
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space read16 vendor ID: {:x}\n",
> + config.read16(0)
> + );
> +
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space read32 BAR 0: {:x}\n",
> + config.read32(0x10)
> + );
> +
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space try_read8 rev ID: {:x}\n",
> + config.try_read8(0x8)?
> + );
> +
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space try_read16 vendor ID: {:x}\n",
> + config.try_read16(0)?
> + );
> +
> + dev_info!(
> + pdev.as_ref(),
> + "pci-testdev config space try_read32 BAR 0: {:x}\n",
> + config.try_read32(0x10)?
> + );
If we want to demonstrate the fallible accessors we should try accesses outside
the bounds of the requested config space size. However, that doesn't really make
sense, because in this case the driver could have been calling
config_space_extended() instead.
So, I think the fallible versions don't really serve a purpose and we should
probably drop them.
On Tue, 04 Nov 2025 16:41:56 +0100
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Tue Nov 4, 2025 at 3:27 PM CET, Zhi Wang wrote:
> > + fn config_space(pdev: &pci::Device<Core>) -> Result {
> > + let config = pdev.config_space()?;
> > +
> > + // TODO: use the register!() macro for defining PCI
> > configuration space registers once it
> > + // has been move out of nova-core.
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space read8 rev ID: {:x}\n",
> > + config.read8(0x8)
> > + );
> > +
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space read16 vendor ID: {:x}\n",
> > + config.read16(0)
> > + );
> > +
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space read32 BAR 0: {:x}\n",
> > + config.read32(0x10)
> > + );
> > +
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space try_read8 rev ID: {:x}\n",
> > + config.try_read8(0x8)?
> > + );
> > +
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space try_read16 vendor ID:
> > {:x}\n",
> > + config.try_read16(0)?
> > + );
> > +
> > + dev_info!(
> > + pdev.as_ref(),
> > + "pci-testdev config space try_read32 BAR 0: {:x}\n",
> > + config.try_read32(0x10)?
> > + );
>
> If we want to demonstrate the fallible accessors we should try
> accesses outside the bounds of the requested config space size.
> However, that doesn't really make sense, because in this case the
> driver could have been calling config_space_extended() instead.
>
> So, I think the fallible versions don't really serve a purpose and we
> should probably drop them.
We can add them back if there is a use case in rust driver later.
Should I arrange the traits like below?
Io trait - Main trait + 32-bit access
|
| -- Common address/bound checks
|
| (accessor traits)
| -- Io Fallible trait - (MMIO backend implements)
| -- Io Infallible trait - (MMIO/ConfigSpace backend implements this)
|
| -- Io64 trait - For backend supports 64 bit access
| (accessor traits)
| -- Io64 Faillable trait (MMIO backend implements this)
| -- Io64 Infallible trait (MMIO backend implements this)
I am also thinking if we should keep 64-bit access accessor in the
backend implementation instead in the Io trait (like {read, write}
_relaxed), because I think few backend (PCI Config Space/I2C/SPI) would
support 64-bit atomic access except MMIO backend.
Z.
On 11/4/25 5:42 PM, Zhi Wang wrote:
> Should I arrange the traits like below?
>
> Io trait - Main trait + 32-bit access
> |
> | -- Common address/bound checks
> |
> | (accessor traits)
> | -- Io Fallible trait - (MMIO backend implements)
> | -- Io Infallible trait - (MMIO/ConfigSpace backend implements this)
> |
> | -- Io64 trait - For backend supports 64 bit access
> | (accessor traits)
> | -- Io64 Faillable trait (MMIO backend implements this)
> | -- Io64 Infallible trait (MMIO backend implements this)
>
> I am also thinking if we should keep 64-bit access accessor in the
> backend implementation instead in the Io trait (like {read, write}
> _relaxed), because I think few backend (PCI Config Space/I2C/SPI) would
> support 64-bit atomic access except MMIO backend.
SGTM, I think it's fine to keep 64-bit in the MMIO backend for now, we can
always split it out into a separate trait once needed.
© 2016 - 2025 Red Hat, Inc.