Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 00/10] Add definition for the number of standard PCI BARs
@ 2019-08-16  9:24 Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Andrew Murray, Jose Abreu, kvm, linux-fbdev, netdev, x86,
	linux-s390

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 +-
 include/linux/pci.h                              |  2 +-
 include/uapi/linux/pci_regs.h                    |  1 +
 17 files changed, 51 insertions(+), 47 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 01/10] PCI: Add define for the number of standard PCI BARs
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS Denis Efremov
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Andrew Murray, Jose Abreu, kvm, linux-fbdev, netdev, x86,
	linux-s390

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
@ 2019-08-16  9:24 ` Denis Efremov
  2019-08-16 10:45   ` Sebastian Ott
  2019-08-16  9:24 ` [PATCH v2 03/10] x86/PCI: " Denis Efremov
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, linux-s390,
	linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 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 +++---
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index a2399eff84ca..3a06c264ea53 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -2,9 +2,6 @@
 #ifndef __ASM_S390_PCI_H
 #define __ASM_S390_PCI_H
 
-/* must be set before including pci_clp.h */
-#define PCI_BAR_COUNT	6
-
 #include <linux/pci.h>
 #include <linux/mutex.h>
 #include <linux/iommu.h>
@@ -138,7 +135,7 @@ struct zpci_dev {
 
 	char res_name[16];
 	bool mio_capable;
-	struct zpci_bar_struct bars[PCI_BAR_COUNT];
+	struct zpci_bar_struct bars[PCI_STD_NUM_BARS];
 
 	u64		start_dma;	/* Start of available DMA addresses */
 	u64		end_dma;	/* End of available DMA addresses */
diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
index 50359172cc48..bd2cb4ea7d93 100644
--- a/arch/s390/include/asm/pci_clp.h
+++ b/arch/s390/include/asm/pci_clp.h
@@ -77,7 +77,7 @@ struct mio_info {
 	struct {
 		u64 wb;
 		u64 wt;
-	} addr[PCI_BAR_COUNT];
+	} addr[PCI_STD_NUM_BARS];
 	u32 reserved[6];
 } __packed;
 
@@ -98,9 +98,9 @@ struct clp_rsp_query_pci {
 	u16 util_str_avail	:  1;	/* utility string available? */
 	u16 pfgid		:  8;	/* pci function group id */
 	u32 fid;			/* pci function id */
-	u8 bar_size[PCI_BAR_COUNT];
+	u8 bar_size[PCI_STD_NUM_BARS];
 	u16 pchid;
-	__le32 bar[PCI_BAR_COUNT];
+	__le32 bar[PCI_STD_NUM_BARS];
 	u8 pfip[CLP_PFIP_NR_SEGMENTS];	/* pci function internal path */
 	u32			: 16;
 	u8 fmb_len;
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index b0e3b9a0e488..aca372c8e34f 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
 static DEFINE_SPINLOCK(zpci_domain_lock);
 
 #define ZPCI_IOMAP_ENTRIES						\
-	min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),	\
+	min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2),	\
 	    ZPCI_IOMAP_MAX_ENTRIES)
 
 static DEFINE_SPINLOCK(zpci_iomap_lock);
@@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar,
 void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar,
 			      unsigned long offset, unsigned long max)
 {
-	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
+	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
 		return NULL;
 
 	if (static_branch_likely(&have_mio))
@@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar,
 void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar,
 				 unsigned long offset, unsigned long max)
 {
-	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
+	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
 		return NULL;
 
 	if (static_branch_likely(&have_mio))
@@ -416,7 +416,7 @@ static void zpci_map_resources(struct pci_dev *pdev)
 	resource_size_t len;
 	int i;
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		len = pci_resource_len(pdev, i);
 		if (!len)
 			continue;
@@ -451,7 +451,7 @@ static void zpci_unmap_resources(struct pci_dev *pdev)
 	if (zpci_use_mio(zdev))
 		return;
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		len = pci_resource_len(pdev, i);
 		if (!len)
 			continue;
@@ -514,7 +514,7 @@ static int zpci_setup_bus_resources(struct zpci_dev *zdev,
 	snprintf(zdev->res_name, sizeof(zdev->res_name),
 		 "PCI Bus %04x:%02x", zdev->domain, ZPCI_BUS_NR);
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (!zdev->bars[i].size)
 			continue;
 		entry = zpci_alloc_iomap(zdev);
@@ -551,7 +551,7 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev)
 {
 	int i;
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (!zdev->bars[i].size || !zdev->bars[i].res)
 			continue;
 
@@ -573,7 +573,7 @@ int pcibios_add_device(struct pci_dev *pdev)
 	pdev->dev.dma_ops = &s390_pci_dma_ops;
 	zpci_map_resources(pdev);
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		res = &pdev->resource[i];
 		if (res->parent || !res->flags)
 			continue;
diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index 9bdff4defef1..8b729b5f2972 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -145,7 +145,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev,
 {
 	int i;
 
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		zdev->bars[i].val = le32_to_cpu(response->bar[i]);
 		zdev->bars[i].size = response->bar_size[i];
 	}
@@ -164,8 +164,8 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev,
 		       sizeof(zdev->util_str));
 	}
 	zdev->mio_capable = response->mio_addr_avail;
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
-		if (!(response->mio.valid & (1 << (PCI_BAR_COUNT - i - 1))))
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+		if (!(response->mio.valid & (1 << (PCI_STD_NUM_BARS - i - 1))))
 			continue;
 
 		zdev->bars[i].mio_wb = (void __iomem *) response->mio.addr[i].wb;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 03/10] x86/PCI: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:32   ` Thomas Gleixner
  2019-08-16  9:24 ` [PATCH v2 04/10] stmmac: pci: " Denis Efremov
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Denis Efremov, H. Peter Anvin, x86, linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 arch/x86/pci/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 9acab6ac28f5..1e59df041456 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -135,7 +135,7 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev)
 		* resource so the kernel doesn't attempt to assign
 		* it later on in pci_assign_unassigned_resources
 		*/
-		for (bar = 0; bar <= PCI_STD_RESOURCE_END; bar++) {
+		for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 			bar_r = &dev->resource[bar];
 			if (bar_r->start == 0 && bar_r->end != 0) {
 				bar_r->flags = 0;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 04/10] stmmac: pci: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (2 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 03/10] x86/PCI: " Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 05/10] net: dwc-xlgmac: " Denis Efremov
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Giuseppe Cavallaro, Alexandre Torgue, netdev,
	linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 86f9c07a38cf..cfe496cdd78b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -258,7 +258,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	}
 
 	/* Get the base address of device */
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
 		ret = pcim_iomap_regions(pdev, BIT(i), pci_name(pdev));
@@ -296,7 +296,7 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
 
 	stmmac_dvr_remove(&pdev->dev);
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
 		pcim_iounmap_regions(pdev, BIT(i));
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 05/10] net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (3 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 04/10] stmmac: pci: " Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 06/10] rapidio/tsi721: " Denis Efremov
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Denis Efremov, Jose Abreu, netdev, linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
index 386bafe74c3f..fa8604d7b797 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
@@ -34,7 +34,7 @@ static int xlgmac_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 		return ret;
 	}
 
-	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pcidev, i) == 0)
 			continue;
 		ret = pcim_iomap_regions(pcidev, BIT(i), XLGMAC_DRV_NAME);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 06/10] rapidio/tsi721: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (4 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 05/10] net: dwc-xlgmac: " Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 07/10] efifb: " Denis Efremov
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Matt Porter, Alexandre Bounine, linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/rapidio/devices/tsi721.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rapidio/devices/tsi721.c b/drivers/rapidio/devices/tsi721.c
index 125a173bed45..4dd31dd9feea 100644
--- a/drivers/rapidio/devices/tsi721.c
+++ b/drivers/rapidio/devices/tsi721.c
@@ -2755,7 +2755,7 @@ static int tsi721_probe(struct pci_dev *pdev,
 	{
 		int i;
 
-		for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 			tsi_debug(INIT, &pdev->dev, "res%d %pR",
 				  i, &pdev->resource[i]);
 		}
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 07/10] efifb: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (5 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 06/10] rapidio/tsi721: " Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 08/10] vfio_pci: " Denis Efremov
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Peter Jones, Bartlomiej Zolnierkiewicz,
	linux-fbdev, linux-pci, linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 08/10] vfio_pci: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (6 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 07/10] efifb: " Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16 16:23   ` Alex Williamson
  2019-08-16  9:24 ` [PATCH v2 09/10] PCI: hv: Use PCI_STD_NUM_BARS Denis Efremov
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Cornelia Huck, Alex Williamson, kvm, linux-pci,
	linux-kernel

Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
'i <= PCI_STD_RESOURCE_END'.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/vfio/pci/vfio_pci.c         | 11 +++++++----
 drivers/vfio/pci/vfio_pci_config.c  | 10 ++++++----
 drivers/vfio/pci/vfio_pci_private.h |  4 ++--
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 703948c9fbe1..cb7d220d3246 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -110,13 +110,15 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev)
 static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev)
 {
 	struct resource *res;
-	int bar;
+	int i;
 	struct vfio_pci_dummy_resource *dummy_res;
 
 	INIT_LIST_HEAD(&vdev->dummy_resources_list);
 
-	for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
-		res = vdev->pdev->resource + bar;
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+		int bar = i + PCI_STD_RESOURCES;
+
+		res = &vdev->pdev->resource[bar];
 
 		if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP))
 			goto no_mmap;
@@ -399,7 +401,8 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
 
 	vfio_config_free(vdev);
 
-	for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
+		bar = i + PCI_STD_RESOURCES;
 		if (!vdev->barmap[bar])
 			continue;
 		pci_iounmap(pdev, vdev->barmap[bar]);
diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index f0891bd8444c..df8772395219 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -455,16 +455,18 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
 
 	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
 
-	for (i = PCI_STD_RESOURCES; i <= PCI_STD_RESOURCE_END; i++, bar++) {
-		if (!pci_resource_start(pdev, i)) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++, bar++) {
+		int ibar = i + PCI_STD_RESOURCES;
+
+		if (!pci_resource_start(pdev, ibar)) {
 			*bar = 0; /* Unmapped by host = unimplemented to user */
 			continue;
 		}
 
-		mask = ~(pci_resource_len(pdev, i) - 1);
+		mask = ~(pci_resource_len(pdev, ibar) - 1);
 
 		*bar &= cpu_to_le32((u32)mask);
-		*bar |= vfio_generate_bar_flags(pdev, i);
+		*bar |= vfio_generate_bar_flags(pdev, ibar);
 
 		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
 			bar++;
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index ee6ee91718a4..8a2c7607d513 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -86,8 +86,8 @@ struct vfio_pci_reflck {
 
 struct vfio_pci_device {
 	struct pci_dev		*pdev;
-	void __iomem		*barmap[PCI_STD_RESOURCE_END + 1];
-	bool			bar_mmap_supported[PCI_STD_RESOURCE_END + 1];
+	void __iomem		*barmap[PCI_STD_NUM_BARS];
+	bool			bar_mmap_supported[PCI_STD_NUM_BARS];
 	u8			*pci_config_map;
 	u8			*vconfig;
 	struct perm_bits	*msi_perm;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 09/10] PCI: hv: Use PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (7 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 08/10] vfio_pci: " Denis Efremov
@ 2019-08-16  9:24 ` Denis Efremov
  2019-08-16  9:24 ` [PATCH v2 10/10] PCI: " Denis Efremov
  2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Denis Efremov, linux-kernel, linux-pci, Andrew Murray

Replace the magic constant with define PCI_STD_NUM_BARS.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/controller/pci-hyperv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 40b625458afa..1665c23b649f 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -307,7 +307,7 @@ struct pci_bus_relations {
 struct pci_q_res_req_response {
 	struct vmpacket_descriptor hdr;
 	s32 status;			/* negative values are failures */
-	u32 probed_bar[6];
+	u32 probed_bar[PCI_STD_NUM_BARS];
 } __packed;
 
 struct pci_set_power {
@@ -503,7 +503,7 @@ struct hv_pci_dev {
 	 * What would be observed if one wrote 0xFFFFFFFF to a BAR and then
 	 * read it back, for each of the BAR offsets within config space.
 	 */
-	u32 probed_bar[6];
+	u32 probed_bar[PCI_STD_NUM_BARS];
 };
 
 struct hv_pci_compl {
@@ -1327,7 +1327,7 @@ static void survey_child_resources(struct hv_pcibus_device *hbus)
 	 * so it's sufficient to just add them up without tracking alignment.
 	 */
 	list_for_each_entry(hpdev, &hbus->children, list_entry) {
-		for (i = 0; i < 6; i++) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 			if (hpdev->probed_bar[i] & PCI_BASE_ADDRESS_SPACE_IO)
 				dev_err(&hbus->hdev->device,
 					"There's an I/O BAR in this list!\n");
@@ -1401,7 +1401,7 @@ static void prepopulate_bars(struct hv_pcibus_device *hbus)
 	/* Pick addresses for the BARs. */
 	do {
 		list_for_each_entry(hpdev, &hbus->children, list_entry) {
-			for (i = 0; i < 6; i++) {
+			for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 				bar_val = hpdev->probed_bar[i];
 				if (bar_val == 0)
 					continue;
@@ -1558,7 +1558,7 @@ static void q_resource_requirements(void *context, struct pci_response *resp,
 			"query resource requirements failed: %x\n",
 			resp->status);
 	} else {
-		for (i = 0; i < 6; i++) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 			completion->hpdev->probed_bar[i] =
 				q_res_req->probed_bar[i];
 		}
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 10/10] PCI: Use PCI_STD_NUM_BARS
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (8 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 09/10] PCI: hv: Use PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-16  9:24 ` " Denis Efremov
  2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
  10 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-16  9:24 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Denis Efremov, linux-kernel, linux-pci, Andrew Murray

Replace the magic constant with define PCI_STD_NUM_BARS.

Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/pci.c    | 11 ++++++-----
 drivers/pci/quirks.c |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1b27b5af3d55..a9005c9eee6c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3768,7 +3768,7 @@ void pci_release_selected_regions(struct pci_dev *pdev, int bars)
 {
 	int i;
 
-	for (i = 0; i < 6; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		if (bars & (1 << i))
 			pci_release_region(pdev, i);
 }
@@ -3779,7 +3779,7 @@ static int __pci_request_selected_regions(struct pci_dev *pdev, int bars,
 {
 	int i;
 
-	for (i = 0; i < 6; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		if (bars & (1 << i))
 			if (__pci_request_region(pdev, i, res_name, excl))
 				goto err_out;
@@ -3827,7 +3827,7 @@ EXPORT_SYMBOL(pci_request_selected_regions_exclusive);
 
 void pci_release_regions(struct pci_dev *pdev)
 {
-	pci_release_selected_regions(pdev, (1 << 6) - 1);
+	pci_release_selected_regions(pdev, (1 << PCI_STD_NUM_BARS) - 1);
 }
 EXPORT_SYMBOL(pci_release_regions);
 
@@ -3846,7 +3846,8 @@ EXPORT_SYMBOL(pci_release_regions);
  */
 int pci_request_regions(struct pci_dev *pdev, const char *res_name)
 {
-	return pci_request_selected_regions(pdev, ((1 << 6) - 1), res_name);
+	return pci_request_selected_regions(pdev,
+			((1 << PCI_STD_NUM_BARS) - 1), res_name);
 }
 EXPORT_SYMBOL(pci_request_regions);
 
@@ -3868,7 +3869,7 @@ EXPORT_SYMBOL(pci_request_regions);
 int pci_request_regions_exclusive(struct pci_dev *pdev, const char *res_name)
 {
 	return pci_request_selected_regions_exclusive(pdev,
-					((1 << 6) - 1), res_name);
+				((1 << PCI_STD_NUM_BARS) - 1), res_name);
 }
 EXPORT_SYMBOL(pci_request_regions_exclusive);
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 02bdf3a0231e..51caa61e6112 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1810,7 +1810,7 @@ static void quirk_alder_ioapic(struct pci_dev *pdev)
 	 * The next five BARs all seem to be rubbish, so just clean
 	 * them out.
 	 */
-	for (i = 1; i < 6; i++)
+	for (i = 1; i < PCI_STD_NUM_BARS; i++)
 		memset(&pdev->resource[i], 0, sizeof(pdev->resource[i]));
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_EESSC,	quirk_alder_ioapic);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 03/10] x86/PCI: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 ` [PATCH v2 03/10] x86/PCI: " Denis Efremov
@ 2019-08-16  9:32   ` Thomas Gleixner
  2019-08-16 13:31     ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2019-08-16  9:32 UTC (permalink / raw)
  To: Denis Efremov; +Cc: Bjorn Helgaas, H. Peter Anvin, x86, linux-pci, linux-kernel

On Fri, 16 Aug 2019, Denis Efremov wrote:

> Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
> 'i <= PCI_STD_RESOURCE_END'.

Please describe the WHY not the WHAT. I can see the WHAT from the patch
itself, but I can't figure out WHY.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 ` [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-16 10:45   ` Sebastian Ott
  0 siblings, 0 replies; 18+ messages in thread
From: Sebastian Ott @ 2019-08-16 10:45 UTC (permalink / raw)
  To: Denis Efremov
  Cc: Bjorn Helgaas, Gerald Schaefer, linux-s390, linux-pci, linux-kernel

On Fri, 16 Aug 2019, Denis Efremov wrote:
> Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
> 'i <= PCI_STD_RESOURCE_END'.
> 
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  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 +++---
>  4 files changed, 15 insertions(+), 18 deletions(-)
> 

Acked-by: Sebastian Ott <sebott@linux.ibm.com>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] Add definition for the number of standard PCI BARs
  2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
                   ` (9 preceding siblings ...)
  2019-08-16  9:24 ` [PATCH v2 10/10] PCI: " Denis Efremov
@ 2019-08-16 10:51 ` Andrew Murray
  2019-08-16 13:35   ` Bjorn Helgaas
  2019-09-05 19:02   ` Denis Efremov
  10 siblings, 2 replies; 18+ messages in thread
From: Andrew Murray @ 2019-08-16 10:51 UTC (permalink / raw)
  To: Denis Efremov
  Cc: Bjorn Helgaas, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Jose Abreu, kvm, linux-fbdev, netdev, x86, linux-s390

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 +-
>  include/linux/pci.h                              |  2 +-
>  include/uapi/linux/pci_regs.h                    |  1 +
>  17 files changed, 51 insertions(+), 47 deletions(-)

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 03/10] x86/PCI: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:32   ` Thomas Gleixner
@ 2019-08-16 13:31     ` Bjorn Helgaas
  0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-16 13:31 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Denis Efremov, H. Peter Anvin, x86, linux-pci, linux-kernel

On Fri, Aug 16, 2019 at 11:32:41AM +0200, Thomas Gleixner wrote:
> On Fri, 16 Aug 2019, Denis Efremov wrote:
> 
> > Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
> > 'i <= PCI_STD_RESOURCE_END'.
> 
> Please describe the WHY not the WHAT. I can see the WHAT from the patch
> itself, but I can't figure out WHY.

Good point; the WHY is to use idiomatic C style and avoid
the fencepost error of using "i < PCI_STD_RESOURCE_END"
when "i <= PCI_STD_RESOURCE_END" is required, e.g.,
2f686f1d9bee ("PCI: Correct PCI_STD_RESOURCE_END usage")

Denis, can you include something along those lines in the next
version?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] Add definition for the number of standard PCI BARs
  2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
@ 2019-08-16 13:35   ` Bjorn Helgaas
  2019-09-05 19:02   ` Denis Efremov
  1 sibling, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-16 13:35 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Denis Efremov, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Jose Abreu, kvm, linux-fbdev, netdev, x86, linux-s390

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:
> > 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 +-
> >  include/linux/pci.h                              |  2 +-
> >  include/uapi/linux/pci_regs.h                    |  1 +
> >  17 files changed, 51 insertions(+), 47 deletions(-)
> 
> 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++) { <-----------

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 08/10] vfio_pci: Loop using PCI_STD_NUM_BARS
  2019-08-16  9:24 ` [PATCH v2 08/10] vfio_pci: " Denis Efremov
@ 2019-08-16 16:23   ` Alex Williamson
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Williamson @ 2019-08-16 16:23 UTC (permalink / raw)
  To: Denis Efremov; +Cc: Bjorn Helgaas, Cornelia Huck, kvm, linux-pci, linux-kernel

On Fri, 16 Aug 2019 12:24:35 +0300
Denis Efremov <efremov@linux.com> wrote:

> Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of
> 'i <= PCI_STD_RESOURCE_END'.
> 
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/vfio/pci/vfio_pci.c         | 11 +++++++----
>  drivers/vfio/pci/vfio_pci_config.c  | 10 ++++++----
>  drivers/vfio/pci/vfio_pci_private.h |  4 ++--
>  3 files changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 703948c9fbe1..cb7d220d3246 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -110,13 +110,15 @@ static inline bool vfio_pci_is_vga(struct pci_dev *pdev)
>  static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev)
>  {
>  	struct resource *res;
> -	int bar;
> +	int i;
>  	struct vfio_pci_dummy_resource *dummy_res;
>  
>  	INIT_LIST_HEAD(&vdev->dummy_resources_list);
>  
> -	for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
> -		res = vdev->pdev->resource + bar;
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +		int bar = i + PCI_STD_RESOURCES;
> +
> +		res = &vdev->pdev->resource[bar];
>  
>  		if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP))
>  			goto no_mmap;
> @@ -399,7 +401,8 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
>  
>  	vfio_config_free(vdev);
>  
> -	for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +		bar = i + PCI_STD_RESOURCES;
>  		if (!vdev->barmap[bar])
>  			continue;
>  		pci_iounmap(pdev, vdev->barmap[bar]);
> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index f0891bd8444c..df8772395219 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -455,16 +455,18 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
>  
>  	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
>  
> -	for (i = PCI_STD_RESOURCES; i <= PCI_STD_RESOURCE_END; i++, bar++) {
> -		if (!pci_resource_start(pdev, i)) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++, bar++) {
> +		int ibar = i + PCI_STD_RESOURCES;
> +
> +		if (!pci_resource_start(pdev, ibar)) {
>  			*bar = 0; /* Unmapped by host = unimplemented to user */
>  			continue;
>  		}
>  
> -		mask = ~(pci_resource_len(pdev, i) - 1);
> +		mask = ~(pci_resource_len(pdev, ibar) - 1);
>  
>  		*bar &= cpu_to_le32((u32)mask);
> -		*bar |= vfio_generate_bar_flags(pdev, i);
> +		*bar |= vfio_generate_bar_flags(pdev, ibar);
>  
>  		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
>  			bar++;

It might be a bit cleaner to rename the 'bar' variable to 'vbar', then
we have 'bar' available to use as the BAR number.  It seems more
consistent with other uses.  Otherwise the logic looks fine.  Thanks,

Alex

> diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
> index ee6ee91718a4..8a2c7607d513 100644
> --- a/drivers/vfio/pci/vfio_pci_private.h
> +++ b/drivers/vfio/pci/vfio_pci_private.h
> @@ -86,8 +86,8 @@ struct vfio_pci_reflck {
>  
>  struct vfio_pci_device {
>  	struct pci_dev		*pdev;
> -	void __iomem		*barmap[PCI_STD_RESOURCE_END + 1];
> -	bool			bar_mmap_supported[PCI_STD_RESOURCE_END + 1];
> +	void __iomem		*barmap[PCI_STD_NUM_BARS];
> +	bool			bar_mmap_supported[PCI_STD_NUM_BARS];
>  	u8			*pci_config_map;
>  	u8			*vconfig;
>  	struct perm_bits	*msi_perm;


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] Add definition for the number of standard PCI BARs
  2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
  2019-08-16 13:35   ` Bjorn Helgaas
@ 2019-09-05 19:02   ` Denis Efremov
  1 sibling, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-09-05 19:02 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Bjorn Helgaas, linux-kernel, linux-pci, Sebastian Ott,
	Gerald Schaefer, H. Peter Anvin, Giuseppe Cavallaro,
	Alexandre Torgue, Matt Porter, Alexandre Bounine, Peter Jones,
	Bartlomiej Zolnierkiewicz, Cornelia Huck, Alex Williamson,
	Jose Abreu, kvm, linux-fbdev, netdev, x86, linux-s390



On 16.08.2019 13:51, Andrew Murray wrote:
> 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 +-
>>  include/linux/pci.h                              |  2 +-
>>  include/uapi/linux/pci_regs.h                    |  1 +
>>  17 files changed, 51 insertions(+), 47 deletions(-)
> 
> 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.
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-16  9:24 [PATCH v2 00/10] Add definition for the number of standard PCI BARs Denis Efremov
2019-08-16  9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
2019-08-16  9:24 ` [PATCH v2 02/10] s390/pci: Loop using PCI_STD_NUM_BARS Denis Efremov
2019-08-16 10:45   ` Sebastian Ott
2019-08-16  9:24 ` [PATCH v2 03/10] x86/PCI: " Denis Efremov
2019-08-16  9:32   ` Thomas Gleixner
2019-08-16 13:31     ` Bjorn Helgaas
2019-08-16  9:24 ` [PATCH v2 04/10] stmmac: pci: " Denis Efremov
2019-08-16  9:24 ` [PATCH v2 05/10] net: dwc-xlgmac: " Denis Efremov
2019-08-16  9:24 ` [PATCH v2 06/10] rapidio/tsi721: " Denis Efremov
2019-08-16  9:24 ` [PATCH v2 07/10] efifb: " Denis Efremov
2019-08-16  9:24 ` [PATCH v2 08/10] vfio_pci: " Denis Efremov
2019-08-16 16:23   ` Alex Williamson
2019-08-16  9:24 ` [PATCH v2 09/10] PCI: hv: Use PCI_STD_NUM_BARS Denis Efremov
2019-08-16  9:24 ` [PATCH v2 10/10] PCI: " Denis Efremov
2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
2019-08-16 13:35   ` Bjorn Helgaas
2019-09-05 19:02   ` Denis Efremov

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox