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

[Jerome Brunet <jbrunet@baylibre.com>]

On Thu 04 Sep 2025 at 14:28, Niklas Cassel <cassel@kernel.org> wrote:

> 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

It's what pci_epf_alloc_space() does too. so it makes sense but it also
means the side must stay aligned.

If a rework is needed, maybe it would be better to get size from
pci_epf_alloc_space() instead of recomputing it ?

>
>> 
>> 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?
>

Indeed no. I've gone with with ntb-test driver on top or ntb-netdev

>
> 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

-- 
Jerome