All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read()
@ 2015-09-29 13:57 Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 1/3] PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage Gabriele Paoloni
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gabriele Paoloni @ 2015-09-29 13:57 UTC (permalink / raw)
  To: bhelgaas, jingoohan1, pratyush.anand
  Cc: linux-pci, gabriele.paoloni, wangzhou1, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liguozhu

From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patchset:
1) fixes a bug in spear13xx when calling dw_pcie_cfg_read and
   dw_pcie_cfg_write usign the patch from Pratyush in
   https://lkml.org/lkml/2015/9/7/5
2) reworks dw_pcie_cfg_read/dw_pcie_cfg_write in pcie-designware.c in
   order to make it easier for callers to pass input parameters;
3) changes dw_pcie_cfg_read implementation to make it symmetric with
   dw_pcie_cfg_write 
4) adds sanity checks in dw_pcie_cfg_read/dw_pcie_cfg_write to make
   sure the PCI header offset does not conflict with the size of
   the read/written field.

Changes from v4:
   - included https://lkml.org/lkml/2015/9/7/5 back as it was lost in v4

Changes from v3:
   - changed dw_pcie_cfg_read implementation to make it symmetric with
     dw_pcie_cfg_write
   - changed dw_pcie_cfg_read/dw_pcie_cfg_write implementations to take
     as input param directly the address of the field to read/write rather
     than the base address and the offset in the PCI header

Change from v2:
   Replaced patch 1/3 with Pratyush one in
   https://lkml.org/lkml/2015/9/7/5


gabriele paoloni (3):
  PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage
  PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read()
  PCI: designware: add sanity checks on the header offset in
    dw_pcie_cfg_read and dw_pcie_cfg_write

 drivers/pci/host/pci-exynos.c      |  5 ++--
 drivers/pci/host/pci-keystone-dw.c |  4 +--
 drivers/pci/host/pcie-designware.c | 52 ++++++++++++++++++++------------------
 drivers/pci/host/pcie-designware.h |  4 +--
 drivers/pci/host/pcie-spear13xx.c  | 24 +++++++++---------
 5 files changed, 46 insertions(+), 43 deletions(-)

-- 
1.9.1


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

* [PATCH v5 1/3] PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage
  2015-09-29 13:57 [PATCH v5 0/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
@ 2015-09-29 13:57 ` Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 2/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write Gabriele Paoloni
  2 siblings, 0 replies; 6+ messages in thread
From: Gabriele Paoloni @ 2015-09-29 13:57 UTC (permalink / raw)
  To: bhelgaas, jingoohan1, pratyush.anand
  Cc: linux-pci, gabriele.paoloni, wangzhou1, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liguozhu, Pratyush Anand

From: gabriele paoloni <gabriele.paoloni@huawei.com>

w_pcie_cfg_read/write() expects 1st argument 'addr' as a 32 bit word
aligned address, 2nd argument 'where' as the offset within that word(0, 1,
2 or 3) and 3rd argument 'size' is the number of bytes need to be read at
the offset(could be 1, 2 ,4).

SPEAr13xx is using dw_pcie_cfg_read() and dw_pcie_cfg_write() incorrectly.
Without this fix, SPEAr13xx host will never work with  few buggy gen1 card
which connects with only gen1 host and also with any endpoint which would
generate a read request of more than 128 bytes.

Cc: stable@vger.kernel.org # v3.17+
Signed-off-by: Pratyush Anand <panand@redhat.com>
Reported-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-spear13xx.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 98d2683..0754ea3 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -163,34 +163,36 @@ static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 	 * default value in capability register is 512 bytes. So force
 	 * it to 128 here.
 	 */
-	dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, &val);
+	dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL,
+				0, 2, &val);
 	val &= ~PCI_EXP_DEVCTL_READRQ;
-	dw_pcie_cfg_write(pp->dbi_base, exp_cap_off + PCI_EXP_DEVCTL, 4, val);
+	dw_pcie_cfg_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL,
+				0, 2, val);
 
-	dw_pcie_cfg_write(pp->dbi_base, PCI_VENDOR_ID, 2, 0x104A);
-	dw_pcie_cfg_write(pp->dbi_base, PCI_DEVICE_ID, 2, 0xCD80);
+	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 0, 2, 0x104A);
+	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 2, 2, 0xCD80);
 
 	/*
 	 * if is_gen1 is set then handle it, so that some buggy card
 	 * also works
 	 */
 	if (spear13xx_pcie->is_gen1) {
-		dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_LNKCAP, 4,
-				 &val);
+		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
+					0, 4, &val);
 		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
 			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
 			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
-			dw_pcie_cfg_write(pp->dbi_base, exp_cap_off +
-					  PCI_EXP_LNKCAP, 4, val);
+			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
+						PCI_EXP_LNKCAP, 0, 4, val);
 		}
 
-		dw_pcie_cfg_read(pp->dbi_base, exp_cap_off + PCI_EXP_LNKCTL2, 4,
-				 &val);
+		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
+					0, 2, &val);
 		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
 			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
 			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
-			dw_pcie_cfg_write(pp->dbi_base, exp_cap_off +
-					  PCI_EXP_LNKCTL2, 4, val);
+			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
+						PCI_EXP_LNKCTL2, 0, 2, val);
 		}
 	}
 
-- 
1.9.1


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

* [PATCH v5 2/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read()
  2015-09-29 13:57 [PATCH v5 0/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 1/3] PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage Gabriele Paoloni
@ 2015-09-29 13:57 ` Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write Gabriele Paoloni
  2 siblings, 0 replies; 6+ messages in thread
From: Gabriele Paoloni @ 2015-09-29 13:57 UTC (permalink / raw)
  To: bhelgaas, jingoohan1, pratyush.anand
  Cc: linux-pci, gabriele.paoloni, wangzhou1, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liguozhu

From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patch changes the implementation of dw_pcie_cfg_read() and
dw_pcie_cfg_write() to improve the function usage from the callers
perspective.
Currently the callers are obliged to pass the 32bit aligned address
of the register that contains the field of the PCI header that they
want to read/write; also they have to pass the offset of the field
in that register. This is quite tricky to use as the callers are
obliged to sum the PCI header base address to the field offset
masked to retrieve the 32b aligned register address.

With the new API the callers have to pass directly the address of the
field they intend to read/write in the PCI header: so once they have
the base address of the PCI header they will just sum up the offset
of the field they intend to access and pass the sum to the functions'
addr field.

This patch also changes the implementation of dw_pcie_cfg_read to
make it symmetric with dw_pcie_cfg_write.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 drivers/pci/host/pci-exynos.c      |  5 ++---
 drivers/pci/host/pci-keystone-dw.c |  4 ++--
 drivers/pci/host/pcie-designware.c | 36 ++++++++++++++++--------------------
 drivers/pci/host/pcie-designware.h |  4 ++--
 drivers/pci/host/pcie-spear13xx.c  | 18 ++++++++----------
 5 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index f9f468d..01095e1 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -454,7 +454,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
 	int ret;
 
 	exynos_pcie_sideband_dbi_r_mode(pp, true);
-	ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where, size, val);
+	ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
 	exynos_pcie_sideband_dbi_r_mode(pp, false);
 	return ret;
 }
@@ -465,8 +465,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 	int ret;
 
 	exynos_pcie_sideband_dbi_w_mode(pp, true);
-	ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3),
-			where, size, val);
+	ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
 	exynos_pcie_sideband_dbi_w_mode(pp, false);
 	return ret;
 }
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index e71da99..51789b7 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -398,7 +398,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 
 	addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
 
-	return dw_pcie_cfg_read(addr + (where & ~0x3), where, size, val);
+	return dw_pcie_cfg_read(addr + where, size, val);
 }
 
 int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
@@ -410,7 +410,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 
 	addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
 
-	return dw_pcie_cfg_write(addr + (where & ~0x3), where, size, val);
+	return dw_pcie_cfg_write(addr + where, size, val);
 }
 
 /**
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 52aa6e3..d771fa5 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -80,28 +80,28 @@ static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
 	return sys->private_data;
 }
 
-int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
+int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 {
-	*val = readl(addr);
-
-	if (size == 1)
-		*val = (*val >> (8 * (where & 3))) & 0xff;
+	if (size == 4)
+		*val = readl(addr);
 	else if (size == 2)
-		*val = (*val >> (8 * (where & 3))) & 0xffff;
-	else if (size != 4)
+		*val = readw(addr);
+	else if (size == 1)
+		*val = readb(addr);
+	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
 
 	return PCIBIOS_SUCCESSFUL;
 }
 
-int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val)
+int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
 {
 	if (size == 4)
 		writel(val, addr);
 	else if (size == 2)
-		writew(val, addr + (where & 2));
+		writew(val, addr);
 	else if (size == 1)
-		writeb(val, addr + (where & 3));
+		writeb(val, addr);
 	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
 
@@ -132,8 +132,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
 	if (pp->ops->rd_own_conf)
 		ret = pp->ops->rd_own_conf(pp, where, size, val);
 	else
-		ret = dw_pcie_cfg_read(pp->dbi_base + (where & ~0x3), where,
-				size, val);
+		ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
 
 	return ret;
 }
@@ -146,8 +145,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
 	if (pp->ops->wr_own_conf)
 		ret = pp->ops->wr_own_conf(pp, where, size, val);
 	else
-		ret = dw_pcie_cfg_write(pp->dbi_base + (where & ~0x3), where,
-				size, val);
+		ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
 
 	return ret;
 }
@@ -539,13 +537,12 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		u32 devfn, int where, int size, u32 *val)
 {
 	int ret, type;
-	u32 address, busdev, cfg_size;
+	u32 busdev, cfg_size;
 	u64 cpu_addr;
 	void __iomem *va_cfg_base;
 
 	busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
 		 PCIE_ATU_FUNC(PCI_FUNC(devfn));
-	address = where & ~0x3;
 
 	if (bus->parent->number == pp->root_bus_nr) {
 		type = PCIE_ATU_TYPE_CFG0;
@@ -562,7 +559,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
 				  type, cpu_addr,
 				  busdev, cfg_size);
-	ret = dw_pcie_cfg_read(va_cfg_base + address, where, size, val);
+	ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
 	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
 				  PCIE_ATU_TYPE_IO, pp->io_mod_base,
 				  pp->io_bus_addr, pp->io_size);
@@ -574,13 +571,12 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 		u32 devfn, int where, int size, u32 val)
 {
 	int ret, type;
-	u32 address, busdev, cfg_size;
+	u32 busdev, cfg_size;
 	u64 cpu_addr;
 	void __iomem *va_cfg_base;
 
 	busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
 		 PCIE_ATU_FUNC(PCI_FUNC(devfn));
-	address = where & ~0x3;
 
 	if (bus->parent->number == pp->root_bus_nr) {
 		type = PCIE_ATU_TYPE_CFG0;
@@ -597,7 +593,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
 	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
 				  type, cpu_addr,
 				  busdev, cfg_size);
-	ret = dw_pcie_cfg_write(va_cfg_base + address, where, size, val);
+	ret = dw_pcie_cfg_write(va_cfg_base + where, size, val);
 	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0,
 				  PCIE_ATU_TYPE_IO, pp->io_mod_base,
 				  pp->io_bus_addr, pp->io_size);
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd27..0b29194 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -76,8 +76,8 @@ struct pcie_host_ops {
 	int (*msi_host_init)(struct pcie_port *pp, struct msi_controller *chip);
 };
 
-int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
-int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val);
+int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val);
+int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val);
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index 0754ea3..b95b756 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -163,14 +163,12 @@ static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 	 * default value in capability register is 512 bytes. So force
 	 * it to 128 here.
 	 */
-	dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL,
-				0, 2, &val);
+	dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
 	val &= ~PCI_EXP_DEVCTL_READRQ;
-	dw_pcie_cfg_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL,
-				0, 2, val);
+	dw_pcie_cfg_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
 
-	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 0, 2, 0x104A);
-	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 2, 2, 0xCD80);
+	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
+	dw_pcie_cfg_write(pp->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
 
 	/*
 	 * if is_gen1 is set then handle it, so that some buggy card
@@ -178,21 +176,21 @@ static int spear13xx_pcie_establish_link(struct pcie_port *pp)
 	 */
 	if (spear13xx_pcie->is_gen1) {
 		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
-					0, 4, &val);
+					4, &val);
 		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
 			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
 			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
 			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
-						PCI_EXP_LNKCAP, 0, 4, val);
+				PCI_EXP_LNKCAP, 4, val);
 		}
 
 		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
-					0, 2, &val);
+					2, &val);
 		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
 			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
 			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
 			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
-						PCI_EXP_LNKCTL2, 0, 2, val);
+					PCI_EXP_LNKCTL2, 2, val);
 		}
 	}
 
-- 
1.9.1


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

* [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write
  2015-09-29 13:57 [PATCH v5 0/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 1/3] PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage Gabriele Paoloni
  2015-09-29 13:57 ` [PATCH v5 2/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
@ 2015-09-29 13:57 ` Gabriele Paoloni
  2015-09-29 15:36   ` Pratyush Anand
  2 siblings, 1 reply; 6+ messages in thread
From: Gabriele Paoloni @ 2015-09-29 13:57 UTC (permalink / raw)
  To: bhelgaas, jingoohan1, pratyush.anand
  Cc: linux-pci, gabriele.paoloni, wangzhou1, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liguozhu

From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patch adds sanity checks on "where" input parameter in
dw_pcie_cfg_read and dw_pcie_cfg_write. These checks make sure
that offset passed in by the caller is not in conflict with
the size of the PCI header field that is being read/written

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 drivers/pci/host/pcie-designware.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index d771fa5..719d2cd 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -82,11 +82,15 @@ static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
 
 int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 {
-	if (size == 4)
+	if (size == 4) {
+		if ((uintptr_t)addr & 3)
+			return PCIBIOS_BAD_REGISTER_NUMBER;
 		*val = readl(addr);
-	else if (size == 2)
+	} else if (size == 2) {
+		if ((uintptr_t)addr & 1)
+			return PCIBIOS_BAD_REGISTER_NUMBER;
 		*val = readw(addr);
-	else if (size == 1)
+	} else if (size == 1)
 		*val = readb(addr);
 	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
@@ -96,11 +100,15 @@ int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 
 int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
 {
-	if (size == 4)
+	if (size == 4) {
+		if ((uintptr_t)addr & 3)
+			return PCIBIOS_BAD_REGISTER_NUMBER;
 		writel(val, addr);
-	else if (size == 2)
+	} else if (size == 2) {
+		if ((uintptr_t)addr & 1)
+			return PCIBIOS_BAD_REGISTER_NUMBER;
 		writew(val, addr);
-	else if (size == 1)
+	} else if (size == 1)
 		writeb(val, addr);
 	else
 		return PCIBIOS_BAD_REGISTER_NUMBER;
-- 
1.9.1


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

* Re: [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write
  2015-09-29 13:57 ` [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write Gabriele Paoloni
@ 2015-09-29 15:36   ` Pratyush Anand
  2015-09-29 15:54     ` Gabriele Paoloni
  0 siblings, 1 reply; 6+ messages in thread
From: Pratyush Anand @ 2015-09-29 15:36 UTC (permalink / raw)
  To: Gabriele Paoloni
  Cc: Bjorn Helgaas, Jingoo Han, linux-pci, Zhou Wang, Zhichang Yuan,
	zhudacai, Zhang Jukuo, qiuzhenfa, Liguozhu

On Tue, Sep 29, 2015 at 7:27 PM, Gabriele Paoloni
<gabriele.paoloni@huawei.com> wrote:
> From: gabriele paoloni <gabriele.paoloni@huawei.com>
>
> This patch adds sanity checks on "where" input parameter in
> dw_pcie_cfg_read and dw_pcie_cfg_write. These checks make sure
> that offset passed in by the caller is not in conflict with
> the size of the PCI header field that is being read/written
>

I am still not convinced that we should doubt the caller..But may be I
am biased in my thoughts...
Since Bjorn has asked about it, so will take it.

> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> ---
>  drivers/pci/host/pcie-designware.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index d771fa5..719d2cd 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -82,11 +82,15 @@ static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
>
>  int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
>  {

Wouldn't a single check would have been better
+       if ((uintptr_t)addr & (size -1))
+               return PCIBIOS_BAD_REGISTER_NUMBER;

~Pratyush

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

* RE: [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write
  2015-09-29 15:36   ` Pratyush Anand
@ 2015-09-29 15:54     ` Gabriele Paoloni
  0 siblings, 0 replies; 6+ messages in thread
From: Gabriele Paoloni @ 2015-09-29 15:54 UTC (permalink / raw)
  To: Pratyush Anand
  Cc: Bjorn Helgaas, Jingoo Han, linux-pci, Wangzhou (B),
	Yuanzhichang, Zhudacai, zhangjukuo, qiuzhenfa, Liguozhu (Kenneth)

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBQcmF0eXVzaCBBbmFuZCBbbWFp
bHRvOnByYXR5dXNoLmFuYW5kQGdtYWlsLmNvbV0NCj4gU2VudDogVHVlc2RheSwgU2VwdGVtYmVy
IDI5LCAyMDE1IDQ6MzYgUE0NCj4gVG86IEdhYnJpZWxlIFBhb2xvbmkNCj4gQ2M6IEJqb3JuIEhl
bGdhYXM7IEppbmdvbyBIYW47IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IFdhbmd6aG91IChC
KTsNCj4gWXVhbnpoaWNoYW5nOyBaaHVkYWNhaTsgemhhbmdqdWt1bzsgcWl1emhlbmZhOyBMaWd1
b3podSAoS2VubmV0aCkNCj4gU3ViamVjdDogUmU6IFtQQVRDSCB2NSAzLzNdIFBDSTogZGVzaWdu
d2FyZTogYWRkIHNhbml0eSBjaGVja3Mgb24gdGhlDQo+IGhlYWRlciBvZmZzZXQgaW4gZHdfcGNp
ZV9jZmdfcmVhZCBhbmQgZHdfcGNpZV9jZmdfd3JpdGUNCj4gDQo+IE9uIFR1ZSwgU2VwIDI5LCAy
MDE1IGF0IDc6MjcgUE0sIEdhYnJpZWxlIFBhb2xvbmkNCj4gPGdhYnJpZWxlLnBhb2xvbmlAaHVh
d2VpLmNvbT4gd3JvdGU6DQo+ID4gRnJvbTogZ2FicmllbGUgcGFvbG9uaSA8Z2FicmllbGUucGFv
bG9uaUBodWF3ZWkuY29tPg0KPiA+DQo+ID4gVGhpcyBwYXRjaCBhZGRzIHNhbml0eSBjaGVja3Mg
b24gIndoZXJlIiBpbnB1dCBwYXJhbWV0ZXIgaW4NCj4gPiBkd19wY2llX2NmZ19yZWFkIGFuZCBk
d19wY2llX2NmZ193cml0ZS4gVGhlc2UgY2hlY2tzIG1ha2Ugc3VyZQ0KPiA+IHRoYXQgb2Zmc2V0
IHBhc3NlZCBpbiBieSB0aGUgY2FsbGVyIGlzIG5vdCBpbiBjb25mbGljdCB3aXRoDQo+ID4gdGhl
IHNpemUgb2YgdGhlIFBDSSBoZWFkZXIgZmllbGQgdGhhdCBpcyBiZWluZyByZWFkL3dyaXR0ZW4N
Cj4gPg0KPiANCj4gSSBhbSBzdGlsbCBub3QgY29udmluY2VkIHRoYXQgd2Ugc2hvdWxkIGRvdWJ0
IHRoZSBjYWxsZXIuLkJ1dCBtYXkgYmUgSQ0KPiBhbSBiaWFzZWQgaW4gbXkgdGhvdWdodHMuLi4N
Cj4gU2luY2UgQmpvcm4gaGFzIGFza2VkIGFib3V0IGl0LCBzbyB3aWxsIHRha2UgaXQuDQoNClll
cywgSSB0aGluayB0aGlzIHNob3VsZCBiZSBhIG1haW50YWluZXIgZGVjaXNpb24gYXMgaXQgaXMg
YSBtYXR0ZXIgb2YNCnRydXN0aW5nIHRoZSBjYWxsZXJzLi4uDQoNCj4gDQo+ID4gU2lnbmVkLW9m
Zi1ieTogR2FicmllbGUgUGFvbG9uaSA8Z2FicmllbGUucGFvbG9uaUBodWF3ZWkuY29tPg0KPiA+
IC0tLQ0KPiA+ICBkcml2ZXJzL3BjaS9ob3N0L3BjaWUtZGVzaWdud2FyZS5jIHwgMjAgKysrKysr
KysrKysrKystLS0tLS0NCj4gPiAgMSBmaWxlIGNoYW5nZWQsIDE0IGluc2VydGlvbnMoKyksIDYg
ZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvaG9zdC9wY2ll
LWRlc2lnbndhcmUuYw0KPiBiL2RyaXZlcnMvcGNpL2hvc3QvcGNpZS1kZXNpZ253YXJlLmMNCj4g
PiBpbmRleCBkNzcxZmE1Li43MTlkMmNkIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvcGNpL2hv
c3QvcGNpZS1kZXNpZ253YXJlLmMNCj4gPiArKysgYi9kcml2ZXJzL3BjaS9ob3N0L3BjaWUtZGVz
aWdud2FyZS5jDQo+ID4gQEAgLTgyLDExICs4MiwxNSBAQCBzdGF0aWMgaW5saW5lIHN0cnVjdCBw
Y2llX3BvcnQNCj4gKnN5c190b19wY2llKHN0cnVjdCBwY2lfc3lzX2RhdGEgKnN5cykNCj4gPg0K
PiA+ICBpbnQgZHdfcGNpZV9jZmdfcmVhZCh2b2lkIF9faW9tZW0gKmFkZHIsIGludCBzaXplLCB1
MzIgKnZhbCkNCj4gPiAgew0KPiANCj4gV291bGRuJ3QgYSBzaW5nbGUgY2hlY2sgd291bGQgaGF2
ZSBiZWVuIGJldHRlcg0KPiArICAgICAgIGlmICgodWludHB0cl90KWFkZHIgJiAoc2l6ZSAtMSkp
DQo+ICsgICAgICAgICAgICAgICByZXR1cm4gUENJQklPU19CQURfUkVHSVNURVJfTlVNQkVSOw0K
DQpPayBhZ3JlZWQgd2lsbCBjaGFuZ2UgaW4gdjYNCg0KVGhhbmtzDQoNCj4gDQo+IH5QcmF0eXVz
aA0K

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

end of thread, other threads:[~2015-09-29 15:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-29 13:57 [PATCH v5 0/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
2015-09-29 13:57 ` [PATCH v5 1/3] PCIe: SPEAr13xx: fix dw_pcie_cfg_read/write() usage Gabriele Paoloni
2015-09-29 13:57 ` [PATCH v5 2/3] PCI: designware: change dw_pcie_cfg_write() and dw_pcie_cfg_read() Gabriele Paoloni
2015-09-29 13:57 ` [PATCH v5 3/3] PCI: designware: add sanity checks on the header offset in dw_pcie_cfg_read and dw_pcie_cfg_write Gabriele Paoloni
2015-09-29 15:36   ` Pratyush Anand
2015-09-29 15:54     ` Gabriele Paoloni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.