Re: [PATCH v5] vfio: selftests: Find devices that have VFIO selftest drivers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Documentation and implementation mismatch regarding script name, output format, and usage instructions.
- [Low] Unused local variable `bdf`.
--

commit dac67adaa35d0b3beddd4d11badc85a43ad7c2e3
Author: Josh Hilke <jrhilke@google.com>

vfio: selftests: Find devices that have VFIO selftest drivers

This commit adds a new script to print the SBDF numbers and names of devices
that have a VFIO selftest driver. It also includes a quiet mode to print only
the SBDFs.

>     Add a new script, list_devices.sh, which prints out the
>     segment:bus:device.function (SBDF) numbers and names of devices on a
>     machine that have a VFIO selftest driver. This makes it easier to
>     determine if the system is capable of running VFIO selftests.
>     
>     Includes a -q (quiet) argument which prints just the SBDFs so that the
>     output can be piped into tools/testing/selftests/vfio/script/setup.sh to
>     bind the devices to VFIO to use in VFIO selftests.

[Severity: Medium]
Are these instructions accurate for how the scripts are meant to be used?

The script is named list_supported_devices.sh in the patch, rather than
list_devices.sh.

Additionally, the commit message states the output can be piped directly into
script/setup.sh (which is missing an 's' for scripts/), but setup.sh requires
arguments to be passed on the command line rather than through standard
input. Piping directly might result in a usage error.

>     Examples:
>     
>     $ ./list_devices.sh
>     0000:6a:01.0 - Intel SPR DSA (8086:0b25)

[Severity: Medium]
The output format in the example shows "Intel SPR DSA (8086:0b25)", but
the script below generates "Intel DSA SPR [8086:0b25]". Could the commit
message be updated to match the implementation?

> diff --git a/tools/testing/selftests/vfio/scripts/list_supported_devices.sh b/tools/testing/selftests/vfio/scripts/list_supported_devices.sh
> new file mode 100755
> index 0000000000000..a123a965d3586
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/scripts/list_supported_devices.sh
> @@ -0,0 +1,44 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later

[ ... ]

> +function main() {
> +	local id_name
> +	local quiet=""
> +	local name
> +	local bdfs
> +	local bdf

[Severity: Low]
Is this bdf variable used anywhere in the script? It appears to be declared
but never assigned or read.

> +	local id

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602185615.3025904-1-jrhilke@google.com?part=1