From: Sean Christopherson <seanjc@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: David Matlack <dmatlack@google.com>,
Josh Hilke <jrhilke@google.com>,
Alex Williamson <alex@shazbot.org>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Raghavendra Rao Ananta <rananta@google.com>
Subject: Re: [PATCH v6] vfio: selftests: Find devices that have VFIO selftest drivers
Date: Wed, 17 Jun 2026 12:38:27 -0700 [thread overview]
Message-ID: <ajL3s6TybZPBC5vJ@google.com> (raw)
In-Reply-To: <20260617180507.GB2704872.vipinsh@google.com>
On Wed, Jun 17, 2026, Vipin Sharma wrote:
> On Wed, Jun 17, 2026 at 09:34:18AM -0700, David Matlack wrote:
> > On Wed, Jun 17, 2026 at 9:28 AM Vipin Sharma <vipinsh@google.com> wrote:
> > >
> > > On Wed, Jun 17, 2026 at 03:37:35PM +0000, David Matlack wrote:
> > > > On 2026-06-05 11:19 AM, Sean Christopherson wrote:
> > > > > On Fri, Jun 05, 2026, David Matlack wrote:
> > > > > > On Thu, Jun 4, 2026 at 1:30 PM David Matlack <dmatlack@google.com> wrote:
> > > > > > >
> > > > > > > On Thu, Jun 4, 2026 at 12:50 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > > > >
> > > > > > > > On Tue, Jun 02, 2026, Josh Hilke wrote:
> > > > > > > > > Add a new script, list_supported_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 passed to tools/testing/selftests/vfio/scripts/setup.sh
> > > > > > > > > (e.g. via xargs) to bind the devices to VFIO to use in VFIO selftests.
> > > > > > > > >
> > > > > > > > > Examples:
> > > > > > > > >
> > > > > > > > > $ ./list_supported_devices.sh
> > > > > > > > > 0000:6a:01.0 - Intel DSA SPR [8086:0b25]
> > > > > > > > > 0000:6f:01.0 - Intel DSA SPR [8086:0b25]
> > > > > > > > > 0000:74:01.0 - Intel DSA SPR [8086:0b25]
> > > > > > > > >
> > > > > > > > > $ ./list_supported_devices.sh -q
> > > > > > > > > 0000:6a:01.0
> > > > > > > > > 0000:6f:01.0
> > > > > > > > > 0000:74:01.0
> > > > > > > > >
> > > > > > > > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > > > > > > > Signed-off-by: Josh Hilke <jrhilke@google.com>
> > > > > > > >
> > > > > > > > NAK, until this stuff is fixed and properly documented.
> > > > > > > >
> > > > > > > > This script needs to (a) communicate that some of the devices may be on VFIO's
> > > > > > > > denylist, (b) NOT report them by default, (c) let the user report denylisted
> > > > > > > > devices, (d) make it *very* clear in the output that a device, and (e) ideally
> > > > > > > > provide the user with a hint of how that might be able to test a denied device,
> > > > > > > > e.g. by reloading vfio-pci with disable_denylist=1.
> > > > > > > >
> > > > > > > > Nothing in here so much as mentions that these "supported" devices may be
> > > > > > > > disallowed by the kernel, including the devices that's USED AS THE EXAMPLE.
> > > > > > > >
> > > > > > > > The DSA SPR devices has been on the naughty list since commit 95feb3160eef ("VFIO:
> > > > > > > > Add the SPR_DSA and SPR_IAX devices to the denylist") from 2024, so I have a
> > > > > > > > very hard time believing y'all weren't aware of this. And if you really truly
> > > > > > > > weren't aware of this quirk, than what exactly are you even testing!?!?
> > > > > > > >
> > > > > > > > C'mon.
> > > > > > >
> > > > > > > Yeah vfio_pci.disable_denylist=Y is required to use the Intel DSA SPR
> > > > > > > device. This script is a great place to capture that previously hidden
> > > > > > > requirement.
> > > > > > >
> > > > > > > Your proposed (a)-(e) sound good to me.
> > > > > >
> > > > > > Another direction we could go in here would be to set disable_denylist
> > > > > > to Y in setup.sh (if it is currently N) and set it back to N in
> > > > > > cleanup.sh (after the last device is cleaned up and only if it was
> > > > > > previously N). I was already thinking about doing something similar
> > > > > > for setting enable_srio to Y for Raghu's new SR-IOV uAPI test.
> > > > > >
> > > > > > The only issue is that disable_denylist is not currently writable. I
> > > > > > don't see any functional reason why it can't be writable though. It is
> > > > > > just sampled during probe. And from a security perspective, privileged
> > > > > > users can already reload vfio-pci with different parameters if it is
> > > > > > built as a module. But for built-ins, this would enable the denylist
> > > > > > to be disabled.
> > > > >
> > > > > IMO, any changes to setup.h are completely orthogonal. list_supported_devices.sh
> > > > > can and will be run independently, e.g. by people like me, and so absolutely needs
> > > > > to claim a device is supported when it's obviously not.
> > > > >
> > > > > Actually, that's a good amendment to (b) above. Instead of *never* reporting
> > > > > denylisted devices by default, have the script check disable_denylist and act
> > > > > accordingly. And then it's "just" a matter of communicating these details to
> > > > > the user, e.g. so that a user can find devices that are supported by selftests,
> > > > > but not by their currently loaded kernel+modules.
> > > >
> > > > SGTM
> > > >
> > > > Josh can you do this in v7? We'll probably need a flag to control
> > > > whether the script lists devices not currently supported due to
> > > > denylist, and how to convey all that to the user. I lean towards
> > > > list_supported_devices.sh printing as much information as possible when
> > > > run without arguments. So maybe a flag to hide denylisted devices.
> > >
> > > Do we need then flag to hide denylisted devices? Instead, I think it should be
> > > opposite (I think that what Sean is suggesting as amendment). If
> > > the script finds vfio_pci.disable_denylist param value is:
> > > - Y, then just show those DSA devices if they are available on the
> > > host. Don't differenciate.
> > > - N, then don't show these denylist devices.
That's going to be extremely frustrating for users like me, that aren't even aware
that there's a denylist (well, now I am, but I wasn't when I ran the script).
> > > A flag can be added to show all of the VFIO selftests supported devices
> > > present on the host with the warning next to denylisted devices or group
> > > them in the output.
> > >
> > > Flag case is for human consumption only.
I disagree, strongly, with this particular statement. IMO, the sole requirement
we should be considering with respect to flags is the default behavior for humans.
Everything beyond that should be an implementation detail. Saying "flags are only
for humans" is placing pointless constraints on the implementation.
> > Can you explain why it should be that way?
> >
> > IMO running list_supported_devices.sh without any arguments should
> > display as much information as possible. I think that will maximimize
> > the probability that people running VFIO selftests for the first time
> > will be able to figure things out for themselves.
>
> My view is that setup.sh should get supported device list by default
> using the list_supported_devices.sh which should correctly tell what is
> supported based on the kernel one has booted and wanted to test.
That's fine, but how setup.sh behaves is largely orthogonal.
> We also have in-progress series for IGB and MLX devices selftest driver
> support, these devices are not in the denylist.
How is that relevant?
> People running VFIO selftests will be intentionally running them on
> supported hardware.
Uh, no. I'm proof that that's not the case. I'm using the script to see if a
system has a supported device.
> They will also first need to check/read scripts on how to use the setup.sh,
> run.sh, and cleanup.sh, or manually pass device BDF value (via args or
> environment variable). These things are not default and one will not have a
> successful run in the first try without knowing these internal details.
I disagree. I have pre-existing scripts to configure VFIO devices, the *only*
thing I need from list_supported_devices.sh is what its name literally says: a
list of supported devices.
> vfio-pci module default is to disable use of these denylist devices. To
> use these devices one has to set Y to the vfio_pci.disable_denytlist
> module param and reboot the kernel.
No, most kernels build vfio-pci as an actual module. A reboot isn't required,
just a module reload.
> So, a pre-requisite for running VFIO selftest is to be an advance user and
I again disagree very strongly. The goal should be to make it as easy as possible
for people to run tests. Claiming that only advanced users will or should run
tests is only going to reduce the overall health of the kernel.
> I don't think they will have any trouble in figuring out these things.
I did. In fact, I was so frustrated with the whole experience that I literally
stepped away for several hours.
> If they see no run has happened or they want to explore more,
> list_supported_devices.sh will be an easy read to discover flags.
Why make the user jump through an extra hoop or two?
> I think my suggested approach is advocating for keeping things in sync
> with what kernel default is or booted with. It looks cleaner to me and
> respect safeguards provided by the kernel.
Printing out that "Hey, this device is supported, but it's denylisted by the
kernel by fault" isn't disprecting kernel safeguards.
prev parent reply other threads:[~2026-06-17 19:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-02 22:29 [PATCH v6] vfio: selftests: Find devices that have VFIO selftest drivers Josh Hilke
2026-06-04 19:50 ` Sean Christopherson
2026-06-04 20:30 ` David Matlack
2026-06-05 16:11 ` David Matlack
2026-06-05 18:19 ` Sean Christopherson
2026-06-17 15:37 ` David Matlack
2026-06-17 16:28 ` Vipin Sharma
2026-06-17 16:34 ` David Matlack
2026-06-17 19:07 ` Vipin Sharma
2026-06-17 19:38 ` Sean Christopherson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajL3s6TybZPBC5vJ@google.com \
--to=seanjc@google.com \
--cc=alex@shazbot.org \
--cc=dmatlack@google.com \
--cc=jrhilke@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rananta@google.com \
--cc=vipinsh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.