On Thu, Apr 03, 2025 at 01:20:58PM +0300, Ilpo Järvinen wrote:
> On Wed, 2 Apr 2025, Michał Winiarski wrote:
>
> > Drivers could leverage the fact that the VF BAR MMIO reservation is
> > created for total number of VFs supported by the device by resizing the
> > BAR to larger size when smaller number of VFs is enabled.
> >
> > Add a pci_iov_vf_bar_set_size() function to control the size and a
> > pci_iov_vf_bar_get_sizes() helper to get the VF BAR sizes that will
> > allow up to num_vfs to be successfully enabled with the current
> > underlying reservation size.
> >
> > Signed-off-by: Michał Winiarski <michal.winiar...@intel.com>
> > ---
> > drivers/pci/iov.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/pci.h | 6 ++++
> > 2 files changed, 75 insertions(+)
> >
> > diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> > index 2fafbd6a998f0..8a62049b56b41 100644
> > --- a/drivers/pci/iov.c
> > +++ b/drivers/pci/iov.c
> > @@ -1313,3 +1313,72 @@ int pci_sriov_configure_simple(struct pci_dev *dev,
> > int nr_virtfn)
> > return nr_virtfn;
> > }
> > EXPORT_SYMBOL_GPL(pci_sriov_configure_simple);
> > +
> > +/**
> > + * pci_iov_vf_bar_set_size - set a new size for a VF BAR
> > + * @dev: the PCI device
> > + * @resno: the resource number
> > + * @size: new size as defined in the spec (0=1MB, 31=128TB)
> > + *
> > + * Set the new size of a VF BAR that supports VF resizable BAR capability.
> > + * Unlike pci_resize_resource(), this does not cause the resource that
> > + * reserves the MMIO space (originally up to total_VFs) to be resized,
> > which
> > + * means that following calls to pci_enable_sriov() can fail if the
> > resources
> > + * no longer fit.
> > + *
> > + * Return: 0 on success, or negative on failure.
> > + */
> > +int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size)
> > +{
> > + int ret;
> > + u32 sizes;
Hi,
I thought this series was already ready to be merged, but now I just
went back through the review comments for v7 and realized that I missed
this email and never applied the changes you requested.
Sorry for the delay.
> Please reverse the order of these variables.
Ok.
>
> > +
> > + if (!pci_resource_is_iov(resno))
> > + return -EINVAL;
> > +
> > + if (pci_iov_is_memory_decoding_enabled(dev))
> > + return -EBUSY;
> > +
> > + sizes = pci_rebar_get_possible_sizes(dev, resno);
> > + if (!sizes)
> > + return -ENOTSUPP;
> > +
> > + if (!(sizes & BIT(size)))
>
> Add include for BIT().
>
> Although adding a helper like this would be helpful for multiple callers:
>
> @@ -3780,6 +3780,88 @@ u32 pci_rebar_get_possible_sizes(struct pci_dev *pdev,
> int bar)
> }
> EXPORT_SYMBOL(pci_rebar_get_possible_sizes);
>
> +/**
> + * pci_rebar_size_supported - check if size is supported for a resizable BAR
> + * @pdev: PCI device
> + * @bar: BAR to check
> + * @size: size as defined in the spec (0=1MB, 31=128TB)
> + *
> + * Return: %0 if @size is supported for @bar,
> + * %-EINVAL if @size is not supported,
> + * %-ENOTSUPP if @bar is not resizable.
> + */
> +int pci_rebar_size_supported(struct pci_dev *pdev, int bar, int size)
> +{
> + u64 sizes;
> +
> + sizes = pci_rebar_get_possible_sizes(pdev, bar);
> + if (!sizes)
> + return -ENOTSUPP;
> +
> + return BIT(size) & sizes ? 0 : -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(pci_rebar_size_supported);
>
> (Yes, I've some rebar helpers pending so this is extract from that.)
Added include for BIT(). I'll wait for your series with helpers :)
>
> > + return -EINVAL;
> > +
> > + ret = pci_rebar_set_size(dev, resno, size);
> > + if (ret)
> > + return ret;
> > +
> > + pci_iov_resource_set_size(dev, resno, pci_rebar_size_to_bytes(size));
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_set_size);
> > +
> > +/**
> > + * pci_iov_vf_bar_get_sizes - get VF BAR sizes allowing to create up to
> > num_vfs
> > + * @dev: the PCI device
> > + * @resno: the resource number
> > + * @num_vfs: number of VFs
> > + *
> > + * Get the sizes of a VF resizable BAR that can accommodate @num_vfs within
> > + * the currently assigned size of the resource @resno.
> > + *
> > + * Return: A bitmask of sizes in format defined in the spec (bit 0=1MB,
> > + * bit 31=128TB).
> > + */
> > +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs)
> > +{
> > + resource_size_t vf_len = pci_resource_len(dev, resno);
> > + u32 sizes;
> > +
> > + if (!num_vfs)
> > + return 0;
> > +
> > + do_div(vf_len, num_vfs);
>
> Add include for do_div().
Ok.
>
> > + sizes = (roundup_pow_of_two(vf_len + 1) - 1) >> ilog2(SZ_1M);
>
> Doesn't resource_size() already do that + 1 so why is a second one needed
> here?
It's not about resource_size() doing + 1 for the resource length, it's
needed because roundup_pow_of_two() is not rounding up to the next power
of two if the input is already a power of two. So we need to add 1 to
ensure that we get the next size up to make the math with bitmap
representing BAR sizes work correctly.
>
> Add include for ilog() and SZ_*.
Ok.
Thanks,
-Michał
>
> > +
> > + return sizes & pci_rebar_get_possible_sizes(dev, resno);
> > +}
> > +EXPORT_SYMBOL_GPL(pci_iov_vf_bar_get_sizes);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 0e8e3fd77e967..c8708f3749757 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2389,6 +2389,8 @@ int pci_sriov_set_totalvfs(struct pci_dev *dev, u16
> > numvfs);
> > int pci_sriov_get_totalvfs(struct pci_dev *dev);
> > int pci_sriov_configure_simple(struct pci_dev *dev, int nr_virtfn);
> > resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
> > +int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno, int size);
> > +u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno, int num_vfs);
> > void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool probe);
> >
> > /* Arch may override these (weak) */
> > @@ -2441,6 +2443,10 @@ static inline int pci_sriov_get_totalvfs(struct
> > pci_dev *dev)
> > #define pci_sriov_configure_simple NULL
> > static inline resource_size_t pci_iov_resource_size(struct pci_dev *dev,
> > int resno)
> > { return 0; }
> > +static inline int pci_iov_vf_bar_set_size(struct pci_dev *dev, int resno,
> > int size)
> > +{ return -ENODEV; }
> > +static inline u32 pci_iov_vf_bar_get_sizes(struct pci_dev *dev, int resno,
> > int num_vfs)
> > +{ return 0; }
> > static inline void pci_vf_drivers_autoprobe(struct pci_dev *dev, bool
> > probe) { }
> > #endif
> >
> >
>
> --
> i.
