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

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

[Denis Efremov]

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

Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/controller/pci-hyperv.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

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

RE: [PATCH v3 02/26] PCI: hv: Use PCI_STD_NUM_BARS

[Haiyang Zhang]

> -----Original Message-----
> From: Denis Efremov <efremov@linux.com>
> Sent: Monday, September 16, 2019 4:42 PM
> To: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Denis Efremov <efremov@linux.com>; linux-kernel@vger.kernel.org;
> linux-pci@vger.kernel.org; Andrew Murray <andrew.murray@arm.com>;
> linux-hyperv@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Haiyang
> Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Sasha Levin <sashal@kernel.org>
> 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" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Sasha Levin <sashal@kernel.org>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/pci/controller/pci-hyperv.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-
> hyperv.c
> index 40b625458afa..1665c23b649f 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -307,7 +307,7 @@ struct pci_bus_relations {  struct
> pci_q_res_req_response {
>  	struct vmpacket_descriptor hdr;
>  	s32 status;			/* negative values are failures */
> -	u32 probed_bar[6];
> +	u32 probed_bar[PCI_STD_NUM_BARS];
>  } __packed;
> 
>  struct pci_set_power {
> @@ -503,7 +503,7 @@ struct hv_pci_dev {
>  	 * What would be observed if one wrote 0xFFFFFFFF to a BAR and
> then
>  	 * read it back, for each of the BAR offsets within config space.
>  	 */
> -	u32 probed_bar[6];
> +	u32 probed_bar[PCI_STD_NUM_BARS];
>  };
> 
>  struct hv_pci_compl {
> @@ -1327,7 +1327,7 @@ static void survey_child_resources(struct
> hv_pcibus_device *hbus)
>  	 * so it's sufficient to just add them up without tracking alignment.
>  	 */
>  	list_for_each_entry(hpdev, &hbus->children, list_entry) {
> -		for (i = 0; i < 6; i++) {
> +		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  			if (hpdev->probed_bar[i] &
> PCI_BASE_ADDRESS_SPACE_IO)
>  				dev_err(&hbus->hdev->device,
>  					"There's an I/O BAR in this list!\n");
> @@ -1401,7 +1401,7 @@ static void prepopulate_bars(struct
> hv_pcibus_device *hbus)
>  	/* Pick addresses for the BARs. */
>  	do {
>  		list_for_each_entry(hpdev, &hbus->children, list_entry) {
> -			for (i = 0; i < 6; i++) {
> +			for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  				bar_val = hpdev->probed_bar[i];
>  				if (bar_val == 0)
>  					continue;
> @@ -1558,7 +1558,7 @@ static void q_resource_requirements(void
> *context, struct pci_response *resp,
>  			"query resource requirements failed: %x\n",
>  			resp->status);
>  	} else {
> -		for (i = 0; i < 6; i++) {
> +		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  			completion->hpdev->probed_bar[i] =
>  				q_res_req->probed_bar[i];
>  		}

Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Thanks.

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

[Bjorn Helgaas]

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?

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

[Bjorn Helgaas]

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)).

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

[Denis Efremov]

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

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

[Bjorn Helgaas]

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.

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

[Denis Efremov]

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 <efremov@linux.com>
---
 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

[PATCH v3 03/26] PCI: dwc: Use PCI_STD_NUM_BARS

[Denis Efremov]

To iterate through all possible BARs, loop conditions refactored to the
*number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
appropriate.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/controller/dwc/pci-dra7xx.c           | 2 +-
 drivers/pci/controller/dwc/pci-layerscape-ep.c    | 2 +-
 drivers/pci/controller/dwc/pcie-artpec6.c         | 2 +-
 drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
 drivers/pci/controller/dwc/pcie-designware.h      | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 4234ddb4722f..b20651cea09f 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -353,7 +353,7 @@ static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
 	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
 	enum pci_barno bar;
 
-	for (bar = BAR_0; bar <= BAR_5; bar++)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
 
 	dra7xx_pcie_enable_wrapper_interrupts(dra7xx);
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index be61d96cc95e..c84218d8ffd3 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -57,7 +57,7 @@ static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	enum pci_barno bar;
 
-	for (bar = BAR_0; bar <= BAR_5; bar++)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
 }
 
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index d00252bd8fae..9e2482bd7b6d 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -422,7 +422,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
 	artpec6_pcie_wait_for_phy(artpec6_pcie);
 	artpec6_pcie_set_nfts(artpec6_pcie);
 
-	for (bar = BAR_0; bar <= BAR_5; bar++)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
 }
 
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index b58fdcbc664b..73646b677aff 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -70,7 +70,7 @@ static void dw_plat_pcie_ep_init(struct dw_pcie_ep *ep)
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	enum pci_barno bar;
 
-	for (bar = BAR_0; bar <= BAR_5; bar++)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
 }
 
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index ffed084a0b4f..7e0526bd71ad 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -205,7 +205,7 @@ struct dw_pcie_ep {
 	phys_addr_t		phys_base;
 	size_t			addr_size;
 	size_t			page_size;
-	u8			bar_to_atu[6];
+	u8			bar_to_atu[PCI_STD_NUM_BARS];
 	phys_addr_t		*outbound_addr;
 	unsigned long		*ib_window_map;
 	unsigned long		*ob_window_map;
-- 
2.21.0

RE: [PATCH v3 03/26] PCI: dwc: Use PCI_STD_NUM_BARS

[Gustavo Pimentel]

On Mon, Sep 16, 2019 at 21:41:35, Denis Efremov <efremov@linux.com> 
wrote:

> To iterate through all possible BARs, loop conditions refactored to the
> *number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
> valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
> the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
> appropriate.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c           | 2 +-
>  drivers/pci/controller/dwc/pci-layerscape-ep.c    | 2 +-
>  drivers/pci/controller/dwc/pcie-artpec6.c         | 2 +-
>  drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
>  drivers/pci/controller/dwc/pcie-designware.h      | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 4234ddb4722f..b20651cea09f 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -353,7 +353,7 @@ static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>  	enum pci_barno bar;
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar++)
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
>  		dw_pcie_ep_reset_bar(pci, bar);
>  
>  	dra7xx_pcie_enable_wrapper_interrupts(dra7xx);
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96cc95e..c84218d8ffd3 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -57,7 +57,7 @@ static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	enum pci_barno bar;
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar++)
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
>  		dw_pcie_ep_reset_bar(pci, bar);
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
> index d00252bd8fae..9e2482bd7b6d 100644
> --- a/drivers/pci/controller/dwc/pcie-artpec6.c
> +++ b/drivers/pci/controller/dwc/pcie-artpec6.c
> @@ -422,7 +422,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>  	artpec6_pcie_wait_for_phy(artpec6_pcie);
>  	artpec6_pcie_set_nfts(artpec6_pcie);
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar++)
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
>  		dw_pcie_ep_reset_bar(pci, bar);
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
> index b58fdcbc664b..73646b677aff 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-plat.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
> @@ -70,7 +70,7 @@ static void dw_plat_pcie_ep_init(struct dw_pcie_ep *ep)
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	enum pci_barno bar;
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar++)
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
>  		dw_pcie_ep_reset_bar(pci, bar);
>  }
>  
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ffed084a0b4f..7e0526bd71ad 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -205,7 +205,7 @@ struct dw_pcie_ep {
>  	phys_addr_t		phys_base;
>  	size_t			addr_size;
>  	size_t			page_size;
> -	u8			bar_to_atu[6];
> +	u8			bar_to_atu[PCI_STD_NUM_BARS];
>  	phys_addr_t		*outbound_addr;
>  	unsigned long		*ib_window_map;
>  	unsigned long		*ob_window_map;
> -- 
> 2.21.0


Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>

[PATCH v3 04/26] PCI: endpoint: Use PCI_STD_NUM_BARS

[Denis Efremov]

To iterate through all possible BARs, loop conditions refactored to the
*number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
appropriate.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++++-----
 include/linux/pci-epc.h                       |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 1cfe3687a211..5d74f81ddfe4 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -44,7 +44,7 @@
 static struct workqueue_struct *kpcitest_workqueue;
 
 struct pci_epf_test {
-	void			*reg[6];
+	void			*reg[PCI_STD_NUM_BARS];
 	struct pci_epf		*epf;
 	enum pci_barno		test_reg_bar;
 	struct delayed_work	cmd_handler;
@@ -377,7 +377,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 
 	cancel_delayed_work(&epf_test->cmd_handler);
 	pci_epc_stop(epc);
-	for (bar = BAR_0; bar <= BAR_5; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		epf_bar = &epf->bar[bar];
 
 		if (epf_test->reg[bar]) {
@@ -400,7 +400,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
 
 	epc_features = epf_test->epc_features;
 
-	for (bar = BAR_0; bar <= BAR_5; bar += add) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
 		epf_bar = &epf->bar[bar];
 		/*
 		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
@@ -450,7 +450,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	}
 	epf_test->reg[test_reg_bar] = base;
 
-	for (bar = BAR_0; bar <= BAR_5; bar += add) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
 		epf_bar = &epf->bar[bar];
 		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
 
@@ -478,7 +478,7 @@ static void pci_epf_configure_bar(struct pci_epf *epf,
 	bool bar_fixed_64bit;
 	int i;
 
-	for (i = BAR_0; i <= BAR_5; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		epf_bar = &epf->bar[i];
 		bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i));
 		if (bar_fixed_64bit)
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index f641badc2c61..56f1846b9d39 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -117,7 +117,7 @@ struct pci_epc_features {
 	unsigned int	msix_capable : 1;
 	u8	reserved_bar;
 	u8	bar_fixed_64bit;
-	u64	bar_fixed_size[BAR_5 + 1];
+	u64	bar_fixed_size[PCI_STD_NUM_BARS];
 	size_t	align;
 };
 
-- 
2.21.0

Re: [PATCH v3 04/26] PCI: endpoint: Use PCI_STD_NUM_BARS

[Andrew Murray]

On Mon, Sep 16, 2019 at 11:41:36PM +0300, Denis Efremov wrote:
> To iterate through all possible BARs, loop conditions refactored to the
> *number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
> valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
> the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
> appropriate.
> 
> Cc: Kishon Vijay Abraham I <kishon@ti.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++++-----
>  include/linux/pci-epc.h                       |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 1cfe3687a211..5d74f81ddfe4 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -44,7 +44,7 @@
>  static struct workqueue_struct *kpcitest_workqueue;
>  
>  struct pci_epf_test {
> -	void			*reg[6];
> +	void			*reg[PCI_STD_NUM_BARS];
>  	struct pci_epf		*epf;
>  	enum pci_barno		test_reg_bar;
>  	struct delayed_work	cmd_handler;
> @@ -377,7 +377,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>  
>  	cancel_delayed_work(&epf_test->cmd_handler);
>  	pci_epc_stop(epc);
> -	for (bar = BAR_0; bar <= BAR_5; bar++) {
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>  		epf_bar = &epf->bar[bar];
>  
>  		if (epf_test->reg[bar]) {
> @@ -400,7 +400,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>  
>  	epc_features = epf_test->epc_features;
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar += add) {
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {

Is it possible to completely remove the BAR_x macros, or are there exsiting
users after this patchset?

As your patchset replaces BAR_0 with 0 and BAR_1 with 1, does this suggest
that other users of BAR_x should be removed and also replaced with a number?

Apologies if you this doesn't fall in the remit of this patchset.

Thanks,

Andrew Murray

>  		epf_bar = &epf->bar[bar];
>  		/*
>  		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
> @@ -450,7 +450,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
>  	}
>  	epf_test->reg[test_reg_bar] = base;
>  
> -	for (bar = BAR_0; bar <= BAR_5; bar += add) {
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
>  		epf_bar = &epf->bar[bar];
>  		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
>  
> @@ -478,7 +478,7 @@ static void pci_epf_configure_bar(struct pci_epf *epf,
>  	bool bar_fixed_64bit;
>  	int i;
>  
> -	for (i = BAR_0; i <= BAR_5; i++) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  		epf_bar = &epf->bar[i];
>  		bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i));
>  		if (bar_fixed_64bit)
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index f641badc2c61..56f1846b9d39 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -117,7 +117,7 @@ struct pci_epc_features {
>  	unsigned int	msix_capable : 1;
>  	u8	reserved_bar;
>  	u8	bar_fixed_64bit;
> -	u64	bar_fixed_size[BAR_5 + 1];
> +	u64	bar_fixed_size[PCI_STD_NUM_BARS];
>  	size_t	align;
>  };
>  
> -- 
> 2.21.0
> 

Re: [PATCH v3 04/26] PCI: endpoint: Use PCI_STD_NUM_BARS

[Denis Efremov]

On 9/18/19 12:19 PM, Andrew Murray wrote:
> On Mon, Sep 16, 2019 at 11:41:36PM +0300, Denis Efremov wrote:
>> To iterate through all possible BARs, loop conditions refactored to the
>> *number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
>> valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
>> the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
>> appropriate.
>>
>> Cc: Kishon Vijay Abraham I <kishon@ti.com>
>> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Signed-off-by: Denis Efremov <efremov@linux.com>
>> ---
>>  drivers/pci/endpoint/functions/pci-epf-test.c | 10 +++++-----
>>  include/linux/pci-epc.h                       |  2 +-
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>> index 1cfe3687a211..5d74f81ddfe4 100644
>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>> @@ -44,7 +44,7 @@
>>  static struct workqueue_struct *kpcitest_workqueue;
>>  
>>  struct pci_epf_test {
>> -	void			*reg[6];
>> +	void			*reg[PCI_STD_NUM_BARS];
>>  	struct pci_epf		*epf;
>>  	enum pci_barno		test_reg_bar;
>>  	struct delayed_work	cmd_handler;
>> @@ -377,7 +377,7 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>>  
>>  	cancel_delayed_work(&epf_test->cmd_handler);
>>  	pci_epc_stop(epc);
>> -	for (bar = BAR_0; bar <= BAR_5; bar++) {
>> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>>  		epf_bar = &epf->bar[bar];
>>  
>>  		if (epf_test->reg[bar]) {
>> @@ -400,7 +400,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>>  
>>  	epc_features = epf_test->epc_features;
>>  
>> -	for (bar = BAR_0; bar <= BAR_5; bar += add) {
>> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
> 
> Is it possible to completely remove the BAR_x macros, or are there exsiting
> users after this patchset?

They are still used in other parts of the code. So, I've decided to preserve
the defines in this case.

pci-epc-core.c
400:        (epf_bar->barno == BAR_5 &&
429:        (epf_bar->barno == BAR_5 &&
functions/pci-epf-test.c
497:    enum pci_barno test_reg_bar = BAR_0;

> 
> As your patchset replaces BAR_0 with 0 and BAR_1 with 1, does this suggest
> that other users of BAR_x should be removed and also replaced with a number?

I changed BAR_0 to 0 in order to not mix different notions, i.e. the number
of bars and the concrete bar.

> 
> Apologies if you this doesn't fall in the remit of this patchset.

I don't know what is better here. It's simple enough to remove these defines.
However, I would prefer to wait for the endpoint developers opinion.

Thanks for the review!
Denis

> 
> Thanks,
> 
> Andrew Murray
> 
>>  		epf_bar = &epf->bar[bar];
>>  		/*
>>  		 * pci_epc_set_bar() sets PCI_BASE_ADDRESS_MEM_TYPE_64
>> @@ -450,7 +450,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>  	}
>>  	epf_test->reg[test_reg_bar] = base;
>>  
>> -	for (bar = BAR_0; bar <= BAR_5; bar += add) {
>> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
>>  		epf_bar = &epf->bar[bar];
>>  		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
>>  
>> @@ -478,7 +478,7 @@ static void pci_epf_configure_bar(struct pci_epf *epf,
>>  	bool bar_fixed_64bit;
>>  	int i;
>>  
>> -	for (i = BAR_0; i <= BAR_5; i++) {
>> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>>  		epf_bar = &epf->bar[i];
>>  		bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i));
>>  		if (bar_fixed_64bit)
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index f641badc2c61..56f1846b9d39 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -117,7 +117,7 @@ struct pci_epc_features {
>>  	unsigned int	msix_capable : 1;
>>  	u8	reserved_bar;
>>  	u8	bar_fixed_64bit;
>> -	u64	bar_fixed_size[BAR_5 + 1];
>> +	u64	bar_fixed_size[PCI_STD_NUM_BARS];
>>  	size_t	align;
>>  };
>>  
>> -- 
>> 2.21.0
>>

[PATCH v3 05/26] misc: pci_endpoint_test: Use PCI_STD_NUM_BARS

[Denis Efremov]

To iterate through all possible BARs, loop conditions refactored to the
*number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
the fencepost error. Array definitions changed to PCI_STD_NUM_BARS where
appropriate.

Cc: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/misc/pci_endpoint_test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 6e208a060a58..a5e317073d95 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -94,7 +94,7 @@ enum pci_barno {
 struct pci_endpoint_test {
 	struct pci_dev	*pdev;
 	void __iomem	*base;
-	void __iomem	*bar[6];
+	void __iomem	*bar[PCI_STD_NUM_BARS];
 	struct completion irq_raised;
 	int		last_irq;
 	int		num_irqs;
@@ -687,7 +687,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	if (!pci_endpoint_test_request_irq(test))
 		goto err_disable_irq;
 
-	for (bar = BAR_0; bar <= BAR_5; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
 			base = pci_ioremap_bar(pdev, bar);
 			if (!base) {
@@ -740,7 +740,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	ida_simple_remove(&pci_endpoint_test_ida, id);
 
 err_iounmap:
-	for (bar = BAR_0; bar <= BAR_5; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		if (test->bar[bar])
 			pci_iounmap(pdev, test->bar[bar]);
 	}
@@ -771,7 +771,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev)
 	misc_deregister(&test->miscdev);
 	kfree(misc_device->name);
 	ida_simple_remove(&pci_endpoint_test_ida, id);
-	for (bar = BAR_0; bar <= BAR_5; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		if (test->bar[bar])
 			pci_iounmap(pdev, test->bar[bar]);
 	}
-- 
2.21.0

[PATCH v3 06/26] s390/pci: Use PCI_STD_NUM_BARS

[Denis Efremov]

Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use
global one PCI_STD_NUM_BARS instead.

Acked-by: Sebastian Ott <sebott@linux.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 arch/s390/include/asm/pci.h     |  5 +----
 arch/s390/include/asm/pci_clp.h |  6 +++---
 arch/s390/pci/pci.c             | 16 ++++++++--------
 arch/s390/pci/pci_clp.c         |  6 +++---
 4 files changed, 15 insertions(+), 18 deletions(-)

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

Re: [PATCH v3 06/26] s390/pci: Use PCI_STD_NUM_BARS

[Andrew Murray]

On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote:
> Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use
> global one PCI_STD_NUM_BARS instead.
> 
> Acked-by: Sebastian Ott <sebott@linux.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  arch/s390/include/asm/pci.h     |  5 +----
>  arch/s390/include/asm/pci_clp.h |  6 +++---
>  arch/s390/pci/pci.c             | 16 ++++++++--------
>  arch/s390/pci/pci_clp.c         |  6 +++---
>  4 files changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> index a2399eff84ca..3a06c264ea53 100644
> --- a/arch/s390/include/asm/pci.h
> +++ b/arch/s390/include/asm/pci.h
> @@ -2,9 +2,6 @@
>  #ifndef __ASM_S390_PCI_H
>  #define __ASM_S390_PCI_H
>  
> -/* must be set before including pci_clp.h */
> -#define PCI_BAR_COUNT	6
> -
>  #include <linux/pci.h>
>  #include <linux/mutex.h>
>  #include <linux/iommu.h>
> @@ -138,7 +135,7 @@ struct zpci_dev {
>  
>  	char res_name[16];
>  	bool mio_capable;
> -	struct zpci_bar_struct bars[PCI_BAR_COUNT];
> +	struct zpci_bar_struct bars[PCI_STD_NUM_BARS];
>  
>  	u64		start_dma;	/* Start of available DMA addresses */
>  	u64		end_dma;	/* End of available DMA addresses */
> diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
> index 50359172cc48..bd2cb4ea7d93 100644
> --- a/arch/s390/include/asm/pci_clp.h
> +++ b/arch/s390/include/asm/pci_clp.h
> @@ -77,7 +77,7 @@ struct mio_info {
>  	struct {
>  		u64 wb;
>  		u64 wt;
> -	} addr[PCI_BAR_COUNT];
> +	} addr[PCI_STD_NUM_BARS];
>  	u32 reserved[6];
>  } __packed;
>  
> @@ -98,9 +98,9 @@ struct clp_rsp_query_pci {
>  	u16 util_str_avail	:  1;	/* utility string available? */
>  	u16 pfgid		:  8;	/* pci function group id */
>  	u32 fid;			/* pci function id */
> -	u8 bar_size[PCI_BAR_COUNT];
> +	u8 bar_size[PCI_STD_NUM_BARS];
>  	u16 pchid;
> -	__le32 bar[PCI_BAR_COUNT];
> +	__le32 bar[PCI_STD_NUM_BARS];
>  	u8 pfip[CLP_PFIP_NR_SEGMENTS];	/* pci function internal path */
>  	u32			: 16;
>  	u8 fmb_len;
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index b0e3b9a0e488..aca372c8e34f 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
>  static DEFINE_SPINLOCK(zpci_domain_lock);
>  
>  #define ZPCI_IOMAP_ENTRIES						\
> -	min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),	\
> +	min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2),	\
>  	    ZPCI_IOMAP_MAX_ENTRIES)
>  
>  static DEFINE_SPINLOCK(zpci_iomap_lock);
> @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar,
>  void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar,
>  			      unsigned long offset, unsigned long max)
>  {
> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
>  		return NULL;
>  
>  	if (static_branch_likely(&have_mio))
> @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar,
>  void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar,
>  				 unsigned long offset, unsigned long max)
>  {
> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
>  		return NULL;

This looks like a latent bug fix here. If 'bar' is out of range we return
NULL instead accessing an invalid item of an array. Should this not be
a separate patch and tagged as stable?

Thanks,

Andrew Murray

>  
>  	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
> 

Re: [PATCH v3 06/26] s390/pci: Use PCI_STD_NUM_BARS

[Denis Efremov]

On 9/18/19 11:58 AM, Andrew Murray wrote:
> On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote:
>> Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use
>> global one PCI_STD_NUM_BARS instead.
>>
>> Acked-by: Sebastian Ott <sebott@linux.ibm.com>
>> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
>> Signed-off-by: Denis Efremov <efremov@linux.com>
>> ---
>>  arch/s390/include/asm/pci.h     |  5 +----
>>  arch/s390/include/asm/pci_clp.h |  6 +++---
>>  arch/s390/pci/pci.c             | 16 ++++++++--------
>>  arch/s390/pci/pci_clp.c         |  6 +++---
>>  4 files changed, 15 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
>> index a2399eff84ca..3a06c264ea53 100644
>> --- a/arch/s390/include/asm/pci.h
>> +++ b/arch/s390/include/asm/pci.h
>> @@ -2,9 +2,6 @@
>>  #ifndef __ASM_S390_PCI_H
>>  #define __ASM_S390_PCI_H
>>  
>> -/* must be set before including pci_clp.h */
>> -#define PCI_BAR_COUNT	6
>> -
>>  #include <linux/pci.h>
>>  #include <linux/mutex.h>
>>  #include <linux/iommu.h>
>> @@ -138,7 +135,7 @@ struct zpci_dev {
>>  
>>  	char res_name[16];
>>  	bool mio_capable;
>> -	struct zpci_bar_struct bars[PCI_BAR_COUNT];
>> +	struct zpci_bar_struct bars[PCI_STD_NUM_BARS];
>>  
>>  	u64		start_dma;	/* Start of available DMA addresses */
>>  	u64		end_dma;	/* End of available DMA addresses */
>> diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
>> index 50359172cc48..bd2cb4ea7d93 100644
>> --- a/arch/s390/include/asm/pci_clp.h
>> +++ b/arch/s390/include/asm/pci_clp.h
>> @@ -77,7 +77,7 @@ struct mio_info {
>>  	struct {
>>  		u64 wb;
>>  		u64 wt;
>> -	} addr[PCI_BAR_COUNT];
>> +	} addr[PCI_STD_NUM_BARS];
>>  	u32 reserved[6];
>>  } __packed;
>>  
>> @@ -98,9 +98,9 @@ struct clp_rsp_query_pci {
>>  	u16 util_str_avail	:  1;	/* utility string available? */
>>  	u16 pfgid		:  8;	/* pci function group id */
>>  	u32 fid;			/* pci function id */
>> -	u8 bar_size[PCI_BAR_COUNT];
>> +	u8 bar_size[PCI_STD_NUM_BARS];
>>  	u16 pchid;
>> -	__le32 bar[PCI_BAR_COUNT];
>> +	__le32 bar[PCI_STD_NUM_BARS];
>>  	u8 pfip[CLP_PFIP_NR_SEGMENTS];	/* pci function internal path */
>>  	u32			: 16;
>>  	u8 fmb_len;
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index b0e3b9a0e488..aca372c8e34f 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
>>  static DEFINE_SPINLOCK(zpci_domain_lock);
>>  
>>  #define ZPCI_IOMAP_ENTRIES						\
>> -	min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),	\
>> +	min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2),	\
>>  	    ZPCI_IOMAP_MAX_ENTRIES)
>>  
>>  static DEFINE_SPINLOCK(zpci_iomap_lock);
>> @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar,
>>  void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar,
>>  			      unsigned long offset, unsigned long max)
>>  {
>> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
>> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
>>  		return NULL;
>>  
>>  	if (static_branch_likely(&have_mio))
>> @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar,
>>  void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar,
>>  				 unsigned long offset, unsigned long max)
>>  {
>> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
>> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
>>  		return NULL;
> 
> This looks like a latent bug fix here. If 'bar' is out of range we return
> NULL instead accessing an invalid item of an array. Should this not be
> a separate patch and tagged as stable?
> 

This fix was suggested by Bjorn in v1 review:
https://lkml.org/lkml/2019/8/12/997


> Thanks,
> 
> Andrew Murray
> 
>>  
>>  	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
>>

Re: [PATCH v3 06/26] s390/pci: Use PCI_STD_NUM_BARS

[Andrew Murray]

On Wed, Sep 18, 2019 at 05:26:59PM +0300, Denis Efremov wrote:
> On 9/18/19 11:58 AM, Andrew Murray wrote:
> > On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote:
> >> Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use
> >> global one PCI_STD_NUM_BARS instead.
> >>
> >> Acked-by: Sebastian Ott <sebott@linux.ibm.com>
> >> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> >> Signed-off-by: Denis Efremov <efremov@linux.com>
> >> ---
> >>  arch/s390/include/asm/pci.h     |  5 +----
> >>  arch/s390/include/asm/pci_clp.h |  6 +++---
> >>  arch/s390/pci/pci.c             | 16 ++++++++--------
> >>  arch/s390/pci/pci_clp.c         |  6 +++---
> >>  4 files changed, 15 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> >> index a2399eff84ca..3a06c264ea53 100644
> >> --- a/arch/s390/include/asm/pci.h
> >> +++ b/arch/s390/include/asm/pci.h
> >> @@ -2,9 +2,6 @@
> >>  #ifndef __ASM_S390_PCI_H
> >>  #define __ASM_S390_PCI_H
> >>  
> >> -/* must be set before including pci_clp.h */
> >> -#define PCI_BAR_COUNT	6
> >> -
> >>  #include <linux/pci.h>
> >>  #include <linux/mutex.h>
> >>  #include <linux/iommu.h>
> >> @@ -138,7 +135,7 @@ struct zpci_dev {
> >>  
> >>  	char res_name[16];
> >>  	bool mio_capable;
> >> -	struct zpci_bar_struct bars[PCI_BAR_COUNT];
> >> +	struct zpci_bar_struct bars[PCI_STD_NUM_BARS];
> >>  
> >>  	u64		start_dma;	/* Start of available DMA addresses */
> >>  	u64		end_dma;	/* End of available DMA addresses */
> >> diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
> >> index 50359172cc48..bd2cb4ea7d93 100644
> >> --- a/arch/s390/include/asm/pci_clp.h
> >> +++ b/arch/s390/include/asm/pci_clp.h
> >> @@ -77,7 +77,7 @@ struct mio_info {
> >>  	struct {
> >>  		u64 wb;
> >>  		u64 wt;
> >> -	} addr[PCI_BAR_COUNT];
> >> +	} addr[PCI_STD_NUM_BARS];
> >>  	u32 reserved[6];
> >>  } __packed;
> >>  
> >> @@ -98,9 +98,9 @@ struct clp_rsp_query_pci {
> >>  	u16 util_str_avail	:  1;	/* utility string available? */
> >>  	u16 pfgid		:  8;	/* pci function group id */
> >>  	u32 fid;			/* pci function id */
> >> -	u8 bar_size[PCI_BAR_COUNT];
> >> +	u8 bar_size[PCI_STD_NUM_BARS];
> >>  	u16 pchid;
> >> -	__le32 bar[PCI_BAR_COUNT];
> >> +	__le32 bar[PCI_STD_NUM_BARS];
> >>  	u8 pfip[CLP_PFIP_NR_SEGMENTS];	/* pci function internal path */
> >>  	u32			: 16;
> >>  	u8 fmb_len;
> >> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> >> index b0e3b9a0e488..aca372c8e34f 100644
> >> --- a/arch/s390/pci/pci.c
> >> +++ b/arch/s390/pci/pci.c
> >> @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
> >>  static DEFINE_SPINLOCK(zpci_domain_lock);
> >>  
> >>  #define ZPCI_IOMAP_ENTRIES						\
> >> -	min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),	\
> >> +	min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2),	\
> >>  	    ZPCI_IOMAP_MAX_ENTRIES)
> >>  
> >>  static DEFINE_SPINLOCK(zpci_iomap_lock);
> >> @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar,
> >>  void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar,
> >>  			      unsigned long offset, unsigned long max)
> >>  {
> >> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> >> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
> >>  		return NULL;
> >>  
> >>  	if (static_branch_likely(&have_mio))
> >> @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar,
> >>  void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar,
> >>  				 unsigned long offset, unsigned long max)
> >>  {
> >> -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> >> +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
> >>  		return NULL;
> > 
> > This looks like a latent bug fix here. If 'bar' is out of range we return
> > NULL instead accessing an invalid item of an array. Should this not be
> > a separate patch and tagged as stable?
> > 
> 
> This fix was suggested by Bjorn in v1 review:
> https://lkml.org/lkml/2019/8/12/997
> 

Ah yes, apologies - I'll re-read the previous threads next time.

Thanks,

Andrew Murray

> 
> > Thanks,
> > 
> > Andrew Murray
> > 
> >>  
> >>  	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
> >>
> 

Re: [PATCH v3 06/26] s390/pci: Use PCI_STD_NUM_BARS

[Bjorn Helgaas]

On Wed, Sep 18, 2019 at 09:58:06AM +0100, Andrew Murray wrote:
> On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote:
> > Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use
> > global one PCI_STD_NUM_BARS instead.
> > 
> > Acked-by: Sebastian Ott <sebott@linux.ibm.com>
> > Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com>
> > Signed-off-by: Denis Efremov <efremov@linux.com>
> > ---
> >  arch/s390/include/asm/pci.h     |  5 +----
> >  arch/s390/include/asm/pci_clp.h |  6 +++---
> >  arch/s390/pci/pci.c             | 16 ++++++++--------
> >  arch/s390/pci/pci_clp.c         |  6 +++---
> >  4 files changed, 15 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> > index a2399eff84ca..3a06c264ea53 100644
> > --- a/arch/s390/include/asm/pci.h
> > +++ b/arch/s390/include/asm/pci.h
> > @@ -2,9 +2,6 @@
> >  #ifndef __ASM_S390_PCI_H
> >  #define __ASM_S390_PCI_H
> >  
> > -/* must be set before including pci_clp.h */
> > -#define PCI_BAR_COUNT	6
> > -
> >  #include <linux/pci.h>
> >  #include <linux/mutex.h>
> >  #include <linux/iommu.h>
> > @@ -138,7 +135,7 @@ struct zpci_dev {
> >  
> >  	char res_name[16];
> >  	bool mio_capable;
> > -	struct zpci_bar_struct bars[PCI_BAR_COUNT];
> > +	struct zpci_bar_struct bars[PCI_STD_NUM_BARS];
> >  
> >  	u64		start_dma;	/* Start of available DMA addresses */
> >  	u64		end_dma;	/* End of available DMA addresses */
> > diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h
> > index 50359172cc48..bd2cb4ea7d93 100644
> > --- a/arch/s390/include/asm/pci_clp.h
> > +++ b/arch/s390/include/asm/pci_clp.h
> > @@ -77,7 +77,7 @@ struct mio_info {
> >  	struct {
> >  		u64 wb;
> >  		u64 wt;
> > -	} addr[PCI_BAR_COUNT];
> > +	} addr[PCI_STD_NUM_BARS];
> >  	u32 reserved[6];
> >  } __packed;
> >  
> > @@ -98,9 +98,9 @@ struct clp_rsp_query_pci {
> >  	u16 util_str_avail	:  1;	/* utility string available? */
> >  	u16 pfgid		:  8;	/* pci function group id */
> >  	u32 fid;			/* pci function id */
> > -	u8 bar_size[PCI_BAR_COUNT];
> > +	u8 bar_size[PCI_STD_NUM_BARS];
> >  	u16 pchid;
> > -	__le32 bar[PCI_BAR_COUNT];
> > +	__le32 bar[PCI_STD_NUM_BARS];
> >  	u8 pfip[CLP_PFIP_NR_SEGMENTS];	/* pci function internal path */
> >  	u32			: 16;
> >  	u8 fmb_len;
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index b0e3b9a0e488..aca372c8e34f 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
> >  static DEFINE_SPINLOCK(zpci_domain_lock);
> >  
> >  #define ZPCI_IOMAP_ENTRIES						\
> > -	min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),	\
> > +	min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2),	\
> >  	    ZPCI_IOMAP_MAX_ENTRIES)
> >  
> >  static DEFINE_SPINLOCK(zpci_iomap_lock);
> > @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar,
> >  void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar,
> >  			      unsigned long offset, unsigned long max)
> >  {
> > -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> > +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
> >  		return NULL;
> >  
> >  	if (static_branch_likely(&have_mio))
> > @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar,
> >  void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar,
> >  				 unsigned long offset, unsigned long max)
> >  {
> > -	if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT)
> > +	if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar))
> >  		return NULL;
> 
> This looks like a latent bug fix here. If 'bar' is out of range we return
> NULL instead accessing an invalid item of an array. Should this not be
> a separate patch and tagged as stable?

Sharp eyes!  I didn't think of this as accessing an invalid item, but
indeed it does (if 'bar' is out of range).  But I doubt it's worth the
hassle of a separate patch, since we return failure anyway.

[PATCH v3 07/26] x86/PCI: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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.

Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 arch/x86/pci/common.c        | 2 +-
 arch/x86/pci/intel_mid_pci.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

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;
diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index 43867bc85368..00c62115f39c 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -382,7 +382,7 @@ static void pci_fixed_bar_fixup(struct pci_dev *dev)
 	    PCI_DEVFN(2, 2) == dev->devfn)
 		return;
 
-	for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		pci_read_config_dword(dev, offset + 8 + (i * 4), &size);
 		dev->resource[i].end = dev->resource[i].start + size - 1;
 		dev->resource[i].flags |= IORESOURCE_PCI_FIXED;
-- 
2.21.0

[PATCH v3 08/26] alpha/PCI: Use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 arch/alpha/kernel/pci-sysfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/alpha/kernel/pci-sysfs.c b/arch/alpha/kernel/pci-sysfs.c
index f94c732fedeb..0021580d79ad 100644
--- a/arch/alpha/kernel/pci-sysfs.c
+++ b/arch/alpha/kernel/pci-sysfs.c
@@ -71,10 +71,10 @@ static int pci_mmap_resource(struct kobject *kobj,
 	struct pci_bus_region bar;
 	int i;
 
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		if (res == &pdev->resource[i])
 			break;
-	if (i >= PCI_ROM_RESOURCE)
+	if (i >= PCI_STD_NUM_BARS)
 		return -ENODEV;
 
 	if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
@@ -115,7 +115,7 @@ 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];
@@ -232,7 +232,7 @@ 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))
-- 
2.21.0

[PATCH v3 09/26] ia64: Use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 arch/ia64/sn/pci/pcibr/pcibr_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/sn/pci/pcibr/pcibr_dma.c b/arch/ia64/sn/pci/pcibr/pcibr_dma.c
index 1e863b277ac9..ff981e415a28 100644
--- a/arch/ia64/sn/pci/pcibr/pcibr_dma.c
+++ b/arch/ia64/sn/pci/pcibr/pcibr_dma.c
@@ -295,14 +295,14 @@ void sn_dma_flush(u64 addr)
 	/* find a matching BAR */
 	for (i = 0; i < DEV_PER_WIDGET; i++,p++) {
 		common = p->common;
-		for (j = 0; j < PCI_ROM_RESOURCE; j++) {
+		for (j = 0; j < PCI_STD_NUM_BARS; j++) {
 			if (common->sfdl_bar_list[j].start == 0)
 				break;
 			if (addr >= common->sfdl_bar_list[j].start
 			    && addr <= common->sfdl_bar_list[j].end)
 				break;
 		}
-		if (j < PCI_ROM_RESOURCE && common->sfdl_bar_list[j].start != 0)
+		if (j < PCI_STD_NUM_BARS && common->sfdl_bar_list[j].start != 0)
 			break;
 	}
 
-- 
2.21.0

[PATCH v3 10/26] stmmac: pci: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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".

Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[PATCH v3 11/26] net: dwc-xlgmac: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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".

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

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

[PATCH v3 12/26] ixgb: use PCI_STD_NUM_BARS

[Denis Efremov]

To iterate through all possible BARs, loop conditions refactored to the
*number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
the fencepost error.

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/intel/ixgb/ixgb.h      | 1 -
 drivers/net/ethernet/intel/ixgb/ixgb_main.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgb/ixgb.h b/drivers/net/ethernet/intel/ixgb/ixgb.h
index e85271b68410..681d44cc9784 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb.h
+++ b/drivers/net/ethernet/intel/ixgb/ixgb.h
@@ -42,7 +42,6 @@
 
 #define BAR_0		0
 #define BAR_1		1
-#define BAR_5		5
 
 struct ixgb_adapter;
 #include "ixgb_hw.h"
diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index e5ac2d3fd816..c5bba18eb32a 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -412,7 +412,7 @@ ixgb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_ioremap;
 	}
 
-	for (i = BAR_1; i <= BAR_5; i++) {
+	for (i = BAR_1; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_len(pdev, i) == 0)
 			continue;
 		if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
-- 
2.21.0

[PATCH v3 13/26] e1000: Use PCI_STD_NUM_BARS

[Denis Efremov]

To iterate through all possible BARs, loop conditions refactored to the
*number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
the fencepost error.

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/net/ethernet/intel/e1000/e1000.h      | 1 -
 drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
index c40729b2c184..7fad2f24dcad 100644
--- a/drivers/net/ethernet/intel/e1000/e1000.h
+++ b/drivers/net/ethernet/intel/e1000/e1000.h
@@ -45,7 +45,6 @@
 
 #define BAR_0		0
 #define BAR_1		1
-#define BAR_5		5
 
 #define INTEL_E1000_ETHERNET_DEVICE(device_id) {\
 	PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index f703fa58458e..db4fd82036af 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -977,7 +977,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_ioremap;
 
 	if (adapter->need_ioport) {
-		for (i = BAR_1; i <= BAR_5; i++) {
+		for (i = BAR_1; i < PCI_STD_NUM_BARS; i++) {
 			if (pci_resource_len(pdev, i) == 0)
 				continue;
 			if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
-- 
2.21.0

Re: [PATCH v3 13/26] e1000: Use PCI_STD_NUM_BARS

[Andrew Murray]

On Mon, Sep 16, 2019 at 11:41:45PM +0300, Denis Efremov wrote:
> To iterate through all possible BARs, loop conditions refactored to the
> *number* of BARs "i < PCI_STD_NUM_BARS", instead of the index of the last
> valid BAR "i <= BAR_5". This is more idiomatic C style and allows to avoid
> the fencepost error.
> 
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/net/ethernet/intel/e1000/e1000.h      | 1 -
>  drivers/net/ethernet/intel/e1000/e1000_main.c | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000/e1000.h b/drivers/net/ethernet/intel/e1000/e1000.h
> index c40729b2c184..7fad2f24dcad 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000.h
> +++ b/drivers/net/ethernet/intel/e1000/e1000.h
> @@ -45,7 +45,6 @@
>  
>  #define BAR_0		0
>  #define BAR_1		1
> -#define BAR_5		5

No issue with this patch. However I noticed that at least 5 of the network
drivers have these same definitions, which are identical to the pci_barno enum
of include/linux/pci-epf.h. There are mostly used with pci_ioremap_bar and
pci_resource_** macros. I wonder if this is an indicator that these defintions
should live in the core.

Thanks,

Andrew Murray

>  
>  #define INTEL_E1000_ETHERNET_DEVICE(device_id) {\
>  	PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index f703fa58458e..db4fd82036af 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -977,7 +977,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		goto err_ioremap;
>  
>  	if (adapter->need_ioport) {
> -		for (i = BAR_1; i <= BAR_5; i++) {
> +		for (i = BAR_1; i < PCI_STD_NUM_BARS; i++) {
>  			if (pci_resource_len(pdev, i) == 0)
>  				continue;
>  			if (pci_resource_flags(pdev, i) & IORESOURCE_IO) {
> -- 
> 2.21.0
> 

[PATCH v3 14/26] rapidio/tsi721: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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".

Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Alexandre Bounine <alex.bou9@gmail.com>
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

[PATCH v3 15/26] efifb: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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".

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Peter Jones <pjones@redhat.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/video/fbdev/efifb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[PATCH v3 16/26] fbmem: use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/video/fbdev/core/fbmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 64dd732021d8..485e798cd404 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1784,7 +1784,7 @@ int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id, const
 	int err, idx, bar;
 	bool res_id_found = false;
 
-	for (idx = 0, bar = 0; bar < PCI_ROM_RESOURCE; bar++) {
+	for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
 			continue;
 		idx++;
@@ -1794,7 +1794,7 @@ int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, int res_id, const
 	if (!ap)
 		return -ENOMEM;
 
-	for (idx = 0, bar = 0; bar < PCI_ROM_RESOURCE; bar++) {
+	for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
 			continue;
 		ap->ranges[idx].base = pci_resource_start(pdev, bar);
-- 
2.21.0

[PATCH v3 17/26] vfio_pci: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

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

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".

Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/vfio/pci/vfio_pci.c         | 11 ++++++----
 drivers/vfio/pci/vfio_pci_config.c  | 32 +++++++++++++++--------------
 drivers/vfio/pci/vfio_pci_private.h |  4 ++--
 3 files changed, 26 insertions(+), 21 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..90c0b80f8acf 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -450,30 +450,32 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
 {
 	struct pci_dev *pdev = vdev->pdev;
 	int i;
-	__le32 *bar;
+	__le32 *vbar;
 	u64 mask;
 
-	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
+	vbar = (__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)) {
-			*bar = 0; /* Unmapped by host = unimplemented to user */
+	for (i = 0; i < PCI_STD_NUM_BARS; i++, vbar++) {
+		int bar = i + PCI_STD_RESOURCES;
+
+		if (!pci_resource_start(pdev, bar)) {
+			*vbar = 0; /* Unmapped by host = unimplemented to user */
 			continue;
 		}
 
-		mask = ~(pci_resource_len(pdev, i) - 1);
+		mask = ~(pci_resource_len(pdev, bar) - 1);
 
-		*bar &= cpu_to_le32((u32)mask);
-		*bar |= vfio_generate_bar_flags(pdev, i);
+		*vbar &= cpu_to_le32((u32)mask);
+		*vbar |= vfio_generate_bar_flags(pdev, bar);
 
-		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
-			bar++;
-			*bar &= cpu_to_le32((u32)(mask >> 32));
+		if (*vbar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
+			vbar++;
+			*vbar &= cpu_to_le32((u32)(mask >> 32));
 			i++;
 		}
 	}
 
-	bar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
+	vbar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
 
 	/*
 	 * NB. REGION_INFO will have reported zero size if we weren't able
@@ -483,14 +485,14 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
 	if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
 		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
 		mask |= PCI_ROM_ADDRESS_ENABLE;
-		*bar &= cpu_to_le32((u32)mask);
+		*vbar &= cpu_to_le32((u32)mask);
 	} else if (pdev->resource[PCI_ROM_RESOURCE].flags &
 					IORESOURCE_ROM_SHADOW) {
 		mask = ~(0x20000 - 1);
 		mask |= PCI_ROM_ADDRESS_ENABLE;
-		*bar &= cpu_to_le32((u32)mask);
+		*vbar &= cpu_to_le32((u32)mask);
 	} else
-		*bar = 0;
+		*vbar = 0;
 
 	vdev->bardirty = false;
 }
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

Re: [PATCH v3 17/26] vfio_pci: Loop using PCI_STD_NUM_BARS

[Andrew Murray]

On Mon, Sep 16, 2019 at 11:41:49PM +0300, Denis Efremov wrote:
> Refactor loops to use idiomatic C style and avoid the fencepost error
> of using "i < PCI_STD_RESOURCE_END" when "i <= PCI_STD_RESOURCE_END"
> is required, e.g., commit 2f686f1d9bee ("PCI: Correct PCI_STD_RESOURCE_END
> usage").
> 
> 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".
> 
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/vfio/pci/vfio_pci.c         | 11 ++++++----
>  drivers/vfio/pci/vfio_pci_config.c  | 32 +++++++++++++++--------------
>  drivers/vfio/pci/vfio_pci_private.h |  4 ++--
>  3 files changed, 26 insertions(+), 21 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];

Why can't we just drop PCI_STD_RESOURCES and replace it was 0. I understand
the abstraction here, but we don't do it elsewhere across the kernel. Is this
necessary?

Thanks,

Andrew Murray

>  
>  		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..90c0b80f8acf 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
> @@ -450,30 +450,32 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
>  {
>  	struct pci_dev *pdev = vdev->pdev;
>  	int i;
> -	__le32 *bar;
> +	__le32 *vbar;
>  	u64 mask;
>  
> -	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
> +	vbar = (__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)) {
> -			*bar = 0; /* Unmapped by host = unimplemented to user */
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++, vbar++) {
> +		int bar = i + PCI_STD_RESOURCES;
> +
> +		if (!pci_resource_start(pdev, bar)) {
> +			*vbar = 0; /* Unmapped by host = unimplemented to user */
>  			continue;
>  		}
>  
> -		mask = ~(pci_resource_len(pdev, i) - 1);
> +		mask = ~(pci_resource_len(pdev, bar) - 1);
>  
> -		*bar &= cpu_to_le32((u32)mask);
> -		*bar |= vfio_generate_bar_flags(pdev, i);
> +		*vbar &= cpu_to_le32((u32)mask);
> +		*vbar |= vfio_generate_bar_flags(pdev, bar);
>  
> -		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> -			bar++;
> -			*bar &= cpu_to_le32((u32)(mask >> 32));
> +		if (*vbar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> +			vbar++;
> +			*vbar &= cpu_to_le32((u32)(mask >> 32));
>  			i++;
>  		}
>  	}
>  
> -	bar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
> +	vbar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
>  
>  	/*
>  	 * NB. REGION_INFO will have reported zero size if we weren't able
> @@ -483,14 +485,14 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
>  	if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
>  		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
>  		mask |= PCI_ROM_ADDRESS_ENABLE;
> -		*bar &= cpu_to_le32((u32)mask);
> +		*vbar &= cpu_to_le32((u32)mask);
>  	} else if (pdev->resource[PCI_ROM_RESOURCE].flags &
>  					IORESOURCE_ROM_SHADOW) {
>  		mask = ~(0x20000 - 1);
>  		mask |= PCI_ROM_ADDRESS_ENABLE;
> -		*bar &= cpu_to_le32((u32)mask);
> +		*vbar &= cpu_to_le32((u32)mask);
>  	} else
> -		*bar = 0;
> +		*vbar = 0;
>  
>  	vdev->bardirty = false;
>  }
> 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
> 

Re: [PATCH v3 17/26] vfio_pci: Loop using PCI_STD_NUM_BARS

[Denis Efremov]

On 9/18/19 12:17 PM, Andrew Murray wrote:
> On Mon, Sep 16, 2019 at 11:41:49PM +0300, Denis Efremov wrote:
>> Refactor loops to use idiomatic C style and avoid the fencepost error
>> of using "i < PCI_STD_RESOURCE_END" when "i <= PCI_STD_RESOURCE_END"
>> is required, e.g., commit 2f686f1d9bee ("PCI: Correct PCI_STD_RESOURCE_END
>> usage").
>>
>> 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".
>>
>> Cc: Cornelia Huck <cohuck@redhat.com>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Signed-off-by: Denis Efremov <efremov@linux.com>
>> ---
>>  drivers/vfio/pci/vfio_pci.c         | 11 ++++++----
>>  drivers/vfio/pci/vfio_pci_config.c  | 32 +++++++++++++++--------------
>>  drivers/vfio/pci/vfio_pci_private.h |  4 ++--
>>  3 files changed, 26 insertions(+), 21 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];
> 
> Why can't we just drop PCI_STD_RESOURCES and replace it was 0. I understand
> the abstraction here, but we don't do it elsewhere across the kernel. Is this
> necessary?

There was a discussion about this particular case:
https://lkml.org/lkml/2019/8/12/999

It was decided to save the original style for vfio drivers.

> 
> Thanks,
> 
> Andrew Murray
> 
>>  
>>  		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..90c0b80f8acf 100644
>> --- a/drivers/vfio/pci/vfio_pci_config.c
>> +++ b/drivers/vfio/pci/vfio_pci_config.c
>> @@ -450,30 +450,32 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
>>  {
>>  	struct pci_dev *pdev = vdev->pdev;
>>  	int i;
>> -	__le32 *bar;
>> +	__le32 *vbar;
>>  	u64 mask;
>>  
>> -	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
>> +	vbar = (__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)) {
>> -			*bar = 0; /* Unmapped by host = unimplemented to user */
>> +	for (i = 0; i < PCI_STD_NUM_BARS; i++, vbar++) {
>> +		int bar = i + PCI_STD_RESOURCES;
>> +
>> +		if (!pci_resource_start(pdev, bar)) {
>> +			*vbar = 0; /* Unmapped by host = unimplemented to user */
>>  			continue;
>>  		}
>>  
>> -		mask = ~(pci_resource_len(pdev, i) - 1);
>> +		mask = ~(pci_resource_len(pdev, bar) - 1);
>>  
>> -		*bar &= cpu_to_le32((u32)mask);
>> -		*bar |= vfio_generate_bar_flags(pdev, i);
>> +		*vbar &= cpu_to_le32((u32)mask);
>> +		*vbar |= vfio_generate_bar_flags(pdev, bar);
>>  
>> -		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
>> -			bar++;
>> -			*bar &= cpu_to_le32((u32)(mask >> 32));
>> +		if (*vbar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
>> +			vbar++;
>> +			*vbar &= cpu_to_le32((u32)(mask >> 32));
>>  			i++;
>>  		}
>>  	}
>>  
>> -	bar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
>> +	vbar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
>>  
>>  	/*
>>  	 * NB. REGION_INFO will have reported zero size if we weren't able
>> @@ -483,14 +485,14 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
>>  	if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
>>  		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
>>  		mask |= PCI_ROM_ADDRESS_ENABLE;
>> -		*bar &= cpu_to_le32((u32)mask);
>> +		*vbar &= cpu_to_le32((u32)mask);
>>  	} else if (pdev->resource[PCI_ROM_RESOURCE].flags &
>>  					IORESOURCE_ROM_SHADOW) {
>>  		mask = ~(0x20000 - 1);
>>  		mask |= PCI_ROM_ADDRESS_ENABLE;
>> -		*bar &= cpu_to_le32((u32)mask);
>> +		*vbar &= cpu_to_le32((u32)mask);
>>  	} else
>> -		*bar = 0;
>> +		*vbar = 0;
>>  
>>  	vdev->bardirty = false;
>>  }
>> 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
>>

Re: [PATCH v3 17/26] vfio_pci: Loop using PCI_STD_NUM_BARS

[Andrew Murray]

On Wed, Sep 18, 2019 at 05:31:33PM +0300, Denis Efremov wrote:
> On 9/18/19 12:17 PM, Andrew Murray wrote:
> > On Mon, Sep 16, 2019 at 11:41:49PM +0300, Denis Efremov wrote:
> >> Refactor loops to use idiomatic C style and avoid the fencepost error
> >> of using "i < PCI_STD_RESOURCE_END" when "i <= PCI_STD_RESOURCE_END"
> >> is required, e.g., commit 2f686f1d9bee ("PCI: Correct PCI_STD_RESOURCE_END
> >> usage").
> >>
> >> 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".
> >>
> >> Cc: Cornelia Huck <cohuck@redhat.com>
> >> Cc: Alex Williamson <alex.williamson@redhat.com>
> >> Signed-off-by: Denis Efremov <efremov@linux.com>
> >> ---
> >>  drivers/vfio/pci/vfio_pci.c         | 11 ++++++----
> >>  drivers/vfio/pci/vfio_pci_config.c  | 32 +++++++++++++++--------------
> >>  drivers/vfio/pci/vfio_pci_private.h |  4 ++--
> >>  3 files changed, 26 insertions(+), 21 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];
> > 
> > Why can't we just drop PCI_STD_RESOURCES and replace it was 0. I understand
> > the abstraction here, but we don't do it elsewhere across the kernel. Is this
> > necessary?
> 
> There was a discussion about this particular case:
> https://lkml.org/lkml/2019/8/12/999
> 
> It was decided to save the original style for vfio drivers.

OK no problem.

Thanks,

Andrew Murray

> 
> > 
> > Thanks,
> > 
> > Andrew Murray
> > 
> >>  
> >>  		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..90c0b80f8acf 100644
> >> --- a/drivers/vfio/pci/vfio_pci_config.c
> >> +++ b/drivers/vfio/pci/vfio_pci_config.c
> >> @@ -450,30 +450,32 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
> >>  {
> >>  	struct pci_dev *pdev = vdev->pdev;
> >>  	int i;
> >> -	__le32 *bar;
> >> +	__le32 *vbar;
> >>  	u64 mask;
> >>  
> >> -	bar = (__le32 *)&vdev->vconfig[PCI_BASE_ADDRESS_0];
> >> +	vbar = (__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)) {
> >> -			*bar = 0; /* Unmapped by host = unimplemented to user */
> >> +	for (i = 0; i < PCI_STD_NUM_BARS; i++, vbar++) {
> >> +		int bar = i + PCI_STD_RESOURCES;
> >> +
> >> +		if (!pci_resource_start(pdev, bar)) {
> >> +			*vbar = 0; /* Unmapped by host = unimplemented to user */
> >>  			continue;
> >>  		}
> >>  
> >> -		mask = ~(pci_resource_len(pdev, i) - 1);
> >> +		mask = ~(pci_resource_len(pdev, bar) - 1);
> >>  
> >> -		*bar &= cpu_to_le32((u32)mask);
> >> -		*bar |= vfio_generate_bar_flags(pdev, i);
> >> +		*vbar &= cpu_to_le32((u32)mask);
> >> +		*vbar |= vfio_generate_bar_flags(pdev, bar);
> >>  
> >> -		if (*bar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> >> -			bar++;
> >> -			*bar &= cpu_to_le32((u32)(mask >> 32));
> >> +		if (*vbar & cpu_to_le32(PCI_BASE_ADDRESS_MEM_TYPE_64)) {
> >> +			vbar++;
> >> +			*vbar &= cpu_to_le32((u32)(mask >> 32));
> >>  			i++;
> >>  		}
> >>  	}
> >>  
> >> -	bar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
> >> +	vbar = (__le32 *)&vdev->vconfig[PCI_ROM_ADDRESS];
> >>  
> >>  	/*
> >>  	 * NB. REGION_INFO will have reported zero size if we weren't able
> >> @@ -483,14 +485,14 @@ static void vfio_bar_fixup(struct vfio_pci_device *vdev)
> >>  	if (pci_resource_start(pdev, PCI_ROM_RESOURCE)) {
> >>  		mask = ~(pci_resource_len(pdev, PCI_ROM_RESOURCE) - 1);
> >>  		mask |= PCI_ROM_ADDRESS_ENABLE;
> >> -		*bar &= cpu_to_le32((u32)mask);
> >> +		*vbar &= cpu_to_le32((u32)mask);
> >>  	} else if (pdev->resource[PCI_ROM_RESOURCE].flags &
> >>  					IORESOURCE_ROM_SHADOW) {
> >>  		mask = ~(0x20000 - 1);
> >>  		mask |= PCI_ROM_ADDRESS_ENABLE;
> >> -		*bar &= cpu_to_le32((u32)mask);
> >> +		*vbar &= cpu_to_le32((u32)mask);
> >>  	} else
> >> -		*bar = 0;
> >> +		*vbar = 0;
> >>  
> >>  	vdev->bardirty = false;
> >>  }
> >> 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
> >>
> 

[PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Denis Efremov]

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

Cc: Jack Wang <jinpu.wang@cloud.ionos.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/scsi/pm8001/pm8001_hwi.c  | 2 +-
 drivers/scsi/pm8001/pm8001_init.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 68a8217032d0..1a3661d6be06 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1186,7 +1186,7 @@ static void pm8001_hw_chip_rst(struct pm8001_hba_info *pm8001_ha)
 void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
 {
 	s8 bar, logical = 0;
-	for (bar = 0; bar < 6; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		/*
 		** logical BARs for SPC:
 		** bar 0 and 1 - logical BAR0
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 3374f553c617..aca913490eb5 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -401,7 +401,7 @@ static int pm8001_ioremap(struct pm8001_hba_info *pm8001_ha)
 
 	pdev = pm8001_ha->pdev;
 	/* map pci mem (PMC pci base 0-3)*/
-	for (bar = 0; bar < 6; bar++) {
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
 		/*
 		** logical BARs for SPC:
 		** bar 0 and 1 - logical BAR0
-- 
2.21.0

Re: [PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Jinpu Wang]

On Mon, Sep 16, 2019 at 10:47 PM Denis Efremov <efremov@linux.com> wrote:
>
> Replace the magic constant (6) with define PCI_STD_NUM_BARS representing
> the number of PCI BARs.
>
> Cc: Jack Wang <jinpu.wang@cloud.ionos.com>
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Signed-off-by: Denis Efremov <efremov@linux.com>
Looks fine, thanks!
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>
> ---
>  drivers/scsi/pm8001/pm8001_hwi.c  | 2 +-
>  drivers/scsi/pm8001/pm8001_init.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index 68a8217032d0..1a3661d6be06 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -1186,7 +1186,7 @@ static void pm8001_hw_chip_rst(struct pm8001_hba_info *pm8001_ha)
>  void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha)
>  {
>         s8 bar, logical = 0;
> -       for (bar = 0; bar < 6; bar++) {
> +       for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>                 /*
>                 ** logical BARs for SPC:
>                 ** bar 0 and 1 - logical BAR0
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 3374f553c617..aca913490eb5 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -401,7 +401,7 @@ static int pm8001_ioremap(struct pm8001_hba_info *pm8001_ha)
>
>         pdev = pm8001_ha->pdev;
>         /* map pci mem (PMC pci base 0-3)*/
> -       for (bar = 0; bar < 6; bar++) {
> +       for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>                 /*
>                 ** logical BARs for SPC:
>                 ** bar 0 and 1 - logical BAR0
> --
> 2.21.0
>


-- 
Jack Wang
Linux Kernel Developer
Platform Engineering Compute (IONOS Cloud)

Re: [PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Martin K. Petersen]

Denis,

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

Applied to 5.4/scsi-fixes. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Denis Efremov]

Hi,

On 24.09.2019 05:22, Martin K. Petersen wrote:
> 
> Denis,
> 
>> Replace the magic constant (6) with define PCI_STD_NUM_BARS
>> representing the number of PCI BARs.
> 
> Applied to 5.4/scsi-fixes. Thanks!
> 

This constant PCI_STD_NUM_BARS is introduced in the first patch [01/26].
I'm afraid that this patch without the first one will break the compilation.
This patchset is dedicated to Bjorn's tree. Sorry for confusing you.

Thanks,
Denis

Re: [PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Bjorn Helgaas]

On Mon, Sep 23, 2019 at 10:22:42PM -0400, Martin K. Petersen wrote:
> 
> Denis,
> 
> > Replace the magic constant (6) with define PCI_STD_NUM_BARS
> > representing the number of PCI BARs.
> 
> Applied to 5.4/scsi-fixes. Thanks!

I think this depends on a previous patch that actually adds the
PCI_STD_NUM_BARS definition.  It will probably be easier if I apply
the whole series via the PCI tree.

Bjorn

Re: [PATCH v3 18/26] scsi: pm80xx: Use PCI_STD_NUM_BARS

[Martin K. Petersen]

Bjorn,

> I think this depends on a previous patch that actually adds the
> PCI_STD_NUM_BARS definition.  It will probably be easier if I apply
> the whole series via the PCI tree.

Looks like my mail about this getting dropped due to the missing
definition got lost in transit. In any case, feel free to take this
through the PCI tree.

Acked-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH v3 19/26] ata: sata_nv: Use PCI_STD_NUM_BARS

[Denis Efremov]

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

Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/ata/sata_nv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index b44b4b64354c..31a32a4bb05b 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -2329,7 +2329,7 @@ static int nv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
         // Make sure this is a SATA controller by counting the number of bars
         // (NVIDIA SATA controllers will always have six bars).  Otherwise,
         // it's an IDE controller and we ignore it.
-	for (bar = 0; bar < 6; bar++)
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
 		if (pci_resource_start(pdev, bar) == 0)
 			return -ENODEV;
 
-- 
2.21.0

[PATCH v3 20/26] staging: gasket: Use PCI_STD_NUM_BARS

[Denis Efremov]

Remove local definition GASKET_NUM_BARS for the number of PCI BARs and use
global one PCI_STD_NUM_BARS instead.

Cc: Rob Springer <rspringer@google.com>
Cc: Todd Poynor <toddpoynor@google.com>
Cc: Ben Chan <benchan@chromium.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/staging/gasket/gasket_constants.h |  3 ---
 drivers/staging/gasket/gasket_core.c      | 12 ++++++------
 drivers/staging/gasket/gasket_core.h      |  4 ++--
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/gasket/gasket_constants.h b/drivers/staging/gasket/gasket_constants.h
index 50d87c7b178c..9ea9c8833f27 100644
--- a/drivers/staging/gasket/gasket_constants.h
+++ b/drivers/staging/gasket/gasket_constants.h
@@ -13,9 +13,6 @@
 /* The maximum devices per each type. */
 #define GASKET_DEV_MAX 256
 
-/* The number of supported (and possible) PCI BARs. */
-#define GASKET_NUM_BARS 6
-
 /* The number of supported Gasket page tables per device. */
 #define GASKET_MAX_NUM_PAGE_TABLES 1
 
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index 13179f063a61..cd8be80d2076 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -371,7 +371,7 @@ static int gasket_setup_pci(struct pci_dev *pci_dev,
 {
 	int i, mapped_bars, ret;
 
-	for (i = 0; i < GASKET_NUM_BARS; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		ret = gasket_map_pci_bar(gasket_dev, i);
 		if (ret) {
 			mapped_bars = i;
@@ -393,7 +393,7 @@ static void gasket_cleanup_pci(struct gasket_dev *gasket_dev)
 {
 	int i;
 
-	for (i = 0; i < GASKET_NUM_BARS; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		gasket_unmap_pci_bar(gasket_dev, i);
 }
 
@@ -493,7 +493,7 @@ static ssize_t gasket_sysfs_data_show(struct device *device,
 		(enum gasket_sysfs_attribute_type)gasket_attr->data.attr_type;
 	switch (sysfs_type) {
 	case ATTR_BAR_OFFSETS:
-		for (i = 0; i < GASKET_NUM_BARS; i++) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 			bar_desc = &driver_desc->bar_descriptions[i];
 			if (bar_desc->size == 0)
 				continue;
@@ -505,7 +505,7 @@ static ssize_t gasket_sysfs_data_show(struct device *device,
 		}
 		break;
 	case ATTR_BAR_SIZES:
-		for (i = 0; i < GASKET_NUM_BARS; i++) {
+		for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 			bar_desc = &driver_desc->bar_descriptions[i];
 			if (bar_desc->size == 0)
 				continue;
@@ -556,7 +556,7 @@ static ssize_t gasket_sysfs_data_show(struct device *device,
 		ret = snprintf(buf, PAGE_SIZE, "%d\n", gasket_dev->reset_count);
 		break;
 	case ATTR_USER_MEM_RANGES:
-		for (i = 0; i < GASKET_NUM_BARS; ++i) {
+		for (i = 0; i < PCI_STD_NUM_BARS; ++i) {
 			current_written =
 				gasket_write_mappable_regions(buf, driver_desc,
 							      i);
@@ -736,7 +736,7 @@ static int gasket_get_bar_index(const struct gasket_dev *gasket_dev,
 	const struct gasket_driver_desc *driver_desc;
 
 	driver_desc = gasket_dev->internal_desc->driver_desc;
-	for (i = 0; i < GASKET_NUM_BARS; ++i) {
+	for (i = 0; i < PCI_STD_NUM_BARS; ++i) {
 		struct gasket_bar_desc bar_desc =
 			driver_desc->bar_descriptions[i];
 
diff --git a/drivers/staging/gasket/gasket_core.h b/drivers/staging/gasket/gasket_core.h
index be44ac1e3118..c417acadb0d5 100644
--- a/drivers/staging/gasket/gasket_core.h
+++ b/drivers/staging/gasket/gasket_core.h
@@ -268,7 +268,7 @@ struct gasket_dev {
 	char kobj_name[GASKET_NAME_MAX];
 
 	/* Virtual address of mapped BAR memory range. */
-	struct gasket_bar_data bar_data[GASKET_NUM_BARS];
+	struct gasket_bar_data bar_data[PCI_STD_NUM_BARS];
 
 	/* Coherent buffer. */
 	struct gasket_coherent_buffer coherent_buffer;
@@ -369,7 +369,7 @@ struct gasket_driver_desc {
 	/* Set of 6 bar descriptions that describe all PCIe bars.
 	 * Note that BUS/AXI devices (i.e. non PCI devices) use those.
 	 */
-	struct gasket_bar_desc bar_descriptions[GASKET_NUM_BARS];
+	struct gasket_bar_desc bar_descriptions[PCI_STD_NUM_BARS];
 
 	/*
 	 * Coherent buffer description.
-- 
2.21.0

[PATCH v3 21/26] serial: 8250_pci: Use PCI_STD_NUM_BARS

[Denis Efremov]

Remove local definition PCI_NUM_BAR_RESOURCES for the number of PCI BARs
and use global one PCI_STD_NUM_BARS instead.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/tty/serial/8250/8250_pci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 7f740b37700b..e557605ec75f 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -43,8 +43,6 @@ struct pci_serial_quirk {
 	void	(*exit)(struct pci_dev *dev);
 };
 
-#define PCI_NUM_BAR_RESOURCES	6
-
 struct serial_private {
 	struct pci_dev		*dev;
 	unsigned int		nr;
@@ -74,7 +72,7 @@ setup_port(struct serial_private *priv, struct uart_8250_port *port,
 {
 	struct pci_dev *dev = priv->dev;
 
-	if (bar >= PCI_NUM_BAR_RESOURCES)
+	if (bar >= PCI_STD_NUM_BARS)
 		return -EINVAL;
 
 	if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {
@@ -3583,7 +3581,7 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 		return -ENODEV;
 
 	num_iomem = num_port = 0;
-	for (i = 0; i < PCI_NUM_BAR_RESOURCES; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_flags(dev, i) & IORESOURCE_IO) {
 			num_port++;
 			if (first_port == -1)
@@ -3611,7 +3609,7 @@ serial_pci_guess_board(struct pci_dev *dev, struct pciserial_board *board)
 	 */
 	first_port = -1;
 	num_port = 0;
-	for (i = 0; i < PCI_NUM_BAR_RESOURCES; i++) {
+	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
 		if (pci_resource_flags(dev, i) & IORESOURCE_IO &&
 		    pci_resource_len(dev, i) == 8 &&
 		    (first_port == -1 || (first_port + num_port) == i)) {
-- 
2.21.0

[PATCH v3 22/26] pata_atp867x: Use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Cc: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/ata/pata_atp867x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/pata_atp867x.c b/drivers/ata/pata_atp867x.c
index 2b9ed4ddef8d..019198eb7099 100644
--- a/drivers/ata/pata_atp867x.c
+++ b/drivers/ata/pata_atp867x.c
@@ -422,7 +422,7 @@ static int atp867x_ata_pci_sff_init_host(struct ata_host *host)
 #ifdef	ATP867X_DEBUG
 	atp867x_check_res(pdev);
 
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		printk(KERN_DEBUG "ATP867X: iomap[%d]=0x%llx\n", i,
 			(unsigned long long)(host->iomap[i]));
 #endif
-- 
2.21.0

[PATCH v3 23/26] memstick: use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Maxim Levitsky <maximlevitsky@gmail.com>
Cc: Alex Dubov <oakad@yahoo.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/memstick/host/jmb38x_ms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
index 32747425297d..fd281c1d39b1 100644
--- a/drivers/memstick/host/jmb38x_ms.c
+++ b/drivers/memstick/host/jmb38x_ms.c
@@ -848,7 +848,7 @@ static int jmb38x_ms_count_slots(struct pci_dev *pdev)
 {
 	int cnt, rc = 0;
 
-	for (cnt = 0; cnt < PCI_ROM_RESOURCE; ++cnt) {
+	for (cnt = 0; cnt < PCI_STD_NUM_BARS; ++cnt) {
 		if (!(IORESOURCE_MEM & pci_resource_flags(pdev, cnt)))
 			break;
 
-- 
2.21.0

Re: [PATCH v3 23/26] memstick: use PCI_STD_NUM_BARS

[Ulf Hansson]

On Mon, 16 Sep 2019 at 22:47, Denis Efremov <efremov@linux.com> wrote:
>
> Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
> PCI BARs.
>
> Cc: Maxim Levitsky <maximlevitsky@gmail.com>
> Cc: Alex Dubov <oakad@yahoo.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Denis Efremov <efremov@linux.com>

Assuming this depends on other changes in the series? Thus this is
probably for PCI maintainers to pick up?

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/memstick/host/jmb38x_ms.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
> index 32747425297d..fd281c1d39b1 100644
> --- a/drivers/memstick/host/jmb38x_ms.c
> +++ b/drivers/memstick/host/jmb38x_ms.c
> @@ -848,7 +848,7 @@ static int jmb38x_ms_count_slots(struct pci_dev *pdev)
>  {
>         int cnt, rc = 0;
>
> -       for (cnt = 0; cnt < PCI_ROM_RESOURCE; ++cnt) {
> +       for (cnt = 0; cnt < PCI_STD_NUM_BARS; ++cnt) {
>                 if (!(IORESOURCE_MEM & pci_resource_flags(pdev, cnt)))
>                         break;
>
> --
> 2.21.0
>

Re: [PATCH v3 23/26] memstick: use PCI_STD_NUM_BARS

[Denis Efremov]

On 20.09.2019 10:42, Ulf Hansson wrote:
> On Mon, 16 Sep 2019 at 22:47, Denis Efremov <efremov@linux.com> wrote:
>>
>> Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
>> PCI BARs.
>>
>> Cc: Maxim Levitsky <maximlevitsky@gmail.com>
>> Cc: Alex Dubov <oakad@yahoo.com>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Denis Efremov <efremov@linux.com>
> 
> Assuming this depends on other changes in the series?

Yes, the first patch introduce define PCI_STD_NUM_BARS.

> Thus this is
> probably for PCI maintainers to pick up?

Yes, this is for Bjorn's tree.

> 
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> 
> Kind regards
> Uffe
> 
>> ---
>>  drivers/memstick/host/jmb38x_ms.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/memstick/host/jmb38x_ms.c b/drivers/memstick/host/jmb38x_ms.c
>> index 32747425297d..fd281c1d39b1 100644
>> --- a/drivers/memstick/host/jmb38x_ms.c
>> +++ b/drivers/memstick/host/jmb38x_ms.c
>> @@ -848,7 +848,7 @@ static int jmb38x_ms_count_slots(struct pci_dev *pdev)
>>  {
>>         int cnt, rc = 0;
>>
>> -       for (cnt = 0; cnt < PCI_ROM_RESOURCE; ++cnt) {
>> +       for (cnt = 0; cnt < PCI_STD_NUM_BARS; ++cnt) {
>>                 if (!(IORESOURCE_MEM & pci_resource_flags(pdev, cnt)))
>>                         break;
>>
>> --
>> 2.21.0
>>

[PATCH v3 24/26] USB: core: Use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/usb/core/hcd-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 7537681355f6..3a79e8623d66 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -234,7 +234,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		/* UHCI */
 		int	region;
 
-		for (region = 0; region < PCI_ROM_RESOURCE; region++) {
+		for (region = 0; region < PCI_STD_NUM_BARS; region++) {
 			if (!(pci_resource_flags(dev, region) &
 					IORESOURCE_IO))
 				continue;
-- 
2.21.0

[PATCH v3 25/26] usb: pci-quirks: Use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Mathias Nyman <mathias.nyman@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 drivers/usb/host/pci-quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index f6d04491df60..6c7f0a876b96 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -728,7 +728,7 @@ static void quirk_usb_handoff_uhci(struct pci_dev *pdev)
 	if (!pio_enabled(pdev))
 		return;
 
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+	for (i = 0; i < PCI_STD_NUM_BARS; i++)
 		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
 			base = pci_resource_start(pdev, i);
 			break;
-- 
2.21.0

[PATCH v3 26/26] devres: use PCI_STD_NUM_BARS

[Denis Efremov]

Use define PCI_STD_NUM_BARS instead of PCI_ROM_RESOURCE for the number of
PCI BARs.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Denis Efremov <efremov@linux.com>
---
 lib/devres.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/devres.c b/lib/devres.c
index 6a0e9bd6524a..ab75d73122b8 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -262,7 +262,7 @@ EXPORT_SYMBOL(devm_ioport_unmap);
 /*
  * PCI iomap devres
  */
-#define PCIM_IOMAP_MAX	PCI_ROM_RESOURCE
+#define PCIM_IOMAP_MAX	PCI_STD_NUM_BARS
 
 struct pcim_iomap_devres {
 	void __iomem *table[PCIM_IOMAP_MAX];
-- 
2.21.0