Skip to contentSkip to conversation

PCI: designware: Fix dw_handle_msi_irq() on 64-bit BE configs

PATCH In review Ben Hutchings 3 participants 3 messages Jul 20, 2017 – Aug 1, 2017 PCI

lore
Share & tools
lore

Ben Hutchings

author
#0 lore raw

PATCH 1/1 PCI: designware: Fix dw_handle_msi_irq() on 64-bit BE configs

Commit 1b497e6493c4 "PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()" fixed one problem for 64-bit architectures but left another. The find_next_bit() function assumes native ordering of bits within each word (unsigned long), so passing a pointer to a u32 variable will cause it to scan the following 32 bits on a 64-bit big-endian configuration. Alternately it could result in an alignment fault on some architectures.

Copy the status to an unsigned long variable before using find_next_bit().

Fixes 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in ...")
Signed-off-by Ben Hutchings <[email protected]>

---
This is compile-tested only.

Ben.

 drivers/pci/dwc/pcie-designware-host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/dwc/pcie-designware-host.c +3 -2
index d29c020da082..511fc9188f98 100644
@@ -57,6 +57,7 @@ static struct irq_chip dw_msi_irq_chip = {
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
 	u32 val;
+	unsigned long bits;
 	int i, pos, irq;
 	irqreturn_t ret = IRQ_NONE;
 

@@ -65,11 +66,11 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 				    &val);
 		if (!val)
 			continue;
+		bits = val;
 
 		ret = IRQ_HANDLED;
 		pos = 0;
-		while ((pos = find_next_bit((unsigned long *) &val, 32,
-					    pos)) != 32) {
+		while ((pos = find_next_bit(&bits, 32, pos)) != 32) {
 			irq = irq_find_mapping(pp->irq_domain, i * 32 + pos);
 			dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12,
 					    4, 1 << pos);
#2 lore raw
In reply to Ben Hutchings

Hi Ben,

Às 12:46 AM de 7/21/2017, Ben Hutchings escreveu:

Quoted 26 lines

Commit 1b497e6493c4 "PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()" fixed one problem for 64-bit architectures but left another. The find_next_bit() function assumes native ordering of bits within each word (unsigned long), so passing a pointer to a u32 variable will cause it to scan the following 32 bits on a 64-bit big-endian configuration. Alternately it could result in an alignment fault on some architectures.

Copy the status to an unsigned long variable before using find_next_bit().

Fixes 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in ...")
Signed-off-by Ben Hutchings <[email protected]>

---
This is compile-tested only.

Ben.

 drivers/pci/dwc/pcie-designware-host.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/pci/dwc/pcie-designware-host.c +0 -0
index d29c020da082..511fc9188f98 100644

drivers/pci/dwc/pcie-designware-host.c static struct irq_chip dw_msi_irq_chip = { · line 60

Quoted 4 lines
drivers/pci/dwc/pcie-designware-host.c +1 -0
@@ -57,6 +57,7 @@ static struct irq_chip dw_msi_irq_chip = {
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
 	u32 val;
+	unsigned long bits;

Could you please declare the variable in a ordered fashion (after "int i, pos, irq;")? I suggest to change the variable name to "status" and val would be a temporary variable only.

Quoted 8 lines
drivers/pci/dwc/pcie-designware-host.c +0 -0
 	int i, pos, irq;
 	irqreturn_t ret = IRQ_NONE;
 

@@ -65,11 +66,11 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 				    &val);
 		if (!val)
 			continue;

drivers/pci/dwc/pcie-designware-host.c irqreturn_t dw_handle_msi_irq(struct pcie_port *pp) · line 76

Quoted 10 lines
drivers/pci/dwc/pcie-designware-host.c +2 -2
+		bits = val;
 
 		ret = IRQ_HANDLED;
 		pos = 0;
-		while ((pos = find_next_bit((unsigned long *) &val, 32,
-					    pos)) != 32) {
+		while ((pos = find_next_bit(&bits, 32, pos)) != 32) {
 			irq = irq_find_mapping(pp->irq_domain, i * 32 + pos);
 			dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12,
 					    4, 1 << pos);

Thanks,
Joao