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

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: <20210117070304.GA1226161@unreal>
Date:   Sun, 17 Jan 2021 09:03:04 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Jakub Kicinski <kuba@...nel.org>, linux-pci@...r.kernel.org,
        linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
        Don Dutile <ddutile@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [PATCH mlx5-next v2 1/5] PCI: Add sysfs callback to allow MSI-X
 table size change of SR-IOV VFs

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@...nel.org> wrote:
> >
> > > From: Leon Romanovsky <leonro@...dia.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@...dia.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