Re: [PATCH] PCI: endpoint: pci-epf-test: Limit PCIe BAR size for fixed BARs

[Niklas Cassel <cassel@kernel.org>]

On Thu, Sep 04, 2025 at 11:40:15AM +0900, Damien Le Moal wrote:
> On 9/4/25 11:37 AM, Marek Vasut wrote:
> > Currently, the test allocates BAR sizes according to fixed table
> > bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 } . This
> > does not work with controllers which have fixed size BARs, like
> > Renesas R-Car V4H PCIe controller, which has BAR4 size limited
> > to 256 Bytes, which is much less than 131072 currently requested
> > by this test.
> > 
> > Adjust the test such, that in case a fixed size BAR is detected
> > on a controller, minimum of requested size and fixed size BAR
> > size is used during the test instead.
> > 
> > This helps with test failures reported as follows:
> > "
> > pci_epf_test pci_epf_test.0: requested BAR size is larger than fixed size
> > pci_epf_test pci_epf_test.0: Failed to allocate space for BAR4
> > "
> > 
> > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > ---
> > Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Damien Le Moal <dlemoal@kernel.org>
> > Cc: Frank Li <Frank.Li@nxp.com>
> > Cc: Kishon Vijay Abraham I <kishon@kernel.org>
> > Cc: Manivannan Sadhasivam <mani@kernel.org>
> > Cc: Niklas Cassel <cassel@kernel.org>
> > Cc: Wang Jiang <jiangwang@kylinos.cn>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-pci@vger.kernel.org
> > Cc: linux-renesas-soc@vger.kernel.org
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index e091193bd8a8a..d9c950d4c9a9e 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -1022,7 +1022,8 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
> >  	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> >  	enum pci_barno bar;
> >  	const struct pci_epc_features *epc_features = epf_test->epc_features;
> > -	size_t test_reg_size;
> > +	size_t test_reg_size, test_bar_size;
> > +	u64 bar_fixed_size;
> >  
> >  	test_reg_bar_size = ALIGN(sizeof(struct pci_epf_test_reg), 128);
> >  
> > @@ -1050,7 +1051,13 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
> >  		if (bar == test_reg_bar)
> >  			continue;
> >  
> > -		base = pci_epf_alloc_space(epf, bar_size[bar], bar,
> > +		test_bar_size = bar_size[bar];
> > +
> > +		bar_fixed_size = epc_features->bar[bar].fixed_size;
> > +		if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size)
> > +			test_bar_size = min(bar_size[bar], bar_fixed_size);
> 
> I think this can be simplified to:
> 
> 		if (epc_features->bar[bar].type == BAR_FIXED)
> 			test_bar_size = epc_features->bar[bar].fixed_size;
> 		else
> 			test_bar_size = bar_size[bar];

+1

> 
> because if the bar type is BAR_FIXED, then the size of the bar can only be its
> fixed size.

Correct, see:
f015b53d634a ("PCI: endpoint: Add size check for fixed size BARs in pci_epc_set_bar()")

Actually, Jerome Brunet was also using this weird Renesas R-Car V4H PCIe
controller where BAR4 is a really small fixed-size BAR.

(Even smaller than the iATU minimum alignment requirement for that same
controller.)

See:
793908d60b87 ("PCI: endpoint: Retain fixed-size BAR size as well as aligned size")

But he only appears to have used the vntb epf driver.

Jerome, I suppose that you never ran with the pci-epf-test driver?


pci_epf_alloc_space() works like this:
If the user requests a BAR size that is smaller than the fixed-size BAR,
it will allocate space matching the fixed-size.

As in most cases, having a BAR larger than needed by an EPF driver is
still acceptable.

However, if the user requests a size larger than the fixed-size BAR,
as in your case, we will return an error, as we cannot fulfill the
user's request.

I don't see any alternative other than your/Damien's proposal above.

Unfortunately, all EPF drivers would probably need this same change.


Kind regards,
Niklas