dmaengine: mv_xor: protect MBUS window access with a spinlock
PATCH 1/1 dmaengine: mv_xor: protect MBUS window access with a spinlock
mv_xor_add_io_win() reads and writes shared MBUS window registers and updates the shared win_start/win_end arrays in mv_xor_device. Multiple DMA channels can call this function concurrently via mv_xor_prep_dma_xor(), leading to races where two threads can select the same free window slot, corrupt the registers, or produce inconsistent cached state.
Add a spinlock to struct mv_xor_device and hold it across the register read-modify-write and cache update in mv_xor_add_io_win().
--- drivers/dma/mv_xor.c | 15 +++++++++++++-- drivers/dma/mv_xor.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)
drivers/dma/mv_xor.c +13 -2
@@ -497,11 +497,14 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)u8 target, attr;int ret;int i; +unsigned long flags;/* Nothing needs to get done for the Armada 3700 */ if (xordev->xor_type ==XOR_ARMADA_37XX )return 0 ; +spin_lock_irqsave (&xordev->win_lock, flags); +/* * Loop over the cached windows to check, if the requested area * is already mapped. If this the case, nothing needs to be done
@@ -511,6 +514,7 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)if (addr >= xordev->win_start[i] && addr <= xordev->win_end[i]) {/* Window is already mapped */ +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; } }
@@ -521,8 +525,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* If no IO window is found that addr has to be located in SDRAM */ ret =mvebu_mbus_get_io_win_info (addr, &size, &target, &attr); -if (ret <0 ) +if (ret <0 ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; + }/* * Mask the base addr 'addr' according to 'size' read back from the
@@ -540,8 +546,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* Set 'i' to the first free window to write the new values to */ i =ffs (~win_enable) -1 ; -if (i >=WINDOW_COUNT ) +if (i >=WINDOW_COUNT ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return -ENOMEM ; + }writel ((addr &0xffff0000 ) | (attr <<8 ) | target, base +WINDOW_BASE (i));
@@ -556,6 +564,8 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)writel (win_enable, base +WINDOW_BAR_ENABLE (0 ));writel (win_enable, base +WINDOW_BAR_ENABLE (1 )); +spin_unlock_irqrestore (&xordev->win_lock, flags); +return 0 ; }
@@ -1353,6 +1363,7 @@ static int mv_xor_probe(struct platform_device *pdev)platform_set_drvdata (pdev, xordev); +spin_lock_init (&xordev->win_lock);/* * We need to know which type of XOR device we use before
drivers/dma/mv_xor.h +1 -0
@@ -80,6 +80,7 @@ struct mv_xor_device {struct clk *clk;struct mv_xor_chan *channels[MV_XOR_MAX_CHANNELS ];int xor_type; +spinlock_t win_lock;u32 win_start[WINDOW_COUNT ];u32 win_end[WINDOW_COUNT ];
Signature · 1 line
2.55.0
Quoted 24 lines · Rosen Penev
mv_xor_add_io_win()reads and writes shared MBUS window registers and updates the shared win_start/win_end arrays inmv_xor_device. Multiple DMA channels can call this function concurrently viamv_xor_prep_dma_xor(), leading to races where two threads can select the same free window slot, corrupt the registers, or produce inconsistent cached state.Add a spinlock to struct
mv_xor_deviceand hold it across the register read-modify-write and cache update inmv_xor_add_io_win().Fixes 77ff7a706f01 ("mv_xor: Add support for IO (PCIe) src/dst areas")Assisted-by LLMSigned-off-by Rosen Penev <[email protected]>--- drivers/dma/mv_xor.c | 15 +++++++++++++-- drivers/dma/mv_xor.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)drivers/dma/mv_xor.c +0
-0@@ -497,11 +497,14 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)u8 target, attr;
drivers/dma/mv_xor.c static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr) · line 507
Quoted 10 lines
drivers/dma/mv_xor.c +3
-0int ret;int i; +unsigned long flags;/* Nothing needs to get done for the Armada 3700 */ if (xordev->xor_type ==XOR_ARMADA_37XX )return 0 ; +spin_lock_irqsave (&xordev->win_lock, flags); +
use auto clean up guard()
Frank
Quoted 67 lines
drivers/dma/mv_xor.c +10
-2/* * Loop over the cached windows to check, if the requested area * is already mapped. If this the case, nothing needs to be done @@ -511,6 +514,7 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)if (addr >= xordev->win_start[i] && addr <= xordev->win_end[i]) {/* Window is already mapped */ +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; } }@@ -521,8 +525,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* If no IO window is found that addr has to be located in SDRAM */ ret =mvebu_mbus_get_io_win_info (addr, &size, &target, &attr); -if (ret <0 ) +if (ret <0 ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; + }/* * Mask the base addr 'addr' according to 'size' read back from the @@ -540,8 +546,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* Set 'i' to the first free window to write the new values to */ i =ffs (~win_enable) -1 ; -if (i >=WINDOW_COUNT ) +if (i >=WINDOW_COUNT ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return -ENOMEM ; + }writel ((addr &0xffff0000 ) | (attr <<8 ) | target, base +WINDOW_BASE (i));@@ -556,6 +564,8 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)writel (win_enable, base +WINDOW_BAR_ENABLE (0 ));writel (win_enable, base +WINDOW_BAR_ENABLE (1 )); +spin_unlock_irqrestore (&xordev->win_lock, flags); +return 0 ; }@@ -1353,6 +1363,7 @@ static int mv_xor_probe(struct platform_device *pdev)platform_set_drvdata (pdev, xordev); +spin_lock_init (&xordev->win_lock);/* * We need to know which type of XOR device we use before drivers/dma/mv_xor.h +1
-1@@ -80,6 +80,7 @@ struct mv_xor_device {struct clk *clk;struct mv_xor_chan *channels[MV_XOR_MAX_CHANNELS ];int xor_type; +spinlock_t win_lock;u32 win_start[WINDOW_COUNT ];u32 win_end[WINDOW_COUNT ]; --2.55.0
Quoted 30 lines · Frank Li
Quoted 28 lines · Rosen Penev
mv_xor_add_io_win()reads and writes shared MBUS window registers and updates the shared win_start/win_end arrays inmv_xor_device. Multiple DMA channels can call this function concurrently viamv_xor_prep_dma_xor(), leading to races where two threads can select the same free window slot, corrupt the registers, or produce inconsistent cached state.Add a spinlock to struct
mv_xor_deviceand hold it across the register read-modify-write and cache update inmv_xor_add_io_win().Fixes 77ff7a706f01 ("mv_xor: Add support for IO (PCIe) src/dst areas")Assisted-by LLMSigned-off-by Rosen Penev <[email protected]>--- drivers/dma/mv_xor.c | 15 +++++++++++++-- drivers/dma/mv_xor.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)drivers/dma/mv_xor.c +1
-0@@ -497,11 +497,14 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)u8 target, attr;int ret;int i; +unsigned long flags;
Quoted 8 lines
drivers/dma/mv_xor.c static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr) · line 507
Quoted 6 lines
drivers/dma/mv_xor.c +2
-0/* Nothing needs to get done for the Armada 3700 */ if (xordev->xor_type ==XOR_ARMADA_37XX )return 0 ; +spin_lock_irqsave (&xordev->win_lock, flags); +use auto clean up
guard()
Could that be backported though?
Quoted 69 lines
Frank
Quoted 67 lines
drivers/dma/mv_xor.c +10
-2/* * Loop over the cached windows to check, if the requested area * is already mapped. If this the case, nothing needs to be done @@ -511,6 +514,7 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)if (addr >= xordev->win_start[i] && addr <= xordev->win_end[i]) {/* Window is already mapped */ +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; } }@@ -521,8 +525,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* If no IO window is found that addr has to be located in SDRAM */ ret =mvebu_mbus_get_io_win_info (addr, &size, &target, &attr); -if (ret <0 ) +if (ret <0 ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; + }/* * Mask the base addr 'addr' according to 'size' read back from the @@ -540,8 +546,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* Set 'i' to the first free window to write the new values to */ i =ffs (~win_enable) -1 ; -if (i >=WINDOW_COUNT ) +if (i >=WINDOW_COUNT ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return -ENOMEM ; + }writel ((addr &0xffff0000 ) | (attr <<8 ) | target, base +WINDOW_BASE (i));@@ -556,6 +564,8 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)writel (win_enable, base +WINDOW_BAR_ENABLE (0 ));writel (win_enable, base +WINDOW_BAR_ENABLE (1 )); +spin_unlock_irqrestore (&xordev->win_lock, flags); +return 0 ; }@@ -1353,6 +1363,7 @@ static int mv_xor_probe(struct platform_device *pdev)platform_set_drvdata (pdev, xordev); +spin_lock_init (&xordev->win_lock);/* * We need to know which type of XOR device we use before drivers/dma/mv_xor.h +1
-1@@ -80,6 +80,7 @@ struct mv_xor_device {struct clk *clk;struct mv_xor_chan *channels[MV_XOR_MAX_CHANNELS ];int xor_type; +spinlock_t win_lock;u32 win_start[WINDOW_COUNT ];u32 win_end[WINDOW_COUNT ]; --2.55.0
Quoted 35 lines · Rosen Penev
Quoted 34 lines · Frank Li
Quoted 32 lines · Rosen Penev
mv_xor_add_io_win()reads and writes shared MBUS window registers and updates the shared win_start/win_end arrays inmv_xor_device. Multiple DMA channels can call this function concurrently viamv_xor_prep_dma_xor(), leading to races where two threads can select the same free window slot, corrupt the registers, or produce inconsistent cached state.Add a spinlock to struct
mv_xor_deviceand hold it across the register read-modify-write and cache update inmv_xor_add_io_win().Fixes 77ff7a706f01 ("mv_xor: Add support for IO (PCIe) src/dst areas")Assisted-by LLMSigned-off-by Rosen Penev <[email protected]>--- drivers/dma/mv_xor.c | 15 +++++++++++++-- drivers/dma/mv_xor.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)drivers/dma/mv_xor.c +1
-0@@ -497,11 +497,14 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)u8 target, attr;int ret;int i; +unsigned long flags;/* Nothing needs to get done for the Armada 3700 */ if (xordev->xor_type ==XOR_ARMADA_37XX )return 0 ;
Quoted 5 lines
Quoted 4 lines
drivers/dma/mv_xor.c static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr) · line 507
Quoted 2 lines
drivers/dma/mv_xor.c +2
-0+spin_lock_irqsave (&xordev->win_lock, flags); +use auto clean up
guard()Could that be backported though?
I think it should be fine now. guard() already use quick popular.
Frank
Quoted 69 lines
Quoted 69 lines
Frank
Quoted 67 lines
drivers/dma/mv_xor.c +10
-2/* * Loop over the cached windows to check, if the requested area * is already mapped. If this the case, nothing needs to be done @@ -511,6 +514,7 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)if (addr >= xordev->win_start[i] && addr <= xordev->win_end[i]) {/* Window is already mapped */ +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; } }@@ -521,8 +525,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* If no IO window is found that addr has to be located in SDRAM */ ret =mvebu_mbus_get_io_win_info (addr, &size, &target, &attr); -if (ret <0 ) +if (ret <0 ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return 0 ; + }/* * Mask the base addr 'addr' according to 'size' read back from the @@ -540,8 +546,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)/* Set 'i' to the first free window to write the new values to */ i =ffs (~win_enable) -1 ; -if (i >=WINDOW_COUNT ) +if (i >=WINDOW_COUNT ) { +spin_unlock_irqrestore (&xordev->win_lock, flags);return -ENOMEM ; + }writel ((addr &0xffff0000 ) | (attr <<8 ) | target, base +WINDOW_BASE (i));@@ -556,6 +564,8 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)writel (win_enable, base +WINDOW_BAR_ENABLE (0 ));writel (win_enable, base +WINDOW_BAR_ENABLE (1 )); +spin_unlock_irqrestore (&xordev->win_lock, flags); +return 0 ; }@@ -1353,6 +1363,7 @@ static int mv_xor_probe(struct platform_device *pdev)platform_set_drvdata (pdev, xordev); +spin_lock_init (&xordev->win_lock);/* * We need to know which type of XOR device we use before drivers/dma/mv_xor.h +1
-1@@ -80,6 +80,7 @@ struct mv_xor_device {struct clk *clk;struct mv_xor_chan *channels[MV_XOR_MAX_CHANNELS ];int xor_type; +spinlock_t win_lock;u32 win_start[WINDOW_COUNT ];u32 win_end[WINDOW_COUNT ]; --2.55.0