[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1jplc54aoc.fsf@starbuckisacylon.baylibre.com>
Date: Fri, 05 Sep 2025 09:32:03 +0200
From: Jerome Brunet <jbrunet@...libre.com>
To: Niklas Cassel <cassel@...nel.org>
Cc: Damien Le Moal <dlemoal@...nel.org>, Marek Vasut
<marek.vasut+renesas@...lbox.org>, linux-pci@...r.kernel.org, Krzysztof
Wilczyński <kwilczynski@...nel.org>, Bjorn Helgaas
<bhelgaas@...gle.com>,
Frank Li <Frank.Li@....com>, Kishon Vijay Abraham I <kishon@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>, Wang Jiang
<jiangwang@...inos.cn>, linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH] PCI: endpoint: pci-epf-test: Limit PCIe BAR size for
fixed BARs
On Thu 04 Sep 2025 at 14:28, Niklas Cassel <cassel@...nel.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@...lbox.org>
>> > ---
>> > Cc: "Krzysztof Wilczyński" <kwilczynski@...nel.org>
>> > Cc: Bjorn Helgaas <bhelgaas@...gle.com>
>> > Cc: Damien Le Moal <dlemoal@...nel.org>
>> > Cc: Frank Li <Frank.Li@....com>
>> > Cc: Kishon Vijay Abraham I <kishon@...nel.org>
>> > Cc: Manivannan Sadhasivam <mani@...nel.org>
>> > Cc: Niklas Cassel <cassel@...nel.org>
>> > Cc: Wang Jiang <jiangwang@...inos.cn>
>> > Cc: linux-kernel@...r.kernel.org
>> > Cc: linux-pci@...r.kernel.org
>> > Cc: linux-renesas-soc@...r.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
Powered by blists - more mailing lists