On Thu, Aug 19, 2021 at 07:16:20PM +0300, Yishai Hadas wrote:
> On 8/19/2021 6:15 PM, Bjorn Helgaas wrote:
> > On Wed, Aug 18, 2021 at 06:16:03PM +0300, Yishai Hadas wrote:
> > > From: Max Gurtovoy <mgurtovoy@xxxxxxxxxx>
> > > /**
> > > * struct pci_device_id - PCI device ID structure
> > > * @vendor: Vendor ID to match (or PCI_ANY_ID)
> > > @@ -34,12 +38,14 @@ typedef unsigned long kernel_ulong_t;
> > > * Best practice is to use driver_data as an index
> > > * into a static list of equivalent device types,
> > > * instead of using it as a pointer.
> > > + * @override_only: Bitmap for override_only PCI drivers.
> > "Match only when dev->driver_override is this driver"?
>
> Just to be aligned here,
>
> This field will stay __u32 and may hold at the most 1 bit value set to
> represent the actual subsystem/driver.
The PCI core does not require "at most 1 bit is set."
Actually, I don't think even the file2alias code requires that. If
you set two bits, you can generate two aliases.
> This is required to later on set the correct prefix in the modules.alias
> file, and you just suggested to change the comment as of above, right ?
Yes, __u32 is fine and I'm only suggesting a comment change here.
> > As far as PCI core is concerned there's no need for this to be a
> > bitmap.
> >
> > I think this would make more sense if split into two patches. The
> > first would add override_only and change pci_match_device(). Then
> > there's no confusion about whether this is specific to VFIO.
>
> Splitting may end-up the first patch with a dead-code on below, as
> found_id->override_only will be always 0.
>
> If you still believe that this is better we can do it.
I think it's fine to add the functionality in one patch and use it in
the next if it makes the commit clearer. I wouldn't want to add
functionality that's not used at all in the series, but it's OK when
they're both posted together.
> if (found_id->override_only) {
> if (dev->driver_override)
> return found_id;
> } else
> return found_id;
>
> > The second can add PCI_ID_F_VFIO_DRIVER_OVERRIDE and make the
> > file2alias.c changes. Most of the commit log applies to this part.
