On Mon, Jan 25, 2021 at 10:50:53AM -0800, Alexander Duyck wrote:

On Mon, Jan 25, 2021 at 10:47 AM Leon Romanovsky [off-list ref] wrote:

quoted

On Sun, Jan 24, 2021 at 09:00:32PM +0200, Leon Romanovsky wrote:

quoted

On Sun, Jan 24, 2021 at 08:47:44AM -0800, Alexander Duyck wrote:

quoted

On Sun, Jan 24, 2021 at 5:11 AM Leon Romanovsky [off-list ref] wrote:

quoted

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>

quoted

+
+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.

I added them to make extension of this vfs_overlay interface more easy,
so we won't forget that current fields needs "msix_cap". Also I followed
same style as other attribute_group which has .is_visible.

quoted

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.

We already have, I don't think that we need to add extra links, it will
give nothing.

[leonro@vm ~]$ ls -l /sys/bus/pci/devices/0000\:01\:00.0/
....
drwxr-xr-x 2 root root        0 Jan 24 14:02 vfs_overlay
lrwxrwxrwx 1 root root        0 Jan 24 14:02 virtfn0 -> ../0000:01:00.1
lrwxrwxrwx 1 root root        0 Jan 24 14:02 virtfn1 -> ../0000:01:00.2
....

Alexander, are we clear here? Do you expect v5 without ".is_visible" from me?

Yeah, I am okay with the .is_visible being left around. It just seems
redundant is all.

Thanks a lot for your review,
I appreciate the effort and time you invested into it.

Thanks.

 -Alex