Re: [PATCH mlx5-next v2 1/5] PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs

[Leon Romanovsky <leon@kernel.org>]

On Sat, Jan 16, 2021 at 10:23:31AM +0200, Leon Romanovsky wrote:
> On Thu, Jan 14, 2021 at 05:05:43PM -0700, Alex Williamson wrote:
> > On Thu, 14 Jan 2021 12:31:36 +0200
> > Leon Romanovsky <leon@kernel.org> wrote:
> >
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > >
> > > Extend PCI sysfs interface with a new callback that allows configure
> > > the number of MSI-X vectors for specific SR-IO VF. This is needed
> > > to optimize the performance of newly bound devices by allocating
> > > the number of vectors based on the administrator knowledge of targeted VM.
> > >
> > > This function is applicable for SR-IOV VF because such devices allocate
> > > their MSI-X table before they will run on the VMs and HW can't guess the
> > > right number of vectors, so the HW allocates them statically and equally.
> > >
> > > The newly added /sys/bus/pci/devices/.../sriov_vf_msix_count file will be seen
> > > for the VFs and it is writable as long as a driver is not bounded to the VF.
> > >
> > > The values accepted are:
> > >  * > 0 - this will be number reported by the VF's MSI-X capability
> > >  * < 0 - not valid
> > >  * = 0 - will reset to the device default value
> > >
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-pci | 20 +++++++++
> > >  drivers/pci/iov.c                       | 58 +++++++++++++++++++++++++
> > >  drivers/pci/msi.c                       | 47 ++++++++++++++++++++
> > >  drivers/pci/pci-sysfs.c                 |  1 +
> > >  drivers/pci/pci.h                       |  2 +
> > >  include/linux/pci.h                     |  3 ++
> > >  6 files changed, 131 insertions(+)

<...>

> > > +static umode_t sriov_vf_attrs_are_visible(struct kobject *kobj,
> > > +					  struct attribute *a, int n)
> > > +{
> > > +	struct device *dev = kobj_to_dev(kobj);
> > > +
> > > +	if (dev_is_pf(dev))
> > > +		return 0;
> >
> > Wouldn't it be cleaner to also hide this on VFs where
> > pci_msix_vec_count() returns an error or where the PF driver doesn't
> > implement .sriov_set_msix_vec_count()?  IOW, expose it only where it
> > could actually work.
>
> I wasn't sure about the policy in PCI/core, but sure will change.

I ended adding checks of msix_cap, but can't check .sriov_set_msix_vec_count.
The latter will require to hold device_lock on PF that can disappear later, it
is too racy.

Thanks