* [PATCH 0/7] Add definition for the number of standard PCI BARs
@ 2019-08-11 15:07 Denis Efremov
2019-08-11 15:07 ` [PATCH 1/7] PCI: Add define " Denis Efremov
` (8 more replies)
0 siblings, 9 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:07 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
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.
The patch is splitted into 7 parts for different drivers/subsystems for
easy readability.
Denis Efremov (7):
PCI: Add define for the number of standard PCI BARs
s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
rapidio/tsi721: use PCI_STD_NUM_BARS in loops instead of
PCI_STD_RESOURCE_END
efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
vfio_pci: Use PCI_STD_NUM_BARS in loops instead of
PCI_STD_RESOURCE_END
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/quirks.c | 2 +-
drivers/rapidio/devices/tsi721.c | 2 +-
drivers/vfio/pci/vfio_pci.c | 4 ++--
drivers/vfio/pci/vfio_pci_config.c | 2 +-
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 +
15 files changed, 29 insertions(+), 31 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/7] PCI: Add define for the number of standard PCI BARs
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
@ 2019-08-11 15:07 ` Denis Efremov
2019-08-11 15:07 ` [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS Denis Efremov
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:07 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
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 related [flat|nested] 18+ messages in thread
* [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
2019-08-11 15:07 ` [PATCH 1/7] PCI: Add define " Denis Efremov
@ 2019-08-11 15:07 ` Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-11 15:07 ` [PATCH 3/7] x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END Denis Efremov
` (6 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:07 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, linux-s390,
linux-pci, linux-kernel
This patch replaces local define PCI_BAR_COUNT with global
PCI_STD_NUM_BARS.
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..c57ccac25c02 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 (!pci_resource_len(pdev, bar) || bar >= PCI_STD_NUM_BARS)
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 (!pci_resource_len(pdev, bar) || bar >= PCI_STD_NUM_BARS)
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 related [flat|nested] 18+ messages in thread
* [PATCH 3/7] x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
2019-08-11 15:07 ` [PATCH 1/7] PCI: Add define " Denis Efremov
2019-08-11 15:07 ` [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-11 15:07 ` Denis Efremov
2019-08-11 15:08 ` [PATCH 4/7] PCI/net: " Denis Efremov
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:07 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Denis Efremov, H. Peter Anvin, x86, linux-pci, linux-kernel
This patch refactors the loop condition scheme from
'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
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 related [flat|nested] 18+ messages in thread
* [PATCH 4/7] PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (2 preceding siblings ...)
2019-08-11 15:07 ` [PATCH 3/7] x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END Denis Efremov
@ 2019-08-11 15:08 ` Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-11 15:08 ` [PATCH 5/7] rapidio/tsi721: use " Denis Efremov
` (4 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Giuseppe Cavallaro, Alexandre Torgue, netdev,
linux-pci, linux-kernel
This patch refactors the loop condition scheme from
'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
Signed-off-by: Denis Efremov <efremov@linux.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
2 files changed, 3 insertions(+), 3 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));
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 related [flat|nested] 18+ messages in thread
* [PATCH 5/7] rapidio/tsi721: use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (3 preceding siblings ...)
2019-08-11 15:08 ` [PATCH 4/7] PCI/net: " Denis Efremov
@ 2019-08-11 15:08 ` Denis Efremov
2019-08-11 15:08 ` [PATCH 6/7] efifb: Use " Denis Efremov
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Matt Porter, Alexandre Bounine, linux-pci, linux-kernel
This patch refactors the loop condition scheme from
'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
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 related [flat|nested] 18+ messages in thread
* [PATCH 6/7] efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (4 preceding siblings ...)
2019-08-11 15:08 ` [PATCH 5/7] rapidio/tsi721: use " Denis Efremov
@ 2019-08-11 15:08 ` Denis Efremov
2019-08-13 12:48 ` Bartlomiej Zolnierkiewicz
2019-08-11 15:08 ` [PATCH 7/7] vfio_pci: " Denis Efremov
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Peter Jones, Bartlomiej Zolnierkiewicz,
linux-fbdev, linux-pci, linux-kernel
This patch refactors the loop condition scheme from
'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
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 related [flat|nested] 18+ messages in thread
* [PATCH 7/7] vfio_pci: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (5 preceding siblings ...)
2019-08-11 15:08 ` [PATCH 6/7] efifb: Use " Denis Efremov
@ 2019-08-11 15:08 ` Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-12 9:06 ` [PATCH 0/7] Add definition for the number of standard PCI BARs Andrew Murray
2019-08-12 20:01 ` Bjorn Helgaas
8 siblings, 1 reply; 18+ messages in thread
From: Denis Efremov @ 2019-08-11 15:08 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Cornelia Huck, Alex Williamson, kvm, linux-pci,
linux-kernel
This patch refactors the loop condition scheme from
'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
Signed-off-by: Denis Efremov <efremov@linux.com>
---
drivers/vfio/pci/vfio_pci.c | 4 ++--
drivers/vfio/pci/vfio_pci_config.c | 2 +-
drivers/vfio/pci/vfio_pci_private.h | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 703948c9fbe1..13f5430e3f3c 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -115,7 +115,7 @@ static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev)
INIT_LIST_HEAD(&vdev->dummy_resources_list);
- for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
+ for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
res = vdev->pdev->resource + bar;
if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP))
@@ -399,7 +399,7 @@ 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 (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
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..6035a2961160 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -455,7 +455,7 @@ 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++) {
+ for (i = 0; i < PCI_STD_NUM_BARS; i++, bar++) {
if (!pci_resource_start(pdev, i)) {
*bar = 0; /* Unmapped by host = unimplemented to user */
continue;
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 related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (6 preceding siblings ...)
2019-08-11 15:08 ` [PATCH 7/7] vfio_pci: " Denis Efremov
@ 2019-08-12 9:06 ` Andrew Murray
2019-08-12 21:00 ` Denis Efremov
2019-08-12 20:01 ` Bjorn Helgaas
8 siblings, 1 reply; 18+ messages in thread
From: Andrew Murray @ 2019-08-12 9:06 UTC (permalink / raw)
To: Denis Efremov
Cc: Bjorn Helgaas, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
On Sun, Aug 11, 2019 at 06:07:55PM +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.
>
> The patch is splitted into 7 parts for different drivers/subsystems for
> easy readability.
>
> Denis Efremov (7):
> PCI: Add define for the number of standard PCI BARs
> s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
> x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> rapidio/tsi721: use PCI_STD_NUM_BARS in loops instead of
> PCI_STD_RESOURCE_END
> efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> vfio_pci: Use PCI_STD_NUM_BARS in loops instead of
> PCI_STD_RESOURCE_END
>
> 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/quirks.c | 2 +-
> drivers/rapidio/devices/tsi721.c | 2 +-
> drivers/vfio/pci/vfio_pci.c | 4 ++--
> drivers/vfio/pci/vfio_pci_config.c | 2 +-
> 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 +
Hi Denis,
You could also fix up a few cases where the number of BARs is hard coded in
loops, e.g.
drivers/pci/controller/pci-hyperv.c - look for uses of probed_bar in loops
drivers/pci/pci.c - pci_release_selected_regions and __pci_request_selected_regions
drivers/pci/quirks.c - quirk_alder_ioapic
Thanks,
Andrew Murray
> 15 files changed, 29 insertions(+), 31 deletions(-)
>
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
` (7 preceding siblings ...)
2019-08-12 9:06 ` [PATCH 0/7] Add definition for the number of standard PCI BARs Andrew Murray
@ 2019-08-12 20:01 ` Bjorn Helgaas
2019-08-12 20:11 ` Thomas Gleixner
8 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-12 20:01 UTC (permalink / raw)
To: Denis Efremov
Cc: Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
On Sun, Aug 11, 2019 at 06:07:55PM +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.
>
> The patch is splitted into 7 parts for different drivers/subsystems for
> easy readability.
This looks good. I can take all these together, since they all depend
on the first patch. I have a few comments on the individual patches.
> Denis Efremov (7):
> PCI: Add define for the number of standard PCI BARs
> s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
> x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> rapidio/tsi721: use PCI_STD_NUM_BARS in loops instead of
> PCI_STD_RESOURCE_END
> efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
> vfio_pci: Use PCI_STD_NUM_BARS in loops instead of
> PCI_STD_RESOURCE_END
>
> 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/quirks.c | 2 +-
> drivers/rapidio/devices/tsi721.c | 2 +-
> drivers/vfio/pci/vfio_pci.c | 4 ++--
> drivers/vfio/pci/vfio_pci_config.c | 2 +-
> 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 +
> 15 files changed, 29 insertions(+), 31 deletions(-)
>
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
2019-08-11 15:07 ` [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-12 20:02 ` Bjorn Helgaas
2019-08-12 21:06 ` Denis Efremov
0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-12 20:02 UTC (permalink / raw)
To: Denis Efremov
Cc: Sebastian Ott, Gerald Schaefer, linux-s390, linux-pci, linux-kernel
On Sun, Aug 11, 2019 at 06:07:57PM +0300, Denis Efremov wrote:
> This patch replaces local define PCI_BAR_COUNT with global
> PCI_STD_NUM_BARS.
>
> 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..c57ccac25c02 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 (!pci_resource_len(pdev, bar) || bar >= PCI_STD_NUM_BARS)
The two conditions in this test should be reversed so we bounds check
"bar" before using it, i.e.,:
if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
That's not strictly related to the purpose of this patch, but it's so
trivial that I don't think it's worth splitting to a separate patch.
> 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 (!pci_resource_len(pdev, bar) || bar >= PCI_STD_NUM_BARS)
Same here.
> 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;
There's a loop just below here that should be rewritten in the typical
style:
i = PCI_IOV_RESOURCES;
for (; i < PCI_SRIOV_NUM_BARS + PCI_IOV_RESOURCES; i++) {
len = pci_resource_len(pdev, i);
Again, not strictly related to this patch, but probably trivial enough
to do in the same patch.
> @@ -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;
Completely unrelated to this patch, but the fact that
zpci_map_resources() deals with both standard BARs and SR-IOV BARs
while zpci_unmap_resources() only deals with standard BARs makes me
wonder why they're not symmetric.
It is true that zpci_map_resources() doesn't handle standard and
SR-IOV BARs similarly, so maybe this is intended. Maybe the s390
folks know.
> @@ -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
* Re: [PATCH 4/7] PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:08 ` [PATCH 4/7] PCI/net: " Denis Efremov
@ 2019-08-12 20:02 ` Bjorn Helgaas
0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-12 20:02 UTC (permalink / raw)
To: Denis Efremov
Cc: Giuseppe Cavallaro, Alexandre Torgue, netdev, linux-pci, linux-kernel
The subject can be simply:
<prefix>: Loop using PCI_STD_NUM_BARS
to keep them a little shorter so "git log --online" doesn't wrap.
On Sun, Aug 11, 2019 at 06:08:00PM +0300, Denis Efremov wrote:
> This patch refactors the loop condition scheme from
> 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of 'i <=
PCI_STD_RESOURCE_END'.
See https://chris.beams.io/posts/git-commit/
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
> drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
This patch touches two unrelated drivers and should be split up.
When you do that, pay attention to the convention for commit log
prefixes, e.g.,
$ git log --oneline drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
37e9c087c814 stmmac: pci: Fix typo in IOT2000 comment
d4a62ea411f9 stmmac: pci: Use pci_dev_id() helper
e0c1d14a1a32 stmmac: pci: Adjust IOT2000 matching
$ git log --oneline drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
ea8c1c642ea5 net: dwc-xlgmac: declaration of dual license in headers
65e0ace2c5cd net: dwc-xlgmac: Initial driver for DesignWare Enterprise Ethernet
> 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));
> 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
* Re: [PATCH 7/7] vfio_pci: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:08 ` [PATCH 7/7] vfio_pci: " Denis Efremov
@ 2019-08-12 20:02 ` Bjorn Helgaas
2019-08-12 20:52 ` Alex Williamson
0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2019-08-12 20:02 UTC (permalink / raw)
To: Denis Efremov
Cc: Cornelia Huck, Alex Williamson, kvm, linux-pci, linux-kernel
On Sun, Aug 11, 2019 at 06:08:04PM +0300, Denis Efremov wrote:
> This patch refactors the loop condition scheme from
> 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> drivers/vfio/pci/vfio_pci.c | 4 ++--
> drivers/vfio/pci/vfio_pci_config.c | 2 +-
> drivers/vfio/pci/vfio_pci_private.h | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 703948c9fbe1..13f5430e3f3c 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -115,7 +115,7 @@ static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev)
>
> INIT_LIST_HEAD(&vdev->dummy_resources_list);
>
> - for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
> + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> res = vdev->pdev->resource + bar;
PCI_STD_RESOURCES is indeed 0, but since the original went to the
trouble of avoiding that assumption, I would probably do this:
for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
res = vdev->pdev->resource + bar + PCI_STD_RESOURCES;
or maybe even this:
res = &vdev->pdev->resource[bar + PCI_STD_RESOURCES];
which is more common outside vfio. But I wouldn't change to using the
&dev->resource[] form if other vfio code that you're *not* changing
uses the dev->resource + bar form.
> if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP))
> @@ -399,7 +399,7 @@ 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 (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> 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..6035a2961160 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -455,7 +455,7 @@ 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++) {
> + for (i = 0; i < PCI_STD_NUM_BARS; i++, bar++) {
> if (!pci_resource_start(pdev, i)) {
> *bar = 0; /* Unmapped by host = unimplemented to user */
> continue;
> 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
* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
2019-08-12 20:01 ` Bjorn Helgaas
@ 2019-08-12 20:11 ` Thomas Gleixner
0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2019-08-12 20:11 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
On Mon, 12 Aug 2019, Bjorn Helgaas wrote:
> On Sun, Aug 11, 2019 at 06:07:55PM +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.
> >
> > The patch is splitted into 7 parts for different drivers/subsystems for
> > easy readability.
>
> This looks good. I can take all these together, since they all depend
> on the first patch. I have a few comments on the individual patches.
>
> > Denis Efremov (7):
> > PCI: Add define for the number of standard PCI BARs
> > s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
> > x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
Fine with me for the x86 part. That's your turf anyway :)
Thanks,
tglx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 7/7] vfio_pci: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-12 20:02 ` Bjorn Helgaas
@ 2019-08-12 20:52 ` Alex Williamson
0 siblings, 0 replies; 18+ messages in thread
From: Alex Williamson @ 2019-08-12 20:52 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Denis Efremov, Cornelia Huck, kvm, linux-pci, linux-kernel
On Mon, 12 Aug 2019 15:02:34 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Sun, Aug 11, 2019 at 06:08:04PM +0300, Denis Efremov wrote:
> > This patch refactors the loop condition scheme from
> > 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
> >
> > Signed-off-by: Denis Efremov <efremov@linux.com>
> > ---
> > drivers/vfio/pci/vfio_pci.c | 4 ++--
> > drivers/vfio/pci/vfio_pci_config.c | 2 +-
> > drivers/vfio/pci/vfio_pci_private.h | 4 ++--
> > 3 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > index 703948c9fbe1..13f5430e3f3c 100644
> > --- a/drivers/vfio/pci/vfio_pci.c
> > +++ b/drivers/vfio/pci/vfio_pci.c
> > @@ -115,7 +115,7 @@ static void vfio_pci_probe_mmaps(struct vfio_pci_device *vdev)
> >
> > INIT_LIST_HEAD(&vdev->dummy_resources_list);
> >
> > - for (bar = PCI_STD_RESOURCES; bar <= PCI_STD_RESOURCE_END; bar++) {
> > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > res = vdev->pdev->resource + bar;
>
> PCI_STD_RESOURCES is indeed 0, but since the original went to the
> trouble of avoiding that assumption, I would probably do this:
>
> for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> res = vdev->pdev->resource + bar + PCI_STD_RESOURCES;
>
> or maybe even this:
>
> res = &vdev->pdev->resource[bar + PCI_STD_RESOURCES];
>
> which is more common outside vfio. But I wouldn't change to using the
> &dev->resource[] form if other vfio code that you're *not* changing
> uses the dev->resource + bar form.
I don't think we have any other instances like that, so the latter form
is fine with me if it's more broadly used. I do spot one use of [bar]
in drivers/vfio/pci/vfio_pci_rdwr.c that could also take on this form
to void the same assumption though. Thanks,
Alex
> > if (!IS_ENABLED(CONFIG_VFIO_PCI_MMAP))
> > @@ -399,7 +399,7 @@ 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 (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > 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..6035a2961160 100644
> > --- a/drivers/vfio/pci/vfio_pci_config.c
> > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > @@ -455,7 +455,7 @@ 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++) {
> > + for (i = 0; i < PCI_STD_NUM_BARS; i++, bar++) {
> > if (!pci_resource_start(pdev, i)) {
> > *bar = 0; /* Unmapped by host = unimplemented to user */
> > continue;
> > 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
* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
2019-08-12 9:06 ` [PATCH 0/7] Add definition for the number of standard PCI BARs Andrew Murray
@ 2019-08-12 21:00 ` Denis Efremov
0 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-12 21:00 UTC (permalink / raw)
To: Andrew Murray
Cc: Bjorn Helgaas, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
linux-s390, linux-pci, linux-kernel
On 12.08.2019 12:06, Andrew Murray wrote:
>
> Hi Denis,
Hi!
>
> You could also fix up a few cases where the number of BARs is hard coded in
> loops, e.g.
>
> drivers/pci/controller/pci-hyperv.c - look for uses of probed_bar in loops
> drivers/pci/pci.c - pci_release_selected_regions and __pci_request_selected_regions
> drivers/pci/quirks.c - quirk_alder_ioapic
>
Thanks for pointing me on that. I will take this into account in v2.
Denis
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
2019-08-12 20:02 ` Bjorn Helgaas
@ 2019-08-12 21:06 ` Denis Efremov
0 siblings, 0 replies; 18+ messages in thread
From: Denis Efremov @ 2019-08-12 21:06 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Sebastian Ott, Gerald Schaefer, linux-s390, linux-pci, linux-kernel
Hi!
>
> There's a loop just below here that should be rewritten in the typical
> style:
>
> i = PCI_IOV_RESOURCES;
> for (; i < PCI_SRIOV_NUM_BARS + PCI_IOV_RESOURCES; i++) {
> len = pci_resource_len(pdev, i);
>
> Again, not strictly related to this patch, but probably trivial enough
> to do in the same patch.
>
I think it's already fixed in a separate patch https://lkml.org/lkml/2019/8/6/694
Thanks for the review, I will send v2.
Denis
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
2019-08-11 15:08 ` [PATCH 6/7] efifb: Use " Denis Efremov
@ 2019-08-13 12:48 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 18+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2019-08-13 12:48 UTC (permalink / raw)
To: Denis Efremov, Bjorn Helgaas
Cc: Peter Jones, linux-fbdev, linux-pci, linux-kernel
On 8/11/19 5:08 PM, Denis Efremov wrote:
> This patch refactors the loop condition scheme from
> 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.
>
> Signed-off-by: Denis Efremov <efremov@linux.com>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.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))
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-08-13 12:48 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-11 15:07 [PATCH 0/7] Add definition for the number of standard PCI BARs Denis Efremov
2019-08-11 15:07 ` [PATCH 1/7] PCI: Add define " Denis Efremov
2019-08-11 15:07 ` [PATCH 2/7] s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-12 21:06 ` Denis Efremov
2019-08-11 15:07 ` [PATCH 3/7] x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END Denis Efremov
2019-08-11 15:08 ` [PATCH 4/7] PCI/net: " Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-11 15:08 ` [PATCH 5/7] rapidio/tsi721: use " Denis Efremov
2019-08-11 15:08 ` [PATCH 6/7] efifb: Use " Denis Efremov
2019-08-13 12:48 ` Bartlomiej Zolnierkiewicz
2019-08-11 15:08 ` [PATCH 7/7] vfio_pci: " Denis Efremov
2019-08-12 20:02 ` Bjorn Helgaas
2019-08-12 20:52 ` Alex Williamson
2019-08-12 9:06 ` [PATCH 0/7] Add definition for the number of standard PCI BARs Andrew Murray
2019-08-12 21:00 ` Denis Efremov
2019-08-12 20:01 ` Bjorn Helgaas
2019-08-12 20:11 ` Thomas Gleixner
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).