2019-09-16 22:23:34

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH v3 02/26] PCI: hv: Use PCI_STD_NUM_BARS

Replace the magic constant (6) with define PCI_STD_NUM_BARS representing
the number of PCI BARs.

Cc: "K. Y. Srinivasan" <[email protected]>
Cc: Haiyang Zhang <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Sasha Levin <[email protected]>
Signed-off-by: Denis Efremov <[email protected]>
---
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


2019-09-26 22:10:20

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 02/26] PCI: hv: Use PCI_STD_NUM_BARS

On Mon, Sep 16, 2019 at 11:41:34PM +0300, Denis Efremov wrote:
> Replace the magic constant (6) with define PCI_STD_NUM_BARS representing
> the number of PCI BARs.

For some reason patches 0 and 1 didn't make it to the list. Can you
resend them?

2019-09-27 12:47:13

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v3 02/26] PCI: hv: Use PCI_STD_NUM_BARS

On Thu, Sep 26, 2019 at 05:05:31PM -0500, Bjorn Helgaas wrote:
> On Mon, Sep 16, 2019 at 11:41:34PM +0300, Denis Efremov wrote:
> > Replace the magic constant (6) with define PCI_STD_NUM_BARS representing
> > the number of PCI BARs.
>
> For some reason patches 0 and 1 didn't make it to the list. Can you
> resend them?

(No need to resend the whole series, which might annoy all the other
maintainers. Just send 0 (the cover letter) and 1 (which I assume
adds the PCI_STD_NUM_BARS definition)).

2019-09-27 23:41:28

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH RESEND v3 00/26] Add definition for the number of standard PCI BARs

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. The patchset also replaces constant (6)
with new define PCI_STD_NUM_BARS where appropriate and removes local
declarations for the number of PCI BARs.

Changes in v3:
- Updated commits description.
- Refactored "< PCI_ROM_RESOURCE" with "< PCI_STD_NUM_BARS" in loops.
- Refactored "<= BAR_5" with "< PCI_STD_NUM_BARS" in loops.
- Removed local define GASKET_NUM_BARS.
- Removed local define PCI_NUM_BAR_RESOURCES.

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

Denis Efremov (26):
PCI: Add define for the number of standard PCI BARs
PCI: hv: Use PCI_STD_NUM_BARS
PCI: dwc: Use PCI_STD_NUM_BARS
PCI: endpoint: Use PCI_STD_NUM_BARS
misc: pci_endpoint_test: Use PCI_STD_NUM_BARS
s390/pci: Use PCI_STD_NUM_BARS
x86/PCI: Loop using PCI_STD_NUM_BARS
alpha/PCI: Use PCI_STD_NUM_BARS
ia64: Use PCI_STD_NUM_BARS
stmmac: pci: Loop using PCI_STD_NUM_BARS
net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
ixgb: use PCI_STD_NUM_BARS
e1000: Use PCI_STD_NUM_BARS
rapidio/tsi721: Loop using PCI_STD_NUM_BARS
efifb: Loop using PCI_STD_NUM_BARS
fbmem: use PCI_STD_NUM_BARS
vfio_pci: Loop using PCI_STD_NUM_BARS
scsi: pm80xx: Use PCI_STD_NUM_BARS
ata: sata_nv: Use PCI_STD_NUM_BARS
staging: gasket: Use PCI_STD_NUM_BARS
serial: 8250_pci: Use PCI_STD_NUM_BARS
pata_atp867x: Use PCI_STD_NUM_BARS
memstick: use PCI_STD_NUM_BARS
USB: core: Use PCI_STD_NUM_BARS
usb: pci-quirks: Use PCI_STD_NUM_BARS
devres: use PCI_STD_NUM_BARS

arch/alpha/kernel/pci-sysfs.c | 8 ++---
arch/ia64/sn/pci/pcibr/pcibr_dma.c | 4 +--
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 +-
arch/x86/pci/intel_mid_pci.c | 2 +-
drivers/ata/pata_atp867x.c | 2 +-
drivers/ata/sata_nv.c | 2 +-
drivers/memstick/host/jmb38x_ms.c | 2 +-
drivers/misc/pci_endpoint_test.c | 8 ++---
drivers/net/ethernet/intel/e1000/e1000.h | 1 -
drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +-
drivers/net/ethernet/intel/ixgb/ixgb.h | 1 -
drivers/net/ethernet/intel/ixgb/ixgb_main.c | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 +--
.../net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
.../pci/controller/dwc/pci-layerscape-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
.../pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 2 +-
drivers/pci/controller/pci-hyperv.c | 10 +++---
drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++---
drivers/pci/pci-sysfs.c | 4 +--
drivers/pci/pci.c | 13 ++++----
drivers/pci/proc.c | 4 +--
drivers/pci/quirks.c | 4 +--
drivers/rapidio/devices/tsi721.c | 2 +-
drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
drivers/scsi/pm8001/pm8001_init.c | 2 +-
drivers/staging/gasket/gasket_constants.h | 3 --
drivers/staging/gasket/gasket_core.c | 12 +++----
drivers/staging/gasket/gasket_core.h | 4 +--
drivers/tty/serial/8250/8250_pci.c | 8 ++---
drivers/usb/core/hcd-pci.c | 2 +-
drivers/usb/host/pci-quirks.c | 2 +-
drivers/vfio/pci/vfio_pci.c | 11 ++++---
drivers/vfio/pci/vfio_pci_config.c | 32 ++++++++++---------
drivers/vfio/pci/vfio_pci_private.h | 4 +--
drivers/video/fbdev/core/fbmem.c | 4 +--
drivers/video/fbdev/efifb.c | 2 +-
include/linux/pci-epc.h | 2 +-
include/linux/pci.h | 2 +-
include/uapi/linux/pci_regs.h | 1 +
lib/devres.c | 2 +-
47 files changed, 112 insertions(+), 115 deletions(-)

--
2.21.0

2019-09-27 23:43:54

by Denis Efremov (Oracle)

[permalink] [raw]
Subject: [PATCH RESEND v3 01/26] PCI: Add define for the number of standard PCI BARs

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". To iterate through all possible BARs, loop
conditions changed to the *number* of BARs "i < PCI_STD_NUM_BARS",
instead of the index of the last valid BAR "i <= PCI_STD_RESOURCE_END"
or PCI_ROM_RESOURCE. The magic constant (6) is also replaced with new
define PCI_STD_NUM_BARS.

Signed-off-by: Denis Efremov <[email protected]>
---
drivers/pci/pci-sysfs.c | 4 ++--
drivers/pci/pci.c | 13 +++++++------
drivers/pci/proc.c | 4 ++--
drivers/pci/quirks.c | 4 ++--
include/linux/pci.h | 2 +-
include/uapi/linux/pci_regs.h | 1 +
6 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 965c72104150..3e26b8e03bd5 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1257,7 +1257,7 @@ static void pci_remove_resource_files(struct pci_dev *pdev)
{
int i;

- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ for (i = 0; i < PCI_STD_NUM_BARS; i++) {
struct bin_attribute *res_attr;

res_attr = pdev->res_attr[i];
@@ -1328,7 +1328,7 @@ static int pci_create_resource_files(struct pci_dev *pdev)
int retval;

/* Expose the PCI resources from this device as files */
- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ for (i = 0; i < PCI_STD_NUM_BARS; i++) {

/* skip empty resources */
if (!pci_resource_len(pdev, i))
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1b27b5af3d55..7d543986026b 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -674,7 +674,7 @@ struct resource *pci_find_resource(struct pci_dev *dev, struct resource *res)
{
int i;

- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ for (i = 0; i < PCI_STD_NUM_BARS; i++) {
struct resource *r = &dev->resource[i];

if (r->start && resource_contains(r, res))
@@ -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/proc.c b/drivers/pci/proc.c
index fe7fe678965b..cb61ec2c24e8 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -248,13 +248,13 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
}

/* Make sure the caller is mapping a real resource for this device */
- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ for (i = 0; i < PCI_STD_NUM_BARS; i++) {
if (dev->resource[i].flags & res_bit &&
pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
break;
}

- if (i >= PCI_ROM_RESOURCE)
+ if (i >= PCI_STD_NUM_BARS)
return -ENODEV;

if (fpriv->mmap_state == pci_mmap_mem &&
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 44c4ae1abd00..998454b0ae8d 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) {
@@ -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);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 82e4cd1b7ac3..cf7d16305243 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

2019-09-30 21:16:52

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH RESEND v3 00/26] Add definition for the number of standard PCI BARs

On Sat, Sep 28, 2019 at 02:40:26AM +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. The patchset also replaces constant (6)
> with new define PCI_STD_NUM_BARS where appropriate and removes local
> declarations for the number of PCI BARs.
>
> Changes in v3:
> - Updated commits description.
> - Refactored "< PCI_ROM_RESOURCE" with "< PCI_STD_NUM_BARS" in loops.
> - Refactored "<= BAR_5" with "< PCI_STD_NUM_BARS" in loops.
> - Removed local define GASKET_NUM_BARS.
> - Removed local define PCI_NUM_BAR_RESOURCES.
>
> Changes in v2:
> - Reversed checks in pci_iomap_range,pci_iomap_wc_range.
> - Refactored loops in vfio_pci to keep PCI_STD_RESOURCES.
> - Added 2 new patches to replace the magic constant with new define.
> - Splitted net patch in v1 to separate stmmac and dwc-xlgmac patches.
>
> Denis Efremov (26):
> PCI: Add define for the number of standard PCI BARs
> PCI: hv: Use PCI_STD_NUM_BARS
> PCI: dwc: Use PCI_STD_NUM_BARS
> PCI: endpoint: Use PCI_STD_NUM_BARS
> misc: pci_endpoint_test: Use PCI_STD_NUM_BARS
> s390/pci: Use PCI_STD_NUM_BARS
> x86/PCI: Loop using PCI_STD_NUM_BARS
> alpha/PCI: Use PCI_STD_NUM_BARS
> ia64: Use PCI_STD_NUM_BARS
> stmmac: pci: Loop using PCI_STD_NUM_BARS
> net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS
> ixgb: use PCI_STD_NUM_BARS
> e1000: Use PCI_STD_NUM_BARS
> rapidio/tsi721: Loop using PCI_STD_NUM_BARS
> efifb: Loop using PCI_STD_NUM_BARS
> fbmem: use PCI_STD_NUM_BARS
> vfio_pci: Loop using PCI_STD_NUM_BARS
> scsi: pm80xx: Use PCI_STD_NUM_BARS
> ata: sata_nv: Use PCI_STD_NUM_BARS
> staging: gasket: Use PCI_STD_NUM_BARS
> serial: 8250_pci: Use PCI_STD_NUM_BARS
> pata_atp867x: Use PCI_STD_NUM_BARS
> memstick: use PCI_STD_NUM_BARS
> USB: core: Use PCI_STD_NUM_BARS
> usb: pci-quirks: Use PCI_STD_NUM_BARS
> devres: use PCI_STD_NUM_BARS
>
> arch/alpha/kernel/pci-sysfs.c | 8 ++---
> arch/ia64/sn/pci/pcibr/pcibr_dma.c | 4 +--
> 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 +-
> arch/x86/pci/intel_mid_pci.c | 2 +-
> drivers/ata/pata_atp867x.c | 2 +-
> drivers/ata/sata_nv.c | 2 +-
> drivers/memstick/host/jmb38x_ms.c | 2 +-
> drivers/misc/pci_endpoint_test.c | 8 ++---
> drivers/net/ethernet/intel/e1000/e1000.h | 1 -
> drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +-
> drivers/net/ethernet/intel/ixgb/ixgb.h | 1 -
> drivers/net/ethernet/intel/ixgb/ixgb_main.c | 2 +-
> .../net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 +--
> .../net/ethernet/synopsys/dwc-xlgmac-pci.c | 2 +-
> drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
> .../pci/controller/dwc/pci-layerscape-ep.c | 2 +-
> drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
> .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 2 +-
> drivers/pci/controller/pci-hyperv.c | 10 +++---
> drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++---
> drivers/pci/pci-sysfs.c | 4 +--
> drivers/pci/pci.c | 13 ++++----
> drivers/pci/proc.c | 4 +--
> drivers/pci/quirks.c | 4 +--
> drivers/rapidio/devices/tsi721.c | 2 +-
> drivers/scsi/pm8001/pm8001_hwi.c | 2 +-
> drivers/scsi/pm8001/pm8001_init.c | 2 +-
> drivers/staging/gasket/gasket_constants.h | 3 --
> drivers/staging/gasket/gasket_core.c | 12 +++----
> drivers/staging/gasket/gasket_core.h | 4 +--
> drivers/tty/serial/8250/8250_pci.c | 8 ++---
> drivers/usb/core/hcd-pci.c | 2 +-
> drivers/usb/host/pci-quirks.c | 2 +-
> drivers/vfio/pci/vfio_pci.c | 11 ++++---
> drivers/vfio/pci/vfio_pci_config.c | 32 ++++++++++---------
> drivers/vfio/pci/vfio_pci_private.h | 4 +--
> drivers/video/fbdev/core/fbmem.c | 4 +--
> drivers/video/fbdev/efifb.c | 2 +-
> include/linux/pci-epc.h | 2 +-
> include/linux/pci.h | 2 +-
> include/uapi/linux/pci_regs.h | 1 +
> lib/devres.c | 2 +-
> 47 files changed, 112 insertions(+), 115 deletions(-)

Applied to pci/resource for v5.5, thanks!

I ended up squashing these all together because they're all related
and tiny.