Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580346 > unrolled thread

[PATCH v4 0/4] Broadcom SBA RAID support

Started byAnup Patel <anup.patel@broadcom.com>
First post2017-02-14 08:00 +0100
Last post2017-02-15 09:40 +0100
Articles 9 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/4] Broadcom SBA RAID support Anup Patel <anup.patel@broadcom.com> - 2017-02-14 08:00 +0100
    [PATCH v4 1/4] lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position Anup Patel <anup.patel@broadcom.com> - 2017-02-14 08:00 +0100
    [PATCH v4 4/4] dt-bindings: Add DT bindings document for Broadcom SBA RAID driver Anup Patel <anup.patel@broadcom.com> - 2017-02-14 08:00 +0100
    Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Dan Williams <dan.j.williams@intel.com> - 2017-02-14 17:40 +0100
      Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Anup Patel <anup.patel@broadcom.com> - 2017-02-15 07:30 +0100
        Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Dan Williams <dan.j.williams@intel.com> - 2017-02-15 07:50 +0100
          Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Anup Patel <anup.patel@broadcom.com> - 2017-02-15 08:10 +0100
            Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Dan Williams <dan.j.williams@intel.com> - 2017-02-15 08:30 +0100
              Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver Anup Patel <anup.patel@broadcom.com> - 2017-02-15 09:40 +0100

#1580346 — [PATCH v4 0/4] Broadcom SBA RAID support

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-14 08:00 +0100
Subject[PATCH v4 0/4] Broadcom SBA RAID support
Message-ID<taFfI-6FF-7@gated-at.bofh.it>
The Broadcom SBA RAID is a stream-based device which provides
RAID5/6 offload.

It requires a SoC specific ring manager (such as Broadcom FlexRM
ring manager) to provide ring-based programming interface. Due to
this, the Broadcom SBA RAID driver (mailbox client) implements
DMA device having one DMA channel using a set of mailbox channels
provided by Broadcom SoC specific ring manager driver (mailbox
controller).

The Broadcom SBA RAID hardware requires PQ disk position instead
of PQ disk coefficient. To address this, we have added raid_gflog
table which will help driver to convert PQ disk coefficient to PQ
disk position.

This patchset is based on Linux-4.10-rc2 and depends on patchset
"[PATCH v4 0/2] Broadcom FlexRM ring manager support"

It is also available at sba-raid-v4 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v3:
 - Replaced SBA_ENC() with sba_cmd_enc() inline function
 - Use list_first_entry_or_null() wherever possible
 - Remove unwanted brances around loops wherever possible
 - Use lockdep_assert_held() where required

Changes since v2:
 - Droped patch to handle DMA devices having support for fewer
   PQ coefficients in Linux Async Tx
 - Added work-around in bcm-sba-raid driver to handle unsupported
   PQ coefficients using multiple SBA requests

Changes since v1:
 - Droped patch to add mbox_channel_device() API
 - Used GENMASK and BIT macros wherever possible in bcm-sba-raid driver
 - Replaced C_MDATA macros with static inline functions in
   bcm-sba-raid driver
 - Removed sba_alloc_chan_resources() callback in bcm-sba-raid driver
 - Used dev_err() instead of dev_info() wherever applicable
 - Removed call to sba_issue_pending() from sba_tx_submit() in
   bcm-sba-raid driver
 - Implemented SBA request chaning for handling (len > sba->req_size)
   in bcm-sba-raid driver
 - Implemented device_terminate_all() callback in bcm-sba-raid driver

Anup Patel (4):
  lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
  async_tx: Fix DMA_PREP_FENCE usage in do_async_gen_syndrome()
  dmaengine: Add Broadcom SBA RAID driver
  dt-bindings: Add DT bindings document for Broadcom SBA RAID driver

 .../devicetree/bindings/dma/brcm,iproc-sba.txt     |   29 +
 crypto/async_tx/async_pq.c                         |    5 +-
 drivers/dma/Kconfig                                |   13 +
 drivers/dma/Makefile                               |    1 +
 drivers/dma/bcm-sba-raid.c                         | 1694 ++++++++++++++++++++
 include/linux/raid/pq.h                            |    1 +
 lib/raid6/mktables.c                               |   20 +
 7 files changed, 1760 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dma/brcm,iproc-sba.txt
 create mode 100644 drivers/dma/bcm-sba-raid.c

-- 
2.7.4

[toc] | [next] | [standalone]

#1580348 — [PATCH v4 1/4] lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-14 08:00 +0100
Subject[PATCH v4 1/4] lib/raid6: Add log-of-2 table for RAID6 HW requiring disk position
Message-ID<taFfJ-6FF-17@gated-at.bofh.it>
In reply to#1580346
The raid6_gfexp table represents {2}^n values for 0 <= n < 256. The
Linux async_tx framework pass values from raid6_gfexp as coefficients
for each source to prep_dma_pq() callback of DMA channel with PQ
capability. This creates problem for RAID6 offload engines (such as
Broadcom SBA) which take disk position (i.e. log of {2}) instead of
multiplicative cofficients from raid6_gfexp table.

This patch adds raid6_gflog table having log-of-2 value for any given
x such that 0 <= x < 256. For any given disk coefficient x, the
corresponding disk position is given by raid6_gflog[x]. The RAID6
offload engine driver can use this newly added raid6_gflog table to
get disk position from multiplicative coefficient.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
---
 include/linux/raid/pq.h |  1 +
 lib/raid6/mktables.c    | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/raid/pq.h b/include/linux/raid/pq.h
index 4d57bba..30f9453 100644
--- a/include/linux/raid/pq.h
+++ b/include/linux/raid/pq.h
@@ -142,6 +142,7 @@ int raid6_select_algo(void);
 extern const u8 raid6_gfmul[256][256] __attribute__((aligned(256)));
 extern const u8 raid6_vgfmul[256][32] __attribute__((aligned(256)));
 extern const u8 raid6_gfexp[256]      __attribute__((aligned(256)));
+extern const u8 raid6_gflog[256]      __attribute__((aligned(256)));
 extern const u8 raid6_gfinv[256]      __attribute__((aligned(256)));
 extern const u8 raid6_gfexi[256]      __attribute__((aligned(256)));
 
diff --git a/lib/raid6/mktables.c b/lib/raid6/mktables.c
index 39787db..e824d08 100644
--- a/lib/raid6/mktables.c
+++ b/lib/raid6/mktables.c
@@ -125,6 +125,26 @@ int main(int argc, char *argv[])
 	printf("EXPORT_SYMBOL(raid6_gfexp);\n");
 	printf("#endif\n");
 
+	/* Compute log-of-2 table */
+	printf("\nconst u8 __attribute__((aligned(256)))\n"
+	       "raid6_gflog[256] =\n" "{\n");
+	for (i = 0; i < 256; i += 8) {
+		printf("\t");
+		for (j = 0; j < 8; j++) {
+			v = 255;
+			for (k = 0; k < 256; k++)
+				if (exptbl[k] == (i + j)) {
+					v = k;
+					break;
+				}
+			printf("0x%02x,%c", v, (j == 7) ? '\n' : ' ');
+		}
+	}
+	printf("};\n");
+	printf("#ifdef __KERNEL__\n");
+	printf("EXPORT_SYMBOL(raid6_gflog);\n");
+	printf("#endif\n");
+
 	/* Compute inverse table x^-1 == x^254 */
 	printf("\nconst u8 __attribute__((aligned(256)))\n"
 	       "raid6_gfinv[256] =\n" "{\n");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1580349 — [PATCH v4 4/4] dt-bindings: Add DT bindings document for Broadcom SBA RAID driver

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-14 08:00 +0100
Subject[PATCH v4 4/4] dt-bindings: Add DT bindings document for Broadcom SBA RAID driver
Message-ID<taFfJ-6FF-19@gated-at.bofh.it>
In reply to#1580346
This patch adds the DT bindings document for newly added Broadcom
SBA RAID driver.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 .../devicetree/bindings/dma/brcm,iproc-sba.txt     | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/brcm,iproc-sba.txt

diff --git a/Documentation/devicetree/bindings/dma/brcm,iproc-sba.txt b/Documentation/devicetree/bindings/dma/brcm,iproc-sba.txt
new file mode 100644
index 0000000..092913a
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/brcm,iproc-sba.txt
@@ -0,0 +1,29 @@
+* Broadcom SBA RAID engine
+
+Required properties:
+- compatible: Should be one of the following
+	      "brcm,iproc-sba"
+	      "brcm,iproc-sba-v2"
+  The "brcm,iproc-sba" has support for only 6 PQ coefficients
+  The "brcm,iproc-sba-v2" has support for only 30 PQ coefficients
+- mboxes: List of phandle and mailbox channel specifiers
+
+Example:
+
+raid_mbox: mbox@67400000 {
+	...
+	#mbox-cells = <3>;
+	...
+};
+
+raid0 {
+	compatible = "brcm,iproc-sba-v2";
+	mboxes = <&raid_mbox 0 0x1 0xffff>,
+		 <&raid_mbox 1 0x1 0xffff>,
+		 <&raid_mbox 2 0x1 0xffff>,
+		 <&raid_mbox 3 0x1 0xffff>,
+		 <&raid_mbox 4 0x1 0xffff>,
+		 <&raid_mbox 5 0x1 0xffff>,
+		 <&raid_mbox 6 0x1 0xffff>,
+		 <&raid_mbox 7 0x1 0xffff>;
+};
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1580653 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromDan Williams <dan.j.williams@intel.com>
Date2017-02-14 17:40 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<taOj0-4aW-15@gated-at.bofh.it>
In reply to#1580346
On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
> The Broadcom stream buffer accelerator (SBA) provides offloading
> capabilities for RAID operations. This SBA offload engine is
> accessible via Broadcom SoC specific ring manager.
>
> This patch adds Broadcom SBA RAID driver which provides one
> DMA device with RAID capabilities using one or more Broadcom
> SoC specific ring manager channels. The SBA RAID driver in its
> current shape implements memcpy, xor, and pq operations.
>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> ---
>  drivers/dma/Kconfig        |   13 +
>  drivers/dma/Makefile       |    1 +
>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1708 insertions(+)
>  create mode 100644 drivers/dma/bcm-sba-raid.c
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 263495d..bf8fb84 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -99,6 +99,19 @@ config AXI_DMAC
>           controller is often used in Analog Device's reference designs for FPGA
>           platforms.
>
> +config BCM_SBA_RAID
> +       tristate "Broadcom SBA RAID engine support"
> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
> +       select DMA_ENGINE
> +       select DMA_ENGINE_RAID
> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH

I thought you agreed to drop this. Its usage is broken.

[toc] | [prev] | [next] | [standalone]

#1581081 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-15 07:30 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<tb1gd-4ya-9@gated-at.bofh.it>
In reply to#1580653
On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>> The Broadcom stream buffer accelerator (SBA) provides offloading
>> capabilities for RAID operations. This SBA offload engine is
>> accessible via Broadcom SoC specific ring manager.
>>
>> This patch adds Broadcom SBA RAID driver which provides one
>> DMA device with RAID capabilities using one or more Broadcom
>> SoC specific ring manager channels. The SBA RAID driver in its
>> current shape implements memcpy, xor, and pq operations.
>>
>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> ---
>>  drivers/dma/Kconfig        |   13 +
>>  drivers/dma/Makefile       |    1 +
>>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1708 insertions(+)
>>  create mode 100644 drivers/dma/bcm-sba-raid.c
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 263495d..bf8fb84 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -99,6 +99,19 @@ config AXI_DMAC
>>           controller is often used in Analog Device's reference designs for FPGA
>>           platforms.
>>
>> +config BCM_SBA_RAID
>> +       tristate "Broadcom SBA RAID engine support"
>> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
>> +       select DMA_ENGINE
>> +       select DMA_ENGINE_RAID
>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>
> I thought you agreed to drop this. Its usage is broken.

If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected
then async_dma_find_channel() will only try to find channel
with DMA_ASYNC_TX capability.

The DMA_ASYNC_TX capability is set by
dma_async_device_register() when all Async Tx
capabilities are supported by a DMA devices namely
DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR,
DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL.

We only support DMA_MEMCPY, DMA_XOR, and
DMA_PQ capabilities in BCM-SBA-RAID driver so
DMA_ASYNC_TX capability is never set for the
DMA device registered by BCM-SBA-RAID driver.

Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH
is not selected then Async Tx APIs fail to find DMA
channel provided by BCM-SBA-RAID hence the
option ASYNC_TX_ENABLE_CHANNEL_SWITCH is
required for BCM-SBA-RAID.

The DMA mappings are violated by channel switching
only if we switch form DMA channel A to DMA channel
B and both these DMA channels have different underlying
"struct device". In most of the cases DMA mappings
are not violated because DMA channels having
Async Tx capabilities are provided using same
underlying "struct device".

Regards,
Anup

[toc] | [prev] | [next] | [standalone]

#1581084 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromDan Williams <dan.j.williams@intel.com>
Date2017-02-15 07:50 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<tb1zA-4ES-5@gated-at.bofh.it>
In reply to#1581081
On Tue, Feb 14, 2017 at 10:25 PM, Anup Patel <anup.patel@broadcom.com> wrote:
> On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>> The Broadcom stream buffer accelerator (SBA) provides offloading
>>> capabilities for RAID operations. This SBA offload engine is
>>> accessible via Broadcom SoC specific ring manager.
>>>
>>> This patch adds Broadcom SBA RAID driver which provides one
>>> DMA device with RAID capabilities using one or more Broadcom
>>> SoC specific ring manager channels. The SBA RAID driver in its
>>> current shape implements memcpy, xor, and pq operations.
>>>
>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>> ---
>>>  drivers/dma/Kconfig        |   13 +
>>>  drivers/dma/Makefile       |    1 +
>>>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 1708 insertions(+)
>>>  create mode 100644 drivers/dma/bcm-sba-raid.c
>>>
>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>> index 263495d..bf8fb84 100644
>>> --- a/drivers/dma/Kconfig
>>> +++ b/drivers/dma/Kconfig
>>> @@ -99,6 +99,19 @@ config AXI_DMAC
>>>           controller is often used in Analog Device's reference designs for FPGA
>>>           platforms.
>>>
>>> +config BCM_SBA_RAID
>>> +       tristate "Broadcom SBA RAID engine support"
>>> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
>>> +       select DMA_ENGINE
>>> +       select DMA_ENGINE_RAID
>>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>
>> I thought you agreed to drop this. Its usage is broken.
>
> If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected
> then async_dma_find_channel() will only try to find channel
> with DMA_ASYNC_TX capability.
>
> The DMA_ASYNC_TX capability is set by
> dma_async_device_register() when all Async Tx
> capabilities are supported by a DMA devices namely
> DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR,
> DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL.
>
> We only support DMA_MEMCPY, DMA_XOR, and
> DMA_PQ capabilities in BCM-SBA-RAID driver so
> DMA_ASYNC_TX capability is never set for the
> DMA device registered by BCM-SBA-RAID driver.
>
> Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH
> is not selected then Async Tx APIs fail to find DMA
> channel provided by BCM-SBA-RAID hence the
> option ASYNC_TX_ENABLE_CHANNEL_SWITCH is
> required for BCM-SBA-RAID.
>
> The DMA mappings are violated by channel switching
> only if we switch form DMA channel A to DMA channel
> B and both these DMA channels have different underlying
> "struct device". In most of the cases DMA mappings
> are not violated because DMA channels having
> Async Tx capabilities are provided using same
> underlying "struct device".

No, fix the infrastructure. Do not put local hack in your driver for
this global problem [1].

[1]: https://lwn.net/Articles/443531/

[toc] | [prev] | [next] | [standalone]

#1581087 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-15 08:10 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<tb1SW-50Q-17@gated-at.bofh.it>
In reply to#1581084
On Wed, Feb 15, 2017 at 12:13 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Feb 14, 2017 at 10:25 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>> On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>> The Broadcom stream buffer accelerator (SBA) provides offloading
>>>> capabilities for RAID operations. This SBA offload engine is
>>>> accessible via Broadcom SoC specific ring manager.
>>>>
>>>> This patch adds Broadcom SBA RAID driver which provides one
>>>> DMA device with RAID capabilities using one or more Broadcom
>>>> SoC specific ring manager channels. The SBA RAID driver in its
>>>> current shape implements memcpy, xor, and pq operations.
>>>>
>>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>> ---
>>>>  drivers/dma/Kconfig        |   13 +
>>>>  drivers/dma/Makefile       |    1 +
>>>>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 1708 insertions(+)
>>>>  create mode 100644 drivers/dma/bcm-sba-raid.c
>>>>
>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>> index 263495d..bf8fb84 100644
>>>> --- a/drivers/dma/Kconfig
>>>> +++ b/drivers/dma/Kconfig
>>>> @@ -99,6 +99,19 @@ config AXI_DMAC
>>>>           controller is often used in Analog Device's reference designs for FPGA
>>>>           platforms.
>>>>
>>>> +config BCM_SBA_RAID
>>>> +       tristate "Broadcom SBA RAID engine support"
>>>> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
>>>> +       select DMA_ENGINE
>>>> +       select DMA_ENGINE_RAID
>>>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>>
>>> I thought you agreed to drop this. Its usage is broken.
>>
>> If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected
>> then async_dma_find_channel() will only try to find channel
>> with DMA_ASYNC_TX capability.
>>
>> The DMA_ASYNC_TX capability is set by
>> dma_async_device_register() when all Async Tx
>> capabilities are supported by a DMA devices namely
>> DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR,
>> DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL.
>>
>> We only support DMA_MEMCPY, DMA_XOR, and
>> DMA_PQ capabilities in BCM-SBA-RAID driver so
>> DMA_ASYNC_TX capability is never set for the
>> DMA device registered by BCM-SBA-RAID driver.
>>
>> Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH
>> is not selected then Async Tx APIs fail to find DMA
>> channel provided by BCM-SBA-RAID hence the
>> option ASYNC_TX_ENABLE_CHANNEL_SWITCH is
>> required for BCM-SBA-RAID.
>>
>> The DMA mappings are violated by channel switching
>> only if we switch form DMA channel A to DMA channel
>> B and both these DMA channels have different underlying
>> "struct device". In most of the cases DMA mappings
>> are not violated because DMA channels having
>> Async Tx capabilities are provided using same
>> underlying "struct device".
>
> No, fix the infrastructure. Do not put local hack in your driver for
> this global problem [1].

There is no hack in the driver. We need
ASYNC_TX_ENABLE_CHANNEL_SWITCH
based on current state of dmaengine framework.

The framework should be fixed as separate patchset.

We have other RAID drivers such as xgene-dma and
mv_xor_v2 who also require
ASYNC_TX_ENABLE_CHANNEL_SWITCH due
to same reason.

Fixing the framework and improving framework is
a ongoing process. I don't see why that should
stop this patchset.

Regards,
Anup

[toc] | [prev] | [next] | [standalone]

#1581096 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromDan Williams <dan.j.williams@intel.com>
Date2017-02-15 08:30 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<tb2ch-59b-1@gated-at.bofh.it>
In reply to#1581087
On Tue, Feb 14, 2017 at 11:03 PM, Anup Patel <anup.patel@broadcom.com> wrote:
> On Wed, Feb 15, 2017 at 12:13 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Tue, Feb 14, 2017 at 10:25 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>> On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>>> The Broadcom stream buffer accelerator (SBA) provides offloading
>>>>> capabilities for RAID operations. This SBA offload engine is
>>>>> accessible via Broadcom SoC specific ring manager.
>>>>>
>>>>> This patch adds Broadcom SBA RAID driver which provides one
>>>>> DMA device with RAID capabilities using one or more Broadcom
>>>>> SoC specific ring manager channels. The SBA RAID driver in its
>>>>> current shape implements memcpy, xor, and pq operations.
>>>>>
>>>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>> ---
>>>>>  drivers/dma/Kconfig        |   13 +
>>>>>  drivers/dma/Makefile       |    1 +
>>>>>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>  3 files changed, 1708 insertions(+)
>>>>>  create mode 100644 drivers/dma/bcm-sba-raid.c
>>>>>
>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> index 263495d..bf8fb84 100644
>>>>> --- a/drivers/dma/Kconfig
>>>>> +++ b/drivers/dma/Kconfig
>>>>> @@ -99,6 +99,19 @@ config AXI_DMAC
>>>>>           controller is often used in Analog Device's reference designs for FPGA
>>>>>           platforms.
>>>>>
>>>>> +config BCM_SBA_RAID
>>>>> +       tristate "Broadcom SBA RAID engine support"
>>>>> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
>>>>> +       select DMA_ENGINE
>>>>> +       select DMA_ENGINE_RAID
>>>>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>>>
>>>> I thought you agreed to drop this. Its usage is broken.
>>>
>>> If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected
>>> then async_dma_find_channel() will only try to find channel
>>> with DMA_ASYNC_TX capability.
>>>
>>> The DMA_ASYNC_TX capability is set by
>>> dma_async_device_register() when all Async Tx
>>> capabilities are supported by a DMA devices namely
>>> DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR,
>>> DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL.
>>>
>>> We only support DMA_MEMCPY, DMA_XOR, and
>>> DMA_PQ capabilities in BCM-SBA-RAID driver so
>>> DMA_ASYNC_TX capability is never set for the
>>> DMA device registered by BCM-SBA-RAID driver.
>>>
>>> Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>> is not selected then Async Tx APIs fail to find DMA
>>> channel provided by BCM-SBA-RAID hence the
>>> option ASYNC_TX_ENABLE_CHANNEL_SWITCH is
>>> required for BCM-SBA-RAID.
>>>
>>> The DMA mappings are violated by channel switching
>>> only if we switch form DMA channel A to DMA channel
>>> B and both these DMA channels have different underlying
>>> "struct device". In most of the cases DMA mappings
>>> are not violated because DMA channels having
>>> Async Tx capabilities are provided using same
>>> underlying "struct device".
>>
>> No, fix the infrastructure. Do not put local hack in your driver for
>> this global problem [1].
>
> There is no hack in the driver. We need
> ASYNC_TX_ENABLE_CHANNEL_SWITCH
> based on current state of dmaengine framework.
>
> The framework should be fixed as separate patchset.
>
> We have other RAID drivers such as xgene-dma and
> mv_xor_v2 who also require
> ASYNC_TX_ENABLE_CHANNEL_SWITCH due
> to same reason.
>
> Fixing the framework and improving framework is
> a ongoing process. I don't see why that should
> stop this patchset.
>

Because this driver is turning on a dangerous compile time option and
is not using the functionality. If this silicon IP block appears in
another product in the future paired with another DMA engine then the
assumptions about a safe/single dma-device is violated.

The realization of how async_tx was breaking DMA mapping api
assumptions came after some of these dma-drivers were added to the
kernel. We should stop making the problem worse.

I should have submitted a patch like the below at the time we
discovered this problem, but unfortunately it languished when I
stopped maintaining the iop-adma and ioat drivers.

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 263495d0adbd..6b30eb9ad125 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -35,6 +35,7 @@ comment "DMA Devices"

 #core
 config ASYNC_TX_ENABLE_CHANNEL_SWITCH
+       depends on BROKEN
        bool

 config ARCH_HAS_ASYNC_TX_FIND_CHANNEL

[toc] | [prev] | [next] | [standalone]

#1581123 — Re: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver

FromAnup Patel <anup.patel@broadcom.com>
Date2017-02-15 09:40 +0100
SubjectRe: [PATCH v4 3/4] dmaengine: Add Broadcom SBA RAID driver
Message-ID<tb3i2-5YV-9@gated-at.bofh.it>
In reply to#1581096
On Wed, Feb 15, 2017 at 12:55 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Feb 14, 2017 at 11:03 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>> On Wed, Feb 15, 2017 at 12:13 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>> On Tue, Feb 14, 2017 at 10:25 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>> On Tue, Feb 14, 2017 at 10:04 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>>> On Mon, Feb 13, 2017 at 10:51 PM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>>>> The Broadcom stream buffer accelerator (SBA) provides offloading
>>>>>> capabilities for RAID operations. This SBA offload engine is
>>>>>> accessible via Broadcom SoC specific ring manager.
>>>>>>
>>>>>> This patch adds Broadcom SBA RAID driver which provides one
>>>>>> DMA device with RAID capabilities using one or more Broadcom
>>>>>> SoC specific ring manager channels. The SBA RAID driver in its
>>>>>> current shape implements memcpy, xor, and pq operations.
>>>>>>
>>>>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>>> ---
>>>>>>  drivers/dma/Kconfig        |   13 +
>>>>>>  drivers/dma/Makefile       |    1 +
>>>>>>  drivers/dma/bcm-sba-raid.c | 1694 ++++++++++++++++++++++++++++++++++++++++++++
>>>>>>  3 files changed, 1708 insertions(+)
>>>>>>  create mode 100644 drivers/dma/bcm-sba-raid.c
>>>>>>
>>>>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>>> index 263495d..bf8fb84 100644
>>>>>> --- a/drivers/dma/Kconfig
>>>>>> +++ b/drivers/dma/Kconfig
>>>>>> @@ -99,6 +99,19 @@ config AXI_DMAC
>>>>>>           controller is often used in Analog Device's reference designs for FPGA
>>>>>>           platforms.
>>>>>>
>>>>>> +config BCM_SBA_RAID
>>>>>> +       tristate "Broadcom SBA RAID engine support"
>>>>>> +       depends on (ARM64 && MAILBOX && RAID6_PQ) || COMPILE_TEST
>>>>>> +       select DMA_ENGINE
>>>>>> +       select DMA_ENGINE_RAID
>>>>>> +       select ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>>>>
>>>>> I thought you agreed to drop this. Its usage is broken.
>>>>
>>>> If ASYNC_TX_ENABLE_CHANNEL_SWITCH is not selected
>>>> then async_dma_find_channel() will only try to find channel
>>>> with DMA_ASYNC_TX capability.
>>>>
>>>> The DMA_ASYNC_TX capability is set by
>>>> dma_async_device_register() when all Async Tx
>>>> capabilities are supported by a DMA devices namely
>>>> DMA_INTERRUPT, DMA_MEMCPY, DMA_XOR,
>>>> DMA_XOR_VAL, DMA_PQ, and DMA_PQ_VAL.
>>>>
>>>> We only support DMA_MEMCPY, DMA_XOR, and
>>>> DMA_PQ capabilities in BCM-SBA-RAID driver so
>>>> DMA_ASYNC_TX capability is never set for the
>>>> DMA device registered by BCM-SBA-RAID driver.
>>>>
>>>> Due to above, if ASYNC_TX_ENABLE_CHANNEL_SWITCH
>>>> is not selected then Async Tx APIs fail to find DMA
>>>> channel provided by BCM-SBA-RAID hence the
>>>> option ASYNC_TX_ENABLE_CHANNEL_SWITCH is
>>>> required for BCM-SBA-RAID.
>>>>
>>>> The DMA mappings are violated by channel switching
>>>> only if we switch form DMA channel A to DMA channel
>>>> B and both these DMA channels have different underlying
>>>> "struct device". In most of the cases DMA mappings
>>>> are not violated because DMA channels having
>>>> Async Tx capabilities are provided using same
>>>> underlying "struct device".
>>>
>>> No, fix the infrastructure. Do not put local hack in your driver for
>>> this global problem [1].
>>
>> There is no hack in the driver. We need
>> ASYNC_TX_ENABLE_CHANNEL_SWITCH
>> based on current state of dmaengine framework.
>>
>> The framework should be fixed as separate patchset.
>>
>> We have other RAID drivers such as xgene-dma and
>> mv_xor_v2 who also require
>> ASYNC_TX_ENABLE_CHANNEL_SWITCH due
>> to same reason.
>>
>> Fixing the framework and improving framework is
>> a ongoing process. I don't see why that should
>> stop this patchset.
>>
>
> Because this driver is turning on a dangerous compile time option and
> is not using the functionality. If this silicon IP block appears in
> another product in the future paired with another DMA engine then the
> assumptions about a safe/single dma-device is violated.
>
> The realization of how async_tx was breaking DMA mapping api
> assumptions came after some of these dma-drivers were added to the
> kernel. We should stop making the problem worse.
>
> I should have submitted a patch like the below at the time we
> discovered this problem, but unfortunately it languished when I
> stopped maintaining the iop-adma and ioat drivers.
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 263495d0adbd..6b30eb9ad125 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -35,6 +35,7 @@ comment "DMA Devices"
>
>  #core
>  config ASYNC_TX_ENABLE_CHANNEL_SWITCH
> +       depends on BROKEN
>         bool
>
>  config ARCH_HAS_ASYNC_TX_FIND_CHANNEL

Instead of selecting
ASYNC_TX_ENABLE_CHANNEL_SWITCH,
we can select the following in BCM_SBA_RAID config
option:
1. ASYNC_TX_DISABLE_XOR_VAL
2. ASYNC_TX_DISABLE_PQ_VAL

This will satisfy the needs of
dma_async_device_register() when
ASYNC_TX_ENABLE_CHANNEL_SWITCH is
not selected.

Will this be acceptable ??

Regards,
Anup

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web