public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: David Matlack <dmatlack@google.com>
Cc: Josh Hilke <jrhilke@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org,  Alex Williamson <alex@shazbot.org>
Subject: Re: [PATCH v2 00/14] KVM: selftests: Link with VFIO selftests lib and test device interrupts
Date: Wed, 1 Apr 2026 12:07:19 -0700	[thread overview]
Message-ID: <ac1s55_oLEUQx8Hs@google.com> (raw)
In-Reply-To: <ac1pUx_jYv2yCMnX@google.com>

On Wed, Apr 01, 2026, David Matlack wrote:
> On 2026-04-01 11:17 AM, Sean Christopherson wrote:
> > On Tue, Mar 31, 2026, Josh Hilke wrote:
> 
> > > This test only supports x86. Testing physical device interrupts (-d argument) is
> > > only supported for Intel because there are no VFIO selftest drivers for AMD
> > > devices.
> > 
> > This needs to be expressed in code and human readable error messages to the user.
> > I already knew the basic of what the VFIO selfests are doing, and it still took
> > me 5+ minutes to find the DSA driver and probing logic.
> > 
> > Concretely, this code needs to be part of the standard vfio selftests APIs:
> > 
> > 	device = vfio_pci_device_init(device_bdf, iommu);
> > 
> > 	has_driver = !!device->driver.ops;
> > 
> > E.g. have vfio_pci_device_init() explicitly return an error (or NULL) if it can't
> > init the device driver.
> 
> No. Not every VFIO selftest or user of libvfio needs a driver. A driver
> is only needed in tests that want to use the driver, i.e. want to call
> one of the very obviously named functions:
> 
>  - vfio_pci_driver_send_msi()
>  - vfio_pci_driver_memcpy()
>  - etc.
> 
> Requiring a driver for vfio_pci_device_init() to return non-NULL would
> significantly reduce our ability to test VFIO on different platforms and
> environments.

Ah, sorry.  I was speed reading and missed that some (most?) tests don't require
a driver.  Scratch the "make it unconditional", but there still needs to be an
API to up-level binding to a driver.  KVM selftests shouldn't have to manually
check for that.

> > The fixtures in vfio_pci_driver_test.c already blindly assume success, so you
> > might not need to do much beyond changing main() in that file.
> 
> I don't follow. What is the issue with vfio_pci_driver_test.?

What I was trying to say is that if vfio_pci_device_init() were to unconditionally
require a driver, then vfio_pci_driver_test() should Just Work.  But that doesn't
make a whole lot of sense given the above.  Though it could use the new API that
checks for a driver.

> > And then, also as a vfio_pci_device_xxx() API, add a helper to print out what
> > devices are supported, e.g. so that KVM selftests can do something like:
> 
> Agree this would be nice to have. Is this a blocker for this series?

Yes.  KVM selftests need to assert there's a driver before trying to proceed.

> > struct vfio_pci_device *kvm_vfio_device_init(const char *bdf)
> > {
> > 	struct vfio_pci_device *device;
> > 	struct iommu *iommu;
> > 
> > 	iommu = iommu_init(default_iommu_mode);
> > 
> > 	device = vfio_pci_device_init(bdf, iommu);
> > 	if (!device) {
> > 		vfio_pci_device_print_drivers(stderr);
> > 		TEST_FAIL("No driver found for BDF '%s'", bdf);
> > 	}
> > 	return device;
> > }
> > 
> > Because as is, this requires way too much a priori knowledge and magic for
> > upstream.  The user shouldn't have to dig through code just to understand what
> > devices are supported.
> >
> > Ideally, VFIO selftests would also provide a script, utility, and/or helper to
> > identify BDFs for devices it has drivers for.  In my experience, binding a device
> > to VFIO is trivial.  Finding the device in the first place is more annoying.
> 
> Agree this would be nice to have. Is this a blocker for this series?

Yes.  As I mentioned to Josh off-list, I'm not willing to accept a "we pinky-swear
we'll make this stuff user friendly".  It's not that I don't trust you and Josh
(and others), it's that for me, this level of user-friendly behavior isn't nice
to have, it's mandatory for these tests to be usable upstream.

Internally we can squeak by with barebones, unhelpful tests, because the run
commands are often hardcoded somewhere and there is piles of documentation elsewhere.
But for upstream, none of that holds true.

If it were weeks of effort, I would be more open to treating this as nice to have,
but AFAICT writing the code should be less than a days worth of work.

  reply	other threads:[~2026-04-01 19:07 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-31 19:40 [PATCH v2 00/14] KVM: selftests: Link with VFIO selftests lib and test device interrupts Josh Hilke
2026-03-31 19:40 ` [PATCH v2 01/14] KVM: selftests: Build and link sefltests/vfio/lib into KVM selftests Josh Hilke
2026-04-01 18:17   ` Sean Christopherson
2026-04-01 23:49     ` Josh Hilke
2026-03-31 19:40 ` [PATCH v2 02/14] KVM: selftests: Add helper functions for IRQ testing Josh Hilke
2026-04-01 18:26   ` Sean Christopherson
2026-04-01 23:54     ` Josh Hilke
2026-03-31 19:40 ` [PATCH v2 03/14] KVM: selftests: Add vfio_pci_irq_test Josh Hilke
2026-04-01 19:58   ` Sean Christopherson
2026-04-02  0:13     ` Josh Hilke
2026-04-02 17:52       ` Sean Christopherson
2026-03-31 19:40 ` [PATCH v2 04/14] KVM: selftests: Reproduce tests that rely on randomization Josh Hilke
2026-03-31 19:40 ` [PATCH v2 05/14] KVM: selftests: Add support for random host IRQ affinity Josh Hilke
2026-04-01 20:01   ` Sean Christopherson
2026-04-02  1:16     ` Josh Hilke
2026-03-31 19:40 ` [PATCH v2 06/14] KVM: selftests: Allow blocking vCPUs via HLT Josh Hilke
2026-04-01 20:03   ` Sean Christopherson
2026-03-31 19:40 ` [PATCH v2 07/14] KVM: selftests: Add support for physical device MSI triggers Josh Hilke
2026-04-01 20:24   ` Sean Christopherson
2026-04-02  3:23     ` Josh Hilke
2026-03-31 19:40 ` [PATCH v2 08/14] KVM: selftests: Add option to clear GSI routes Josh Hilke
2026-03-31 19:40 ` [PATCH v2 09/14] KVM: selftests: Make test IRQ count configurable Josh Hilke
2026-03-31 19:40 ` [PATCH v2 10/14] KVM: selftests: Add support for NMI delivery Josh Hilke
2026-04-01 20:29   ` Sean Christopherson
2026-04-02  5:27     ` Josh Hilke
2026-03-31 19:40 ` [PATCH v2 11/14] KVM: selftests: Add support for vCPU pinning Josh Hilke
2026-04-01 20:55   ` Sean Christopherson
2026-03-31 19:40 ` [PATCH v2 12/14] KVM: selftests: Support testing with multiple vCPUs Josh Hilke
2026-03-31 19:40 ` [PATCH v2 13/14] KVM: selftests: Add xAPIC mode support Josh Hilke
2026-03-31 19:40 ` [PATCH v2 14/14] KVM: selftests: Make vfio_pci_irq_test timeout configurable Josh Hilke
2026-04-01 21:00   ` Sean Christopherson
2026-04-01 18:17 ` [PATCH v2 00/14] KVM: selftests: Link with VFIO selftests lib and test device interrupts Sean Christopherson
2026-04-01 18:52   ` David Matlack
2026-04-01 19:07     ` Sean Christopherson [this message]
2026-04-01 20:12       ` David Matlack
2026-04-01 23:41         ` Josh Hilke
2026-04-01 23:58           ` David Matlack
2026-04-02  0:38             ` Josh Hilke
2026-04-02  1:49               ` Josh Hilke
2026-04-02 17:35                 ` Sean Christopherson
2026-04-02 17:56                   ` David Matlack
2026-04-02 18:07                     ` Josh Hilke

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=ac1s55_oLEUQx8Hs@google.com \
    --to=seanjc@google.com \
    --cc=alex@shazbot.org \
    --cc=dmatlack@google.com \
    --cc=jrhilke@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox