* [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
` (2 more replies)
0 siblings, 3 replies; 7+ 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] 7+ 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 08/10] vfio_pci: Loop using PCI_STD_NUM_BARS Denis Efremov
2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
2 siblings, 0 replies; 7+ 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 related [flat|nested] 7+ 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
2019-08-16 9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
@ 2019-08-16 9:24 ` Denis Efremov
2019-08-16 16:23 ` Alex Williamson
2019-08-16 10:51 ` [PATCH v2 00/10] Add definition for the number of standard PCI BARs Andrew Murray
2 siblings, 1 reply; 7+ 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 related [flat|nested] 7+ 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
2019-08-16 9:24 ` [PATCH v2 01/10] PCI: Add define " Denis Efremov
2019-08-16 9:24 ` [PATCH v2 08/10] vfio_pci: Loop using PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-16 10:51 ` Andrew Murray
2019-08-16 13:35 ` Bjorn Helgaas
2019-09-05 19:02 ` Denis Efremov
2 siblings, 2 replies; 7+ 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] 7+ 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; 7+ 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] 7+ 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: Loop using PCI_STD_NUM_BARS Denis Efremov
@ 2019-08-16 16:23 ` Alex Williamson
0 siblings, 0 replies; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2019-09-05 19:02 UTC | newest]
Thread overview: 7+ 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 08/10] vfio_pci: Loop using PCI_STD_NUM_BARS Denis Efremov
2019-08-16 16:23 ` Alex Williamson
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
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).