public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ARM: mvebu: misc important fixes
@ 2013-11-25 16:26 Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2013-11-25 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

Jason, Andrew, Greg, Sebastian,

Here are four important fixes for Armada 370/XP support:

 * Two fixes for the irqchip driver, fixing a similar problem in the
   IPI and MSI implementation. I've splitted the patches in two,
   because the IPI side of the fix should normally be backported all
   the way to 3.8, while the MSI side of the fix is only needed in
   3.13, since MSI support didn't exist before. The IPI problem causes
   some really nasty SMP scheduling problems.

 * One fix for the I/O coherency, which should be backported all the
   way to 3.8.

 * One fix for the PCIe controller on Armada 370 DB, which should be
   backported to 3.12, where the problem was introduced.

Thanks to Lior Amsalem for debugging the IPI issue, and Gregory
Clement for finding the I/O coherency register address problem.

Thomas

Gregory CLEMENT (1):
  ARM: mvebu: use the virtual CPU registers to access coherency
    registers

Lior Amsalem (2):
  irqchip: armada-370-xp: fix IPI race condition
  irqchip: armada-370-xp: fix MSI race condition

Thomas Petazzoni (1):
  ARM: mvebu: re-enable PCIe on Armada 370 DB

 arch/arm/boot/dts/armada-370-db.dts  | 28 ++++++++++++++--------------
 arch/arm/boot/dts/armada-370-xp.dtsi |  2 +-
 drivers/irqchip/irq-armada-370-xp.c  |  4 ++--
 3 files changed, 17 insertions(+), 17 deletions(-)

-- 
1.8.1.2

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

* [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition
  2013-11-25 16:26 [PATCH 0/4] ARM: mvebu: misc important fixes Thomas Petazzoni
@ 2013-11-25 16:26 ` Thomas Petazzoni
  2013-11-25 18:17   ` Jason Cooper
  2013-12-12 22:23   ` Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 2/4] irqchip: armada-370-xp: fix MSI " Thomas Petazzoni
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2013-11-25 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Lior Amsalem <alior@marvell.com>

In the Armada 370/XP driver, when we receive an IRQ 0, we read the
list of doorbells that caused the interrupt from register
ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS. This gives the list of IPIs that
were generated. However, instead of acknowledging only the IPIs that
were generated, we acknowledge *all* the IPIs, by writing
~IPI_DOORBELL_MASK in the ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS register.

This creates a race condition: if a new IPI that isn't part of the
ones read into the temporary "ipimask" variable is fired before we
acknowledge all IPIs, then we will simply loose it. This is causing
scheduling hangs on SMP intensive workloads.

It is important to mention that this ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS
register has the following behavior: "A CPU write of 0 clears the bits
in this field. A CPU write of 1 has no effect". This is what allows us
to simply write ~ipimask to acknoledge the handled IPIs.

Notice that the same problem is present in the MSI implementation, but
it will be fixed as a separate patch, so that this IPI fix can be
pushed to older stable versions as appropriate (all the way to 3.8),
while the MSI code only appeared in 3.13.

Signed-off-by: Lior Amsalem <alior@marvell.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: stable at vger.kernel.org
Cc: Thomas Gleixner <tglx@linutronix.de>
---
The problem has been present since 344e873e5657e8dc0 ('arm: mvebu: Add
IPI support via doorbells'), that is since v3.8. However, notice that
the IRQ driver was moved from arch/arm/mach-mvebu/ to drivers/irqchip
in the process, and also that the very line being changed was slightly
modified in 5ec69017cc944f3ed8 ('irqchip: armada-370-xp: slightly
cleanup irq controller driver').
---
 drivers/irqchip/irq-armada-370-xp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index 433cc85..f5e49a2 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -407,7 +407,7 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
 						ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
 				& IPI_DOORBELL_MASK;
 
-			writel(~IPI_DOORBELL_MASK, per_cpu_int_base +
+			writel(~ipimask, per_cpu_int_base +
 				ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
 
 			/* Handle all pending doorbells */
-- 
1.8.1.2

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

* [PATCH 2/4] irqchip: armada-370-xp: fix MSI race condition
  2013-11-25 16:26 [PATCH 0/4] ARM: mvebu: misc important fixes Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
@ 2013-11-25 16:26 ` Thomas Petazzoni
  2013-11-25 18:18   ` Jason Cooper
  2013-11-25 16:26 ` [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB Thomas Petazzoni
  3 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2013-11-25 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Lior Amsalem <alior@marvell.com>

In the Armada 370/XP driver, when we receive an IRQ 1, we read the
list of doorbells that caused the interrupt from register
ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS. This gives the list of MSIs that
were generated. However, instead of acknowledging only the MSIs that
were generated, we acknowledge *all* the MSIs, by writing
~MSI_DOORBELL_MASK in the ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS register.

This creates a race condition: if a new MSI that isn't part of the
ones read into the temporary "msimask" variable is fired before we
acknowledge all MSIs, then we will simply loose it.

It is important to mention that this ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS
register has the following behavior: "A CPU write of 0 clears the bits
in this field. A CPU write of 1 has no effect". This is what allows us
to simply write ~msimask to acknoledge the handled MSIs.

Notice that the same problem is present in the IPI implementation, but
it is fixed as a separate patch, so that this IPI fix can be pushed to
older stable versions as appropriate (all the way to 3.8), while the
MSI code only appeared in 3.13.

Signed-off-by: Lior Amsalem <alior@marvell.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
This is only needed for 3.13, since the MSI support didn't exist in
earlier kernel versions.
---
 drivers/irqchip/irq-armada-370-xp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index f5e49a2..3fac063 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -381,7 +381,7 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
 						ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
 				& PCI_MSI_DOORBELL_MASK;
 
-			writel(~PCI_MSI_DOORBELL_MASK, per_cpu_int_base +
+			writel(~msimask, per_cpu_int_base +
 			       ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
 
 			for (msinr = PCI_MSI_DOORBELL_START;
-- 
1.8.1.2

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

* [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers
  2013-11-25 16:26 [PATCH 0/4] ARM: mvebu: misc important fixes Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
  2013-11-25 16:26 ` [PATCH 2/4] irqchip: armada-370-xp: fix MSI " Thomas Petazzoni
@ 2013-11-25 16:26 ` Thomas Petazzoni
  2013-11-25 18:23   ` Jason Cooper
  2013-11-25 16:26 ` [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB Thomas Petazzoni
  3 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2013-11-25 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

From: Gregory CLEMENT <gregory.clement@free-electrons.com>

The Armada XP provides a mechanism called "virtual CPU registers" or
"per-CPU register banking", to access the per-CPU registers of the
current CPU, without having to worry about finding on which CPU we're
running. CPU0 has its registers at 0x21800, CPU1 at 0x21900, CPU2 at
0x21A00 and CPU3 at 0x21B00. The virtual registers accessing the
current CPU registers are at 0x21000.

However, in the Device Tree node that provides the register addresses
for the coherency unit (which is responsible for ensuring coherency
between processors, and I/O coherency between processors and the
DMA-capable devices), a mistake was made: the CPU0-specific registers
were specified instead of the virtual CPU registers. This means that
the coherency barrier needed for I/O coherency was not behaving
properly when executed from a CPU different from CPU0. This patch
fixes that by using the virtual CPU registers.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: stable at vger.kernel.org
---
This bug has been introduced in e60304f8cb7bb5 ("arm: mvebu: Add
hardware I/O Coherency support"), merged in v3.8. Therefore this fix
should be backported into -stable all the way to 3.8. Due to various
reorganizations in the DT file, the patch will certainly not apply as
is, but the fix itself is trivial.
---
 arch/arm/boot/dts/armada-370-xp.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index 00d6a79..7f10f62 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -118,7 +118,7 @@
 
 			coherency-fabric at 20200 {
 				compatible = "marvell,coherency-fabric";
-				reg = <0x20200 0xb0>, <0x21810 0x1c>;
+				reg = <0x20200 0xb0>, <0x21010 0x1c>;
 			};
 
 			serial at 12000 {
-- 
1.8.1.2

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

* [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB
  2013-11-25 16:26 [PATCH 0/4] ARM: mvebu: misc important fixes Thomas Petazzoni
                   ` (2 preceding siblings ...)
  2013-11-25 16:26 ` [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers Thomas Petazzoni
@ 2013-11-25 16:26 ` Thomas Petazzoni
  2013-11-25 18:25   ` Jason Cooper
  3 siblings, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2013-11-25 16:26 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 14fd8ed0a7fd19913 ("ARM: mvebu: Relocate Armada 370/XP PCIe
device tree nodes") relocated the PCIe controller DT nodes one level
up in the Device Tree, to reflect a more correct representation of the
hardware introduced by the mvebu-mbus Device Tree binding.

However, while most of the boards were properly adjusted accordingly,
the Armada 370 DB board was left unchanged, and therefore, PCIe is
seen as not enabled on this board. This patch fixes that by moving the
PCIe controller node one level-up in armada-370-db.dts.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: stable at vger.kernel.org
---
This problem exists since kernel v3.12, and therefore this patch
should probably be pushed to v3.12 stable.
---
 arch/arm/boot/dts/armada-370-db.dts | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index 90ce29d..08a56bc 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -99,22 +99,22 @@
 					spi-max-frequency = <50000000>;
 				};
 			};
+		};
 
-			pcie-controller {
+		pcie-controller {
+			status = "okay";
+			/*
+			 * The two PCIe units are accessible through
+			 * both standard PCIe slots and mini-PCIe
+			 * slots on the board.
+			 */
+			pcie at 1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+			pcie at 2,0 {
+				/* Port 1, Lane 0 */
 				status = "okay";
-				/*
-				 * The two PCIe units are accessible through
-				 * both standard PCIe slots and mini-PCIe
-				 * slots on the board.
-				 */
-				pcie at 1,0 {
-					/* Port 0, Lane 0 */
-					status = "okay";
-				};
-				pcie at 2,0 {
-					/* Port 1, Lane 0 */
-					status = "okay";
-				};
 			};
 		};
 	};
-- 
1.8.1.2

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

* [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition
  2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
@ 2013-11-25 18:17   ` Jason Cooper
  2013-12-12 22:23   ` Thomas Petazzoni
  1 sibling, 0 replies; 12+ messages in thread
From: Jason Cooper @ 2013-11-25 18:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 25, 2013 at 05:26:44PM +0100, Thomas Petazzoni wrote:
> From: Lior Amsalem <alior@marvell.com>
> 
> In the Armada 370/XP driver, when we receive an IRQ 0, we read the
> list of doorbells that caused the interrupt from register
> ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS. This gives the list of IPIs that
> were generated. However, instead of acknowledging only the IPIs that
> were generated, we acknowledge *all* the IPIs, by writing
> ~IPI_DOORBELL_MASK in the ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS register.
> 
> This creates a race condition: if a new IPI that isn't part of the
> ones read into the temporary "ipimask" variable is fired before we
> acknowledge all IPIs, then we will simply loose it. This is causing
> scheduling hangs on SMP intensive workloads.
> 
> It is important to mention that this ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS
> register has the following behavior: "A CPU write of 0 clears the bits
> in this field. A CPU write of 1 has no effect". This is what allows us
> to simply write ~ipimask to acknoledge the handled IPIs.
> 
> Notice that the same problem is present in the MSI implementation, but
> it will be fixed as a separate patch, so that this IPI fix can be
> pushed to older stable versions as appropriate (all the way to 3.8),
> while the MSI code only appeared in 3.13.
> 
> Signed-off-by: Lior Amsalem <alior@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: stable at vger.kernel.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> The problem has been present since 344e873e5657e8dc0 ('arm: mvebu: Add
> IPI support via doorbells'), that is since v3.8. However, notice that
> the IRQ driver was moved from arch/arm/mach-mvebu/ to drivers/irqchip
> in the process, and also that the very line being changed was slightly
> modified in 5ec69017cc944f3ed8 ('irqchip: armada-370-xp: slightly
> cleanup irq controller driver').
> ---
>  drivers/irqchip/irq-armada-370-xp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

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

* [PATCH 2/4] irqchip: armada-370-xp: fix MSI race condition
  2013-11-25 16:26 ` [PATCH 2/4] irqchip: armada-370-xp: fix MSI " Thomas Petazzoni
@ 2013-11-25 18:18   ` Jason Cooper
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Cooper @ 2013-11-25 18:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 25, 2013 at 05:26:45PM +0100, Thomas Petazzoni wrote:
> From: Lior Amsalem <alior@marvell.com>
> 
> In the Armada 370/XP driver, when we receive an IRQ 1, we read the
> list of doorbells that caused the interrupt from register
> ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS. This gives the list of MSIs that
> were generated. However, instead of acknowledging only the MSIs that
> were generated, we acknowledge *all* the MSIs, by writing
> ~MSI_DOORBELL_MASK in the ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS register.
> 
> This creates a race condition: if a new MSI that isn't part of the
> ones read into the temporary "msimask" variable is fired before we
> acknowledge all MSIs, then we will simply loose it.
> 
> It is important to mention that this ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS
> register has the following behavior: "A CPU write of 0 clears the bits
> in this field. A CPU write of 1 has no effect". This is what allows us
> to simply write ~msimask to acknoledge the handled MSIs.
> 
> Notice that the same problem is present in the IPI implementation, but
> it is fixed as a separate patch, so that this IPI fix can be pushed to
> older stable versions as appropriate (all the way to 3.8), while the
> MSI code only appeared in 3.13.
> 
> Signed-off-by: Lior Amsalem <alior@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> This is only needed for 3.13, since the MSI support didn't exist in
> earlier kernel versions.
> ---
>  drivers/irqchip/irq-armada-370-xp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

Jason.

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

* [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers
  2013-11-25 16:26 ` [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers Thomas Petazzoni
@ 2013-11-25 18:23   ` Jason Cooper
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Cooper @ 2013-11-25 18:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 25, 2013 at 05:26:46PM +0100, Thomas Petazzoni wrote:
> From: Gregory CLEMENT <gregory.clement@free-electrons.com>
> 
> The Armada XP provides a mechanism called "virtual CPU registers" or
> "per-CPU register banking", to access the per-CPU registers of the
> current CPU, without having to worry about finding on which CPU we're
> running. CPU0 has its registers at 0x21800, CPU1 at 0x21900, CPU2 at
> 0x21A00 and CPU3 at 0x21B00. The virtual registers accessing the
> current CPU registers are at 0x21000.
> 
> However, in the Device Tree node that provides the register addresses
> for the coherency unit (which is responsible for ensuring coherency
> between processors, and I/O coherency between processors and the
> DMA-capable devices), a mistake was made: the CPU0-specific registers
> were specified instead of the virtual CPU registers. This means that
> the coherency barrier needed for I/O coherency was not behaving
> properly when executed from a CPU different from CPU0. This patch
> fixes that by using the virtual CPU registers.
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: stable at vger.kernel.org
> ---
> This bug has been introduced in e60304f8cb7bb5 ("arm: mvebu: Add
> hardware I/O Coherency support"), merged in v3.8. Therefore this fix
> should be backported into -stable all the way to 3.8. Due to various
> reorganizations in the DT file, the patch will certainly not apply as
> is, but the fix itself is trivial.
> ---
>  arch/arm/boot/dts/armada-370-xp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to mvebu/dt-fixes

thx,

Jason.

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

* [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB
  2013-11-25 16:26 ` [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB Thomas Petazzoni
@ 2013-11-25 18:25   ` Jason Cooper
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Cooper @ 2013-11-25 18:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Nov 25, 2013 at 05:26:47PM +0100, Thomas Petazzoni wrote:
> Commit 14fd8ed0a7fd19913 ("ARM: mvebu: Relocate Armada 370/XP PCIe
> device tree nodes") relocated the PCIe controller DT nodes one level
> up in the Device Tree, to reflect a more correct representation of the
> hardware introduced by the mvebu-mbus Device Tree binding.
> 
> However, while most of the boards were properly adjusted accordingly,
> the Armada 370 DB board was left unchanged, and therefore, PCIe is
> seen as not enabled on this board. This patch fixes that by moving the
> PCIe controller node one level-up in armada-370-db.dts.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: stable at vger.kernel.org
> ---
> This problem exists since kernel v3.12, and therefore this patch
> should probably be pushed to v3.12 stable.
> ---
>  arch/arm/boot/dts/armada-370-db.dts | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

Applied to mvebu/dt-fixes

thx,

Jason.

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

* [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition
  2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
  2013-11-25 18:17   ` Jason Cooper
@ 2013-12-12 22:23   ` Thomas Petazzoni
  2013-12-13 16:38     ` Jason Cooper
  1 sibling, 1 reply; 12+ messages in thread
From: Thomas Petazzoni @ 2013-12-12 22:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Thomas,

As the irqchip maintainer, this patch, as well as the following patch
"irqchip: armada-370-xp: fix MSI race condition" should go through your
tree.

They have been sent for quite some time now. Is there any reason you
haven't picked them up? I realized I only Cc'ed you when sending them
originally, while you should have been the main recipient, maybe that
explains why I didn't get any feedback.

Would it be possible to merge them for 3.13 ? Notice that the below
patch should also be backported into stable kernel all the way to 3.8.

Thanks a lot!

Thomas

On Mon, 25 Nov 2013 17:26:44 +0100, Thomas Petazzoni wrote:
> From: Lior Amsalem <alior@marvell.com>
> 
> In the Armada 370/XP driver, when we receive an IRQ 0, we read the
> list of doorbells that caused the interrupt from register
> ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS. This gives the list of IPIs that
> were generated. However, instead of acknowledging only the IPIs that
> were generated, we acknowledge *all* the IPIs, by writing
> ~IPI_DOORBELL_MASK in the ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS register.
> 
> This creates a race condition: if a new IPI that isn't part of the
> ones read into the temporary "ipimask" variable is fired before we
> acknowledge all IPIs, then we will simply loose it. This is causing
> scheduling hangs on SMP intensive workloads.
> 
> It is important to mention that this ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS
> register has the following behavior: "A CPU write of 0 clears the bits
> in this field. A CPU write of 1 has no effect". This is what allows us
> to simply write ~ipimask to acknoledge the handled IPIs.
> 
> Notice that the same problem is present in the MSI implementation, but
> it will be fixed as a separate patch, so that this IPI fix can be
> pushed to older stable versions as appropriate (all the way to 3.8),
> while the MSI code only appeared in 3.13.
> 
> Signed-off-by: Lior Amsalem <alior@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: stable at vger.kernel.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---
> The problem has been present since 344e873e5657e8dc0 ('arm: mvebu: Add
> IPI support via doorbells'), that is since v3.8. However, notice that
> the IRQ driver was moved from arch/arm/mach-mvebu/ to drivers/irqchip
> in the process, and also that the very line being changed was slightly
> modified in 5ec69017cc944f3ed8 ('irqchip: armada-370-xp: slightly
> cleanup irq controller driver').
> ---
>  drivers/irqchip/irq-armada-370-xp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
> index 433cc85..f5e49a2 100644
> --- a/drivers/irqchip/irq-armada-370-xp.c
> +++ b/drivers/irqchip/irq-armada-370-xp.c
> @@ -407,7 +407,7 @@ armada_370_xp_handle_irq(struct pt_regs *regs)
>  						ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS)
>  				& IPI_DOORBELL_MASK;
>  
> -			writel(~IPI_DOORBELL_MASK, per_cpu_int_base +
> +			writel(~ipimask, per_cpu_int_base +
>  				ARMADA_370_XP_IN_DRBEL_CAUSE_OFFS);
>  
>  			/* Handle all pending doorbells */



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition
  2013-12-12 22:23   ` Thomas Petazzoni
@ 2013-12-13 16:38     ` Jason Cooper
  2013-12-14 17:35       ` Thomas Petazzoni
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Cooper @ 2013-12-13 16:38 UTC (permalink / raw)
  To: linux-arm-kernel

Thomass :)

On Thu, Dec 12, 2013 at 11:23:27PM +0100, Thomas Petazzoni wrote:
> Hello Thomas,
> 
> As the irqchip maintainer, this patch, as well as the following patch
> "irqchip: armada-370-xp: fix MSI race condition" should go through your
> tree.
> 
> They have been sent for quite some time now. Is there any reason you
> haven't picked them up? I realized I only Cc'ed you when sending them
> originally, while you should have been the main recipient, maybe that
> explains why I didn't get any feedback.
> 
> Would it be possible to merge them for 3.13 ? Notice that the below
> patch should also be backported into stable kernel all the way to 3.8.

oops.  I'm maintaining an mvebu topic branch for irqchip changes, and
forgot to look in my -fixes mail folder when putting it together.  I'll
take both of these into mvebu/irqchip-fixes and send a pull request to
Thomas in a few days.

Added:

Cc: stable at vger.kernel.org # v3.8+
Fixes: 344e873e5657e8dc0 'arm: mvebu: Add IPI support via doorbells'

to this patch.

thx,

Jason.

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

* [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition
  2013-12-13 16:38     ` Jason Cooper
@ 2013-12-14 17:35       ` Thomas Petazzoni
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2013-12-14 17:35 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Jason Cooper,

On Fri, 13 Dec 2013 11:38:54 -0500, Jason Cooper wrote:

> > Would it be possible to merge them for 3.13 ? Notice that the below
> > patch should also be backported into stable kernel all the way to 3.8.
> 
> oops.  I'm maintaining an mvebu topic branch for irqchip changes, and
> forgot to look in my -fixes mail folder when putting it together.  I'll
> take both of these into mvebu/irqchip-fixes and send a pull request to
> Thomas in a few days.
> 
> Added:
> 
> Cc: stable at vger.kernel.org # v3.8+
> Fixes: 344e873e5657e8dc0 'arm: mvebu: Add IPI support via doorbells'
> 
> to this patch.

Great, thanks! Note that the patch will not apply as is on 3.8, but I
will be able to provide a backported variant of it when needed.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

end of thread, other threads:[~2013-12-14 17:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-25 16:26 [PATCH 0/4] ARM: mvebu: misc important fixes Thomas Petazzoni
2013-11-25 16:26 ` [PATCH 1/4] irqchip: armada-370-xp: fix IPI race condition Thomas Petazzoni
2013-11-25 18:17   ` Jason Cooper
2013-12-12 22:23   ` Thomas Petazzoni
2013-12-13 16:38     ` Jason Cooper
2013-12-14 17:35       ` Thomas Petazzoni
2013-11-25 16:26 ` [PATCH 2/4] irqchip: armada-370-xp: fix MSI " Thomas Petazzoni
2013-11-25 18:18   ` Jason Cooper
2013-11-25 16:26 ` [PATCH 3/4] ARM: mvebu: use the virtual CPU registers to access coherency registers Thomas Petazzoni
2013-11-25 18:23   ` Jason Cooper
2013-11-25 16:26 ` [PATCH 4/4] ARM: mvebu: re-enable PCIe on Armada 370 DB Thomas Petazzoni
2013-11-25 18:25   ` Jason Cooper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox