linux-kernel - Re: [PATCH 1/4] vfio: selftests: Add support for passing vf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJHc60y0KA00KmcW09ft-O_oUwp=5Wr9nBpF8RtYytrQ2tFYaw@mail.gmail.com>
Date: Fri, 7 Nov 2025 08:16:01 +0530
From: Raghavendra Rao Ananta <rananta@...gle.com>
To: David Matlack <dmatlack@...gle.com>
Cc: Alex Williamson <alex@...zbot.org>, Alex Williamson <alex.williamson@...hat.com>, 
	Josh Hilke <jrhilke@...gle.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] vfio: selftests: Add support for passing vf_token in
 device init

On Thu, Nov 6, 2025 at 10:47 PM David Matlack <dmatlack@...gle.com> wrote:
>
> On 2025-11-06 09:56 PM, Raghavendra Rao Ananta wrote:
> > On Thu, Nov 6, 2025 at 5:22 AM David Matlack <dmatlack@...gle.com> wrote:
> > > On 2025-11-04 12:35 AM, Raghavendra Rao Ananta wrote:
> > >
> > > > -struct vfio_pci_device *vfio_pci_device_init(const char *bdf, const char *iommu_mode);
> > > > +struct vfio_pci_device *vfio_pci_device_init(const char *bdf,
> > > > +                                           const char *iommu_mode,
> > > > +                                           const char *vf_token);
> > >
> > > Vipin is also looking at adding an optional parameter to
> > > vfio_pci_device_init():
> > > https://lore.kernel.org/kvm/20251018000713.677779-20-vipinsh@google.com/
> > >
> > > I am wondering if we should support an options struct for such
> > > parameters. e.g. something like this
> > >
> > > diff --git a/tools/testing/selftests/vfio/lib/include/vfio_util.h b/tools/testing/selftests/vfio/lib/include/vfio_util.h
> > > index b01068d98fda..cee837fe561c 100644
> > > --- a/tools/testing/selftests/vfio/lib/include/vfio_util.h
> > > +++ b/tools/testing/selftests/vfio/lib/include/vfio_util.h
> > > @@ -160,6 +160,10 @@ struct vfio_pci_driver {
> > >         int msi;
> > >  };
> > >
> > > +struct vfio_pci_device_options {
> > > +       const char *vf_token;
> > > +};
> > > +
> > >  struct vfio_pci_device {
> > >         int fd;
> > >
> > > @@ -202,9 +206,18 @@ const char *vfio_pci_get_cdev_path(const char *bdf);
> > >
> > >  extern const char *default_iommu_mode;
> > >
> > > -struct vfio_pci_device *vfio_pci_device_init(const char *bdf,
> > > -                                             const char *iommu_mode,
> > > -                                             const char *vf_token);
> > > +struct vfio_pci_device *__vfio_pci_device_init(const char *bdf,
> > > +                                              const char *iommu_mode,
> > > +                                              const struct vfio_pci_device_options *options);
> > > +
> > > +static inline struct vfio_pci_device *vfio_pci_device_init(const char *bdf,
> > > +                                                          const char *iommu_mode)
> > > +{
> > > +       static const struct vfio_pci_device_options default_options = {};
> > > +
> > > +       return __vfio_pci_device_init(bdf, iommu_mode, &default_options);
> > > +}
> > > +
> > >
> > > This will avoid you having to update every test.
> > >
> > > You can create a helper function in vfio_pci_sriov_uapi_test.c to call
> > > __vfio_pci_device_init() and abstract away the options stuff from your
> > > test.
> > >
> > I like the idea of an optional expandable struct. I'll implement this in v2.
>
> Just to make sure we're on the same page: I don't think you need to add
> this in v2 since you don't need to call vfio_pci_device_init(). For the
> inner functions that you want to call from your test, passing vf_token
> directly makes more sense IMO. vfio_pci_device_init() will just pass in
> NULL to those functions for vf_token by default.
>
> If/when we want to pass vf_token to vfio_pci_device_init() we can add
> the options struct.
>
Yes, of course. I'll avoid it until we have a need.

> > > No space necessary after a cast. This is another one checkpatch.pl will
> > > catch for you.
> > >
> > >   CHECK:SPACING: No space is necessary after a cast
> > >   #81: FILE: tools/testing/selftests/vfio/lib/vfio_pci_device.c:338:
> > >   +       char *arg = (char *) bdf;
> > >
> > Actually, I did run checkpatch.pl on the entire series as:
> > .$ ./scripts/checkpatch.pl *.patch
> >
> > I didn't see any of these warnings. Are there any other options to consider?
>
> Ah, I run with a few additional options. That's probably why we are
> seeing different output. Here's what I have in my .bashrc:
>
> function checkpatch() {
>         scripts/checkpatch.pl \
>                 -q \
>                 --strict \
>                 --codespell \
>                 --no-signoff \
>                 --show-types \
>                 --ignore gerrit_change_id,FILE_PATH_CHANGES,NOT_UNIFIED_DIFF \
>                 --no-summary \
>                 "$@"
> }
Thanks! I'll give this a shot!