* [PATCH 0/2] PCI: endpoint: set prefetchable bit @ 2024-02-29 10:48 Niklas Cassel 2024-02-29 10:48 ` [PATCH 1/2] PCI: endpoint: Move .only_64bit check to core Niklas Cassel 2024-02-29 10:48 ` [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs Niklas Cassel 0 siblings, 2 replies; 6+ messages in thread From: Niklas Cassel @ 2024-02-29 10:48 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Kishon Vijay Abraham I, Bjorn Helgaas Cc: Shradha Todi, Niklas Cassel, linux-pci Shradha Todi wanted to add prefetchable BAR support by adding more things to epc_features: https://lore.kernel.org/linux-pci/20240228134448.56372-1-shradha.t@samsung.com/T/#t This series sets the prefetchable bit for all backing memory that the EPF core allocates for a 64-bit BAR. So far, only compile tested. Please test. Kind regards, Niklas Niklas Cassel (2): PCI: endpoint: Move .only_64bit check to core PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs drivers/pci/endpoint/functions/pci-epf-test.c | 14 -------------- drivers/pci/endpoint/pci-epf-core.c | 8 +++++--- 2 files changed, 5 insertions(+), 17 deletions(-) -- 2.44.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] PCI: endpoint: Move .only_64bit check to core 2024-02-29 10:48 [PATCH 0/2] PCI: endpoint: set prefetchable bit Niklas Cassel @ 2024-02-29 10:48 ` Niklas Cassel 2024-02-29 10:48 ` [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs Niklas Cassel 1 sibling, 0 replies; 6+ messages in thread From: Niklas Cassel @ 2024-02-29 10:48 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Kishon Vijay Abraham I, Bjorn Helgaas Cc: Shradha Todi, Niklas Cassel, linux-pci Move the .only_64bit check to pci-epf-core, such that this check does not need to be duplicated in all endpoint function drivers. Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/pci/endpoint/functions/pci-epf-test.c | 14 -------------- drivers/pci/endpoint/pci-epf-core.c | 7 ++++--- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index cd4ffb39dcdc..a7f0b5ebc1e8 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -870,19 +870,6 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf) return 0; } -static void pci_epf_configure_bar(struct pci_epf *epf, - const struct pci_epc_features *epc_features) -{ - struct pci_epf_bar *epf_bar; - int i; - - for (i = 0; i < PCI_STD_NUM_BARS; i++) { - epf_bar = &epf->bar[i]; - if (epc_features->bar[i].only_64bit) - epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; - } -} - static int pci_epf_test_bind(struct pci_epf *epf) { int ret; @@ -907,7 +894,6 @@ static int pci_epf_test_bind(struct pci_epf *epf) test_reg_bar = pci_epc_get_first_free_bar(epc_features); if (test_reg_bar < 0) return -EINVAL; - pci_epf_configure_bar(epf, epc_features); epf_test->test_reg_bar = test_reg_bar; epf_test->epc_features = epc_features; diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index 0a28a0b0911b..e7dbbeb1f0de 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -304,9 +304,10 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar, epf_bar[bar].addr = space; epf_bar[bar].size = size; epf_bar[bar].barno = bar; - epf_bar[bar].flags |= upper_32_bits(size) ? - PCI_BASE_ADDRESS_MEM_TYPE_64 : - PCI_BASE_ADDRESS_MEM_TYPE_32; + 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; return space; } -- 2.44.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs 2024-02-29 10:48 [PATCH 0/2] PCI: endpoint: set prefetchable bit Niklas Cassel 2024-02-29 10:48 ` [PATCH 1/2] PCI: endpoint: Move .only_64bit check to core Niklas Cassel @ 2024-02-29 10:48 ` Niklas Cassel 2024-02-29 18:37 ` Niklas Cassel 1 sibling, 1 reply; 6+ messages in thread From: Niklas Cassel @ 2024-02-29 10:48 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Kishon Vijay Abraham I, Bjorn Helgaas Cc: Shradha Todi, Niklas Cassel, linux-pci 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@kernel.org> --- 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; -- 2.44.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs 2024-02-29 10:48 ` [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs Niklas Cassel @ 2024-02-29 18:37 ` Niklas Cassel 2024-03-01 10:37 ` Shradha Todi 0 siblings, 1 reply; 6+ messages in thread From: Niklas Cassel @ 2024-02-29 18:37 UTC (permalink / raw) To: Lorenzo Pieralisi, Krzysztof Wilczyński, Manivannan Sadhasivam, Kishon Vijay Abraham I, Bjorn Helgaas Cc: Shradha Todi, linux-pci 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@kernel.org> > --- > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs 2024-02-29 18:37 ` Niklas Cassel @ 2024-03-01 10:37 ` Shradha Todi 2024-03-01 14:15 ` Niklas Cassel 0 siblings, 1 reply; 6+ messages in thread From: Shradha Todi @ 2024-03-01 10:37 UTC (permalink / raw) To: 'Niklas Cassel', 'Lorenzo Pieralisi', 'Krzysztof Wilczyński', 'Manivannan Sadhasivam', 'Kishon Vijay Abraham I', 'Bjorn Helgaas' Cc: linux-pci > -----Original Message----- > From: Niklas Cassel <cassel@kernel.org> > Sent: 01 March 2024 00:08 > To: Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof Wilczyński > <kw@linux.com>; Manivannan Sadhasivam > <manivannan.sadhasivam@linaro.org>; Kishon Vijay Abraham I > <kishon@kernel.org>; Bjorn Helgaas <bhelgaas@google.com> > Cc: Shradha Todi <shradha.t@samsung.com>; linux-pci@vger.kernel.org > Subject: Re: [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for > 64-bit BARs > > 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@kernel.org> > > --- > > 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 Hi Niklas, A similar fix should go for pci_epf_test_set_bar() as well, since pci_epc_set_bar() may change the flags. diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index cd4ffb39dcdc..3bf4340e9d7b 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -741,6 +741,8 @@ static int pci_epf_test_set_bar(struct pci_epf *epf) if (bar == test_reg_bar) return ret; } + + add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1; } return 0; Do you want to add it as part of the same patch? Or should I post another patch for this? ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs 2024-03-01 10:37 ` Shradha Todi @ 2024-03-01 14:15 ` Niklas Cassel 0 siblings, 0 replies; 6+ messages in thread From: Niklas Cassel @ 2024-03-01 14:15 UTC (permalink / raw) To: Shradha Todi Cc: 'Lorenzo Pieralisi', 'Krzysztof Wilczyński', 'Manivannan Sadhasivam', 'Kishon Vijay Abraham I', 'Bjorn Helgaas', linux-pci Hello Shradha, On Fri, Mar 01, 2024 at 04:07:21PM +0530, Shradha Todi wrote: > > > > -----Original Message----- > > From: Niklas Cassel <cassel@kernel.org> > > Sent: 01 March 2024 00:08 > > To: Lorenzo Pieralisi <lpieralisi@kernel.org>; Krzysztof Wilczyński > > <kw@linux.com>; Manivannan Sadhasivam > > <manivannan.sadhasivam@linaro.org>; Kishon Vijay Abraham I > > <kishon@kernel.org>; Bjorn Helgaas <bhelgaas@google.com> > > Cc: Shradha Todi <shradha.t@samsung.com>; linux-pci@vger.kernel.org > > Subject: Re: [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory > for > > 64-bit BARs > > > > 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@kernel.org> > > > --- > > > 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 > > Hi Niklas, > > A similar fix should go for pci_epf_test_set_bar() as well, since > pci_epc_set_bar() > may change the flags. I do see that this text is here: https://github.com/torvalds/linux/blob/v6.8-rc6/drivers/pci/endpoint/functions/pci-epf-test.c#L725-L729 Looking at the orginal author of that text: https://github.com/torvalds/linux/commit/fca83058753456528bef62579ae2b50799d7a473 (turned out that it was me :p) I see that the reason was that epc->ops->set_bar() for Cadence EP controller could set PCI_BASE_ADDRESS_MEM_TYPE_64 even if only 32 bits BAR was requested. Looking at the Cadence set_bar(): https://github.com/torvalds/linux/blob/master/drivers/pci/controller/cadence/pcie-cadence-ep.c#L107-L108 They do set a 64-bit even if a user only requested a 32-bit BAR, if the requested BAR size was > 4G. However, we have this check: https://github.com/torvalds/linux/commit/f25b5fae29d4e5fe6ae17d3f898a959d72519b6a So that would never happen. pci_epc_set_bar() would error out before calling the lower level device driver with such a requested configuration. Now when we have epc_features, if a BAR requires 64-bits, they should set that as a hardware requirement in epc_features. A epc->ops->set_bar() should never be allowed to override whas is being requested IMO. (It could give an error if it can't fulfill the requested configuration though.) So what I think should be done: 1) Clean up the Cadence driver, since that code is dead code. 2) Remove the "pci_epc_set_bar() might have given us a 64-bit BAR", since it is not true. Note that pci_epf_test_set_bar() still needs to do: https://github.com/torvalds/linux/blob/master/drivers/pci/endpoint/functions/pci-epf-test.c#L730 But that is because pci_epf_alloc_space() could have set PCI_BASE_ADDRESS_MEM_TYPE_64. But I don't think that we need a comment for this, just like how the "add =" in pci_epf_test_alloc_space() does not have a comment: https://github.com/torvalds/linux/blob/v6.8-rc6/drivers/pci/endpoint/functions/pci-epf-test.c#L860 Kind regards, Niklas ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-01 14:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-29 10:48 [PATCH 0/2] PCI: endpoint: set prefetchable bit Niklas Cassel 2024-02-29 10:48 ` [PATCH 1/2] PCI: endpoint: Move .only_64bit check to core Niklas Cassel 2024-02-29 10:48 ` [PATCH 2/2] PCI: endpoint: Set prefetch when allocating memory for 64-bit BARs Niklas Cassel 2024-02-29 18:37 ` Niklas Cassel 2024-03-01 10:37 ` Shradha Todi 2024-03-01 14:15 ` Niklas Cassel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).