On Fri, Sep 20, 2024 at 02:30:00PM +0300, Ilpo Järvinen wrote:
> On Fri, 20 Sep 2024, Michał Winiarski wrote:
>
> > VF MMIO resource reservation, either created by system firmware and
> > inherited by Linux PCI subsystem or created by the subsystem itself,
> > contains enough space to fit the BAR of all SR-IOV Virtual Functions
> > that can potentially be created (total VFs supported by the device).
> > This can be leveraged when the device is exposing lower than optimal BAR
> > size as a default, allowing access to the entire resource when lower
> > number of VFs are created.
> > It is achieved by dynamically resizing the BAR to largest possible value
> > that allows to fit all newly created VFs within the original resource
> > boundary.
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > ---
> > drivers/pci/iov.c | 92 ++++++++++++++++++++++++++++++++++++++++++++-
> > drivers/pci/pci.h | 1 +
> > include/linux/pci.h | 3 ++
> > 3 files changed, 95 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index e8ccd2ae0f024..d88efbfa70e42 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -181,6 +181,86 @@ bool pci_iov_memory_decoding_enabled(struct pci_dev *dev)
> > return cmd & PCI_SRIOV_CTRL_MSE;
> > }
> >
> > +static void pci_iov_resource_do_extend(struct pci_dev *dev, int resno, u16 num_vfs)
> > +{
> > + resource_size_t size;
> > + int ret, old, i;
> > + u32 sizes;
> > +
> > + pci_config_pm_runtime_get(dev);
> > +
> > + if (pci_iov_memory_decoding_enabled(dev)) {
> > + ret = -EBUSY;
> > + goto err;
> > + }
> > +
> > + sizes = pci_rebar_get_possible_sizes(dev, resno);
> > + if (!sizes) {
> > + ret = -ENOTSUPP;
> > + goto err;
> > + }
> > +
> > + old = pci_rebar_get_current_size(dev, resno);
> > + if (old < 0) {
> > + ret = old;
> > + goto err;
> > + }
> > +
> > + while (sizes > 0) {
> > + i = __fls(sizes);
> > + size = pci_rebar_size_to_bytes(i);
> > + if (size * num_vfs <= pci_resource_len(dev, resno)) {
> > + if (i != old) {
> > + ret = pci_rebar_set_size(dev, resno, size);
> > + if (ret)
> > + goto err;
> > +
> > + pci_iov_resource_set_size(dev, resno, size);
> > + pci_iov_update_resource(dev, resno);
> > + }
> > + break;
> > + }
> > + sizes &= ~BIT(i);
> > + }
> > +
> > + pci_config_pm_runtime_put(dev);
> > +
> > + return;
> > +
> > +err:
> > + dev_WARN(&dev->dev, "Failed to extend %s: %d\n",
> > + pci_resource_name(dev, resno), ret);
>
> Why do you use dev_WARN()? (analoguous to WARN_ON() / friends).
>
> I suppose you'd want to use pci_warn() instead.
This should never happen - but fair point, with something like
panic_on_warn it would have more negative consequences. Let's downgrade
it to pci_warn().
>
> > + pci_config_pm_runtime_put(dev);
> > +}
> > +
> > +static void pci_iov_resource_do_restore(struct pci_dev *dev, int resno)
> > +{
> > + if (dev->sriov->rebar_extend[resno - PCI_IOV_RESOURCES])
> > + pci_iov_resource_do_extend(dev, resno, dev->sriov->total_VFs);
> > +}
> > +
> > +int pci_iov_resource_extend(struct pci_dev *dev, int resno, bool enable)
> > +{
> > + if (!pci_resource_is_iov(dev, resno)) {
> > + dev_WARN(&dev->dev, "%s is not an IOV resource\n",
> > + pci_resource_name(dev, resno));
>
> pci_warn() ?
And this one is programmer error. But same as above, let's use
pci_warn().
Thanks
-Michał
