On Wed, 2 Apr 2025, Michał Winiarski wrote:
> On Wed, Mar 26, 2025 at 05:22:50PM +0200, Ilpo Järvinen wrote:
> > On Thu, 20 Mar 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>
> > > +/**
> > > + * 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 be accommodated within
> > > the
> > > + * resource that reserves the MMIO space if num_vfs are enabled.
> >
> > I'd rephrase to:
> >
> > Get the sizes of a VF resizable BAR that can be accommodate @num_vfs
> > within the currently assigned size of the resource @resno.
>
> Ok.
I have small grammar error in that:
"can be accomodate" -> "can accomodate"
> > > + * 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 size;
> > > + u32 sizes;
> > > + int i;
> > > +
> > > + sizes = pci_rebar_get_possible_sizes(dev, resno);
> > > + if (!sizes)
> > > + return 0;
> > > +
> > > + while (sizes > 0) {
> > > + i = __fls(sizes);
> > > + size = pci_rebar_size_to_bytes(i);
> > > +
> > > + if (size * num_vfs <= pci_resource_len(dev, resno))
> > > + break;
> > > +
> > > + sizes &= ~BIT(i);
> > > + }
> >
> > Couldn't this be handled without a loop:
> >
> > bar_sizes = (round_up(pci_resource_len(dev, resno) / num_vfs) - 1) >>
> > ilog2(SZ_1M);
> >
> > sizes &= bar_sizes;
> >
> > (Just to given an idea, I wrote this into the email so it might contain
> > some off-by-one errors or like).
>
> I think the division will need to be wrapped with something like do_div
> (because IIUC, we have 32bit architectures where resource_size_t is
> u64).
>
> But yeah, we can drop the loop, turning it into something like this:
>
> vf_len = pci_resource_len(dev, resno);
> do_div(vf_len, num_vfs);
> sizes = (roundup_pow_of_two(vf_len + 1) - 1) >> ilog2(SZ_1M);
Yes, good point, 64-bit division is required.
--
i.
