On Fri, Mar 28, 2025 at 03:53:43PM +0100, Jerome Brunet wrote:
> When allocating the shared ctrl/spad space, epf_ntb_config_spad_bar_alloc()
> should not try to handle the size quirks for the underlying BAR, whether it
> is fixed size or alignment. This is already handled by
> pci_epf_alloc_space().
>
> Also, when handling the alignment, this allocate more space than necessary.
> For example, with a spad size of 1024B and a ctrl size of 308B, the space
> necessary is 1332B. If the alignment is 1MB,
> epf_ntb_config_spad_bar_alloc() tries to allocate 2MB where 1MB would have
> been more than enough.
>
> Just drop all the handling of the BAR size quirks and let
> pci_epf_alloc_space() handle that.
>
> Signed-off-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> ---
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 24 ++----------------------
> 1 file changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 874cb097b093ae645bbc4bf3c9d28ca812d7689d..c20a60fcb99e6e16716dd78ab59ebf7cf074b2a6 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -408,11 +408,9 @@ static void epf_ntb_config_spad_bar_free(struct epf_ntb *ntb)
> */
> static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
> {
> - size_t align;
> enum pci_barno barno;
> struct epf_ntb_ctrl *ctrl;
> u32 spad_size, ctrl_size;
> - u64 size;
> struct pci_epf *epf = ntb->epf;
> struct device *dev = &epf->dev;
> u32 spad_count;
> @@ -422,31 +420,13 @@ static int epf_ntb_config_spad_bar_alloc(struct epf_ntb *ntb)
> epf->func_no,
> epf->vfunc_no);
> barno = ntb->epf_ntb_bar[BAR_CONFIG];
> - size = epc_features->bar[barno].fixed_size;
> - align = epc_features->align;
> -
> - if ((!IS_ALIGNED(size, align)))
> - return -EINVAL;
> -
> spad_count = ntb->spad_count;
>
> ctrl_size = sizeof(struct epf_ntb_ctrl);
I think keep ctrl_size at least align to 4 bytes. keep align 2^n is more
safe to keep spad area start at align possition.
ctrl_size = roundup_pow_of_two(sizeof(struct epf_ntb_ctrl));
Frank
> spad_size = 2 * spad_count * sizeof(u32);
>
> - if (!align) {
> - ctrl_size = roundup_pow_of_two(ctrl_size);
> - spad_size = roundup_pow_of_two(spad_size);
> - } else {
> - ctrl_size = ALIGN(ctrl_size, align);
> - spad_size = ALIGN(spad_size, align);
> - }
> -
> - if (!size)
> - size = ctrl_size + spad_size;
> - else if (size < ctrl_size + spad_size)
> - return -EINVAL;
> -
> - base = pci_epf_alloc_space(epf, size, barno, epc_features, 0);
> + base = pci_epf_alloc_space(epf, ctrl_size + spad_size,
> + barno, epc_features, 0);
> if (!base) {
> dev_err(dev, "Config/Status/SPAD alloc region fail\n");
> return -ENOMEM;
>
> --
> 2.47.2
>
