Add definition for the number of standard PCI BARs

Started by Denis Efremov on Aug 16, 2019 · 6 messages from 3 authors · Last reply Sep 5, 2019

Series

10 patches, 17 files

+51 −47

Subsystems (14)

Show 11 more

Maintainers (29)

Show 25 more
STALE2627d SUPERSEDED

Status: Superseded by v3 (posted 2019-09-27). [go to v3]

Revision v2 (3 revisions archived).

Revisions (3)
  1. v1
  2. v2 current [diff against previous]
  3. v3 [diff against previous] re-posted go to this post

open the first message on its own page

  1. [PATCH v2 01/10] PCI: Add define for the number of standard PCI BARs
    Revisions and related patches
    Revisions (3)
    1. v1
    2. v2 current [diff against previous]
    3. v3 [diff against previous]
  2. [PATCH v2 07/10] efifb: Loop using PCI_STD_NUM_BARS
    Revisions and related patches
    Revisions (3)
    1. v1
    2. v2 current [diff against previous]
    3. v3 [diff against previous]

Denis Efremov

Message details and related content
Open message details and related content
Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END, but this is error-prone because it requires
"i <= PCI_STD_RESOURCE_END" rather than something like
"i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
way PCI_SRIOV_NUM_BARS is used. There is already the definition
PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.

Changes in v2:
  - Reverse checks in pci_iomap_range,pci_iomap_wc_range.
  - Refactor loops in vfio_pci to keep PCI_STD_RESOURCES.
  - Add 2 new patches to replace the magic constant with new define.
  - Split net patch in v1 to separate stmmac and dwc-xlgmac patches.

Denis Efremov (10):
  PCI: Add define for the number of standard PCI BARs
  s390/pci: Loop using PCI_STD_NUM_BARS
  x86/PCI: Loop using PCI_STD_NUM_BARS
  stmmac: pci: Loop using PCI_STD_NUM_BARS
  net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  efifb: Loop using PCI_STD_NUM_BARS
  vfio_pci: Loop using PCI_STD_NUM_BARS
  PCI: hv: Use PCI_STD_NUM_BARS
  PCI: Use PCI_STD_NUM_BARS

 arch/s390/include/asm/pci.h                      |  5 +----
 arch/s390/include/asm/pci_clp.h                  |  6 +++---
 arch/s390/pci/pci.c                              | 16 ++++++++--------
 arch/s390/pci/pci_clp.c                          |  6 +++---
 arch/x86/pci/common.c                            |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
 drivers/pci/controller/pci-hyperv.c              | 10 +++++-----
 drivers/pci/pci.c                                | 11 ++++++-----
 drivers/pci/quirks.c                             |  4 ++--
 drivers/rapidio/devices/tsi721.c                 |  2 +-
 drivers/vfio/pci/vfio_pci.c                      | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c               | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h              |  4 ++--
 drivers/video/fbdev/efifb.c                      |  2 +-

6 more lines

[PATCH v2 01/10] PCI: Add define for the number of standard PCI BARs

Denis Efremov replied to Denis Efremov

Message details and related content
Open message details and related content
Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END. However, it requires the "unusual" loop condition
"i <= PCI_STD_RESOURCE_END" rather than something more standard like
"i < PCI_STD_NUM_BARS".

This patch adds the definition PCI_STD_NUM_BARS which is equivalent to
"PCI_STD_RESOURCE_END + 1" and updates loop conditions to use it.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/quirks.c          | 2 +-
 include/linux/pci.h           | 2 +-
 include/uapi/linux/pci_regs.h | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 208aacf39329..02bdf3a0231e 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -475,7 +475,7 @@ static void quirk_extend_bar_to_page(struct pci_dev *dev)
 {
 	int i;
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		struct resource *r = &dev->resource[i];
 
 		if (r->flags & IORESOURCE_MEM && resource_size(r) < PAGE_SIZE) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e700d9f9f28..7b9590d5dc2d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -76,7 +76,7 @@ enum pci_mmap_state {
 enum {
 	/* #0-5: standard PCI resources */
 	PCI_STD_RESOURCES,
-	PCI_STD_RESOURCE_END = 5,
+	PCI_STD_RESOURCE_END = PCI_STD_RESOURCES + PCI_STD_NUM_BARS - 1,
 
 	/* #6: expansion ROM resource */
 	PCI_ROM_RESOURCE,
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index f28e562d7ca8..68b571d491eb 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -34,6 +34,7 @@
  * of which the first 64 bytes are standardized as follows:
  */
 #define PCI_STD_HEADER_SIZEOF	64
+#define PCI_STD_NUM_BARS	6	/* Number of standard BARs */
 #define PCI_VENDOR_ID		0x00	/* 16 bits */
 #define PCI_DEVICE_ID		0x02	/* 16 bits */
 #define PCI_COMMAND		0x04	/* 16 bits */
-- 
2.21.0

[PATCH v2 07/10] efifb: Loop using PCI_STD_NUM_BARS

Denis Efremov replied to Denis Efremov

Message details and related content
Open message details and related content
Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Acked-by: Bartlomiej Zolnierkiewicz <redacted>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/video/fbdev/efifb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 04a22663b4fb..6c72b825e92a 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -668,7 +668,7 @@ static void efifb_fixup_resources(struct pci_dev *dev)
 	if (!base)
 		return;
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		struct resource *res = &dev->resource[i];
 
 		if (!(res->flags & IORESOURCE_MEM))
-- 
2.21.0

Andrew Murray replied to Denis Efremov

Message details and related content
Open message details and related content
On Fri, Aug 16, 2019 at 12:24:27PM +0300, Denis Efremov wrote:
Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END, but this is error-prone because it requires
"i <= PCI_STD_RESOURCE_END" rather than something like
"i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
way PCI_SRIOV_NUM_BARS is used. There is already the definition
PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.

Changes in v2:
  - Reverse checks in pci_iomap_range,pci_iomap_wc_range.
  - Refactor loops in vfio_pci to keep PCI_STD_RESOURCES.
  - Add 2 new patches to replace the magic constant with new define.
  - Split net patch in v1 to separate stmmac and dwc-xlgmac patches.

Denis Efremov (10):
  PCI: Add define for the number of standard PCI BARs
  s390/pci: Loop using PCI_STD_NUM_BARS
  x86/PCI: Loop using PCI_STD_NUM_BARS
  stmmac: pci: Loop using PCI_STD_NUM_BARS
  net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  efifb: Loop using PCI_STD_NUM_BARS
  vfio_pci: Loop using PCI_STD_NUM_BARS
  PCI: hv: Use PCI_STD_NUM_BARS
  PCI: Use PCI_STD_NUM_BARS

 arch/s390/include/asm/pci.h                      |  5 +----
 arch/s390/include/asm/pci_clp.h                  |  6 +++---
 arch/s390/pci/pci.c                              | 16 ++++++++--------
 arch/s390/pci/pci_clp.c                          |  6 +++---
 arch/x86/pci/common.c                            |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
 drivers/pci/controller/pci-hyperv.c              | 10 +++++-----
 drivers/pci/pci.c                                | 11 ++++++-----
 drivers/pci/quirks.c                             |  4 ++--
 drivers/rapidio/devices/tsi721.c                 |  2 +-
 drivers/vfio/pci/vfio_pci.c                      | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c               | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h              |  4 ++--
 drivers/video/fbdev/efifb.c                      |  2 +-

3 more quoted lines

I've come across a few more places where this change can be made. There
may be multiple instances in the same file, but only the first is shown
below:

drivers/misc/pci_endpoint_test.c:       for (bar = BAR_0; bar <= BAR_5; bar++) {
drivers/net/ethernet/intel/e1000/e1000_main.c:          for (i = BAR_1; i <= BAR_5; i++) {
drivers/net/ethernet/intel/ixgb/ixgb_main.c:    for (i = BAR_1; i <= BAR_5; i++) {
drivers/pci/controller/dwc/pci-dra7xx.c:        for (bar = BAR_0; bar <= BAR_5; bar++)
drivers/pci/controller/dwc/pci-layerscape-ep.c: for (bar = BAR_0; bar <= BAR_5; bar++)
drivers/pci/controller/dwc/pcie-artpec6.c:      for (bar = BAR_0; bar <= BAR_5; bar++)
drivers/pci/controller/dwc/pcie-designware-plat.c:      for (bar = BAR_0; bar <= BAR_5; bar++)
drivers/pci/endpoint/functions/pci-epf-test.c:  for (bar = BAR_0; bar <= BAR_5; bar++) {
include/linux/pci-epc.h:        u64     bar_fixed_size[BAR_5 + 1];
drivers/scsi/pm8001/pm8001_hwi.c:       for (bar = 0; bar < 6; bar++) {
drivers/scsi/pm8001/pm8001_init.c:      for (bar = 0; bar < 6; bar++) {
drivers/ata/sata_nv.c:  for (bar = 0; bar < 6; bar++)
drivers/video/fbdev/core/fbmem.c:       for (idx = 0, bar = 0; bar < PCI_ROM_RESOURCE; bar++) {
drivers/staging/gasket/gasket_core.c:   for (i = 0; i < GASKET_NUM_BARS; i++) {
drivers/tty/serial/8250/8250_pci.c:     for (i = 0; i < PCI_NUM_BAR_RESOURCES; i++) { <-----------

It looks like BARs are often iterated with PCI_NUM_BAR_RESOURCES, there
are a load of these too found with:

git grep PCI_ROM_RESOURCE | grep "< "

I'm happy to share patches if preferred.

Thanks,

Andrew Murray

-- 
2.21.0

Bjorn Helgaas replied to Andrew Murray

Message details and related content
Open message details and related content
On Fri, Aug 16, 2019 at 11:51:28AM +0100, Andrew Murray wrote:
On Fri, Aug 16, 2019 at 12:24:27PM +0300, Denis Efremov wrote:
quoted
Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END, but this is error-prone because it requires
"i <= PCI_STD_RESOURCE_END" rather than something like
"i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
way PCI_SRIOV_NUM_BARS is used. There is already the definition
PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.

Changes in v2:
  - Reverse checks in pci_iomap_range,pci_iomap_wc_range.
  - Refactor loops in vfio_pci to keep PCI_STD_RESOURCES.
  - Add 2 new patches to replace the magic constant with new define.
  - Split net patch in v1 to separate stmmac and dwc-xlgmac patches.

Denis Efremov (10):
  PCI: Add define for the number of standard PCI BARs
  s390/pci: Loop using PCI_STD_NUM_BARS
  x86/PCI: Loop using PCI_STD_NUM_BARS
  stmmac: pci: Loop using PCI_STD_NUM_BARS
  net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  efifb: Loop using PCI_STD_NUM_BARS
  vfio_pci: Loop using PCI_STD_NUM_BARS
  PCI: hv: Use PCI_STD_NUM_BARS
  PCI: Use PCI_STD_NUM_BARS

 arch/s390/include/asm/pci.h                      |  5 +----
 arch/s390/include/asm/pci_clp.h                  |  6 +++---
 arch/s390/pci/pci.c                              | 16 ++++++++--------
 arch/s390/pci/pci_clp.c                          |  6 +++---
 arch/x86/pci/common.c                            |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
 drivers/pci/controller/pci-hyperv.c              | 10 +++++-----
 drivers/pci/pci.c                                | 11 ++++++-----
 drivers/pci/quirks.c                             |  4 ++--
 drivers/rapidio/devices/tsi721.c                 |  2 +-
 drivers/vfio/pci/vfio_pci.c                      | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c               | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h              |  4 ++--

24 more quoted lines

Thanks, I agree, these look like good candidates as well.

It looks like BARs are often iterated with PCI_NUM_BAR_RESOURCES, there
are a load of these too found with:

git grep PCI_ROM_RESOURCE | grep "< "
Good point, those are slightly questionable and I'd change those too.

Bjorn

Denis Efremov replied to Andrew Murray

Message details and related content
Open message details and related content

On 16.08.2019 13:51, Andrew Murray wrote:
On Fri, Aug 16, 2019 at 12:24:27PM +0300, Denis Efremov wrote:
quoted
Code that iterates over all standard PCI BARs typically uses
PCI_STD_RESOURCE_END, but this is error-prone because it requires
"i <= PCI_STD_RESOURCE_END" rather than something like
"i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
way PCI_SRIOV_NUM_BARS is used. There is already the definition
PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.

Changes in v2:
  - Reverse checks in pci_iomap_range,pci_iomap_wc_range.
  - Refactor loops in vfio_pci to keep PCI_STD_RESOURCES.
  - Add 2 new patches to replace the magic constant with new define.
  - Split net patch in v1 to separate stmmac and dwc-xlgmac patches.

Denis Efremov (10):
  PCI: Add define for the number of standard PCI BARs
  s390/pci: Loop using PCI_STD_NUM_BARS
  x86/PCI: Loop using PCI_STD_NUM_BARS
  stmmac: pci: Loop using PCI_STD_NUM_BARS
  net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  efifb: Loop using PCI_STD_NUM_BARS
  vfio_pci: Loop using PCI_STD_NUM_BARS
  PCI: hv: Use PCI_STD_NUM_BARS
  PCI: Use PCI_STD_NUM_BARS

 arch/s390/include/asm/pci.h                      |  5 +----
 arch/s390/include/asm/pci_clp.h                  |  6 +++---
 arch/s390/pci/pci.c                              | 16 ++++++++--------
 arch/s390/pci/pci_clp.c                          |  6 +++---
 arch/x86/pci/common.c                            |  2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
 drivers/pci/controller/pci-hyperv.c              | 10 +++++-----
 drivers/pci/pci.c                                | 11 ++++++-----
 drivers/pci/quirks.c                             |  4 ++--
 drivers/rapidio/devices/tsi721.c                 |  2 +-
 drivers/vfio/pci/vfio_pci.c                      | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c               | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h              |  4 ++--

32 more quoted lines

I'm not sure about lib/devres.c
265:#define PCIM_IOMAP_MAX      PCI_ROM_RESOURCE
268:    void __iomem *table[PCIM_IOMAP_MAX];
277:    for (i = 0; i < PCIM_IOMAP_MAX; i++)
324:    BUG_ON(bar >= PCIM_IOMAP_MAX);
352:    for (i = 0; i < PCIM_IOMAP_MAX; i++)
455:    for (i = 0; i < PCIM_IOMAP_MAX; i++) {

Is it worth changing?
#define PCIM_IOMAP_MAX  PCI_STD_NUM_BARS

Thanks,
Denis
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help