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

[Alexander Duyck <alexander.duyck@gmail.com>]

On Sun, Jan 24, 2021 at 5:11 AM 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.
>
> 1) The newly added /sys/bus/pci/devices/.../vfs_overlay/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
>
> 2) In order to make management easy, provide new read-only sysfs file that
> returns a total number of possible to configure MSI-X vectors.
>
> cat /sys/bus/pci/devices/.../vfs_overlay/sriov_vf_total_msix
>   = 0 - feature is not supported
>   > 0 - total number of MSI-X vectors to consume by the VFs
>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |  32 +++++
>  drivers/pci/iov.c                       | 180 ++++++++++++++++++++++++
>  drivers/pci/msi.c                       |  47 +++++++
>  drivers/pci/pci.h                       |   4 +
>  include/linux/pci.h                     |  10 ++
>  5 files changed, 273 insertions(+)
>

<snip>

> +
> +static umode_t sriov_pf_attrs_are_visible(struct kobject *kobj,
> +                                         struct attribute *a, int n)
> +{
> +       struct device *dev = kobj_to_dev(kobj);
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +
> +       if (!pdev->msix_cap || !dev_is_pf(dev))
> +               return 0;
> +
> +       return a->mode;
> +}
> +
> +static umode_t sriov_vf_attrs_are_visible(struct kobject *kobj,
> +                                         struct attribute *a, int n)
> +{
> +       struct device *dev = kobj_to_dev(kobj);
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +
> +       if (!pdev->msix_cap || dev_is_pf(dev))
> +               return 0;
> +
> +       return a->mode;
> +}
> +

Given the changes I don't see why we need to add the "visible"
functions. We are only registering this from the PF if there is a need
to make use of the interfaces, correct? If so we can just assume that
the interfaces should always be visible if they are requested.

Also you may want to look at placing a link to the VF folders in the
PF folder, although I suppose there are already links from the PF PCI
device to the VF PCI devices so maybe that isn't necessary. It just
takes a few extra steps to navigate between the two.