On Thu, Feb 29, 2024 at 11:48:59AM +0100, Niklas Cassel wrote:
> From the PCIe 6.0 base spec:
> "Generally only 64-bit BARs are good candidates, since only Legacy
> Endpoints are permitted to set the Prefetchable bit in 32-bit BARs,
> and most scalable platforms map all 32-bit Memory BARs into
> non-prefetchable Memory Space regardless of the Prefetchable bit value."
>
> "For a PCI Express Endpoint, 64-bit addressing must be supported for all
> BARs that have the Prefetchable bit Set. 32-bit addressing is permitted
> for all BARs that do not have the Prefetchable bit Set."
>
> "Any device that has a range that behaves like normal memory should mark
> the range as prefetchable. A linear frame buffer in a graphics device is
> an example of a range that should be marked prefetchable."
>
> The PCIe spec tells us that we should have the prefetchable bit set for
> 64-bit BARs backed by "normal memory". The backing memory that we allocate
> for a 64-bit BAR using pci_epf_alloc_space() (which calls
> dma_alloc_coherent()) is obviously "normal memory".
>
> Thus, set the prefetchable bit when allocating backing memory for a 64-bit
> BAR.
>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/pci/endpoint/pci-epf-core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index e7dbbeb1f0de..10264d662abf 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -305,7 +305,8 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> epf_bar[bar].size = size;
> epf_bar[bar].barno = bar;
> if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
> - epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> + epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64 |
> + PCI_BASE_ADDRESS_MEM_PREFETCH;
> else
> epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
This should probably be:
if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
else
epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
if (epf_bar[bar].flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_PREFETCH;
so that we set PREFETCH even for a EPF driver that had PCI_BASE_ADDRESS_MEM_TYPE_64
set in flags even before calling pci_epf_alloc_space. Will fix in V2.
I also found a bug in the existing code.
If pci_epf_alloc_space() allocated a 64-bit BAR because of bits in size,
then the increment in pci_epf_test_alloc_space() was incorrect.
(I guess no one uses BARs > 4GB).
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -865,6 +865,12 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
dev_err(dev, "Failed to allocate space for BAR%d\n",
bar);
epf_test->reg[bar] = base;
+
+ /*
+ * pci_epf_alloc_space() might have given us a 64-bit BAR,
+ * even if we only requested a 32-bit BAR.
+ */
+ add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
Will send a separate fix with the above.
Kind regards,
Niklas
