Re: [PATCH v4 2/7] PCI: endpoint: Add pci_epc_bar_size_to_rebar_cap()

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Feb 19, 2025 at 06:24:10PM +0100, Niklas Cassel wrote:
> On Tue, Feb 18, 2025 at 10:48:04AM -0600, Bjorn Helgaas wrote:
> > On Fri, Jan 31, 2025 at 07:29:51PM +0100, Niklas Cassel wrote:
> > > Add a helper function to convert a size to the representation used by the
> > > Resizable BAR Capability Register.
> > 
> > > +/**
> > > + * pci_epc_bar_size_to_rebar_cap() - convert a size to the representation used
> > > + *				     by the Resizable BAR Capability Register
> > > + * @size: the size to convert
> > > + * @cap: where to store the result
> > > + *
> > > + * Returns 0 on success and a negative error code in case of error.
> > > + */
> > > +int pci_epc_bar_size_to_rebar_cap(size_t size, u32 *cap)
> > > +{
> > > +	/*
> > > +	 * According to PCIe base spec, min size for a resizable BAR is 1 MB,
> > > +	 * thus disallow a requested BAR size smaller than 1 MB.
> > > +	 * Disallow a requested BAR size larger than 128 TB.
> > > +	 */
> > > +	if (size < SZ_1M || (u64)size > (SZ_128G * 1024))
> > > +		return -EINVAL;
> > > +
> > > +	*cap = ilog2(size) - ilog2(SZ_1M);
> > > +
> > > +	/* Sizes in REBAR_CAP start at BIT(4). */
> > > +	*cap = BIT(*cap + 4);
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_bar_size_to_rebar_cap);
> > 
> > This doesn't seem specific to EPC.  It looks basically similar to
> > pci_rebar_bytes_to_size().  Can we consolidate anything there?
> 
> This function is to convert a size to
> "Resizable BAR Capability Register", which includes one or more supported
> BAR sizes.
> 
> This register should only ever by written by a PCI endpoint function,
> so I think its current home in drivers/pci/endpoint/ is correct.
> 
> pci_rebar_bytes_to_size() is used to convert a size to
> "Resizable BAR Control Register", specifically the field
> "BAR Size".
> 
> This "BAR Size" field in the "Resizable BAR Control Register" can be
> written by the host side (or endpoint side), to select one of the
> supported bar sizes. So I think it makes sense for
> pci_rebar_bytes_to_size() to live in pci.h.

Thanks, I agree.

It looks like pci_epc_bar_size_to_rebar_cap() is only called once per
BAR.  Does that mean an endpoint driver can only set a single
supported size for a Resizable BAR in the Capability register?

Bjorn