[PATCH] net: netsec: reduce DMA mask to 40 bits

[PATCH] net: netsec: reduce DMA mask to 40 bits

[Ard Biesheuvel]

The netsec network controller IP can drive 64 address bits for DMA, and
the DMA mask is set accordingly in the driver. However, the SynQuacer
SoC, which is the only silicon incorporating this IP at the moment,
integrates this IP in a manner that leaves address bits [63:40]
unconnected.

Up until now, this has not resulted in any problems, given that the DDR
controller doesn't decode those bits to begin with. However, recent
firmware updates for platforms incorporating this SoC allow the IOMMU
to be enabled, which does decode address bits [47:40], and allocates
top down from the IOVA space, producing DMA addresses that have bits
set that have been left unconnected.

Both the DT and ACPI (IORT) descriptions of the platform take this into
account, and only describe a DMA address space of 40 bits (using either
dma-ranges DT properties, or DMA address limits in IORT named component
nodes). However, even though our IOMMU and bus layers may take such
limitations into account by setting a narrower DMA mask when creating
the platform device, the netsec probe() entrypoint follows the common
practice of setting the DMA mask uncondionally, according to the
capabilities of the IP block itself rather than to its integration into
the chip.

It is currently unclear what the correct fix is here. We could hack around
it by only setting the DMA mask if it deviates from its default value of
DMA_BIT_MASK(32). However, this makes it impossible for the bus layer to
use DMA_BIT_MASK(32) as the bus limit, and so it appears that a more
comprehensive approach is required to take DMA limits imposed by the
SoC as a whole into account.

In the mean time, let's limit the DMA mask to 40 bits. Given that there
is currently only one SoC that incorporates this IP, this is a reasonable
approach that can be backported to -stable and buys us some time to come
up with a proper fix going forward.

Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Please cc to -stable (v4.16+)

 drivers/net/ethernet/socionext/netsec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
index f4c0b02ddad8..59fbf74dcada 100644
--- a/drivers/net/ethernet/socionext/netsec.c
+++ b/drivers/net/ethernet/socionext/netsec.c
@@ -1674,8 +1674,8 @@ static int netsec_probe(struct platform_device *pdev)
 	if (ret)
 		goto unreg_napi;
 
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)))
-		dev_warn(&pdev->dev, "Failed to enable 64-bit DMA\n");
+	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(40)))
+		dev_warn(&pdev->dev, "Failed to set DMA mask\n");
 
 	ret = register_netdev(ndev);
 	if (ret) {
-- 
2.17.0

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Robin Murphy]

On 25/05/18 13:50, Ard Biesheuvel wrote:
> The netsec network controller IP can drive 64 address bits for DMA, and
> the DMA mask is set accordingly in the driver. However, the SynQuacer
> SoC, which is the only silicon incorporating this IP at the moment,
> integrates this IP in a manner that leaves address bits [63:40]
> unconnected.
> 
> Up until now, this has not resulted in any problems, given that the DDR
> controller doesn't decode those bits to begin with. However, recent
> firmware updates for platforms incorporating this SoC allow the IOMMU
> to be enabled, which does decode address bits [47:40], and allocates
> top down from the IOVA space, producing DMA addresses that have bits
> set that have been left unconnected.
> 
> Both the DT and ACPI (IORT) descriptions of the platform take this into
> account, and only describe a DMA address space of 40 bits (using either
> dma-ranges DT properties, or DMA address limits in IORT named component
> nodes). However, even though our IOMMU and bus layers may take such
> limitations into account by setting a narrower DMA mask when creating
> the platform device, the netsec probe() entrypoint follows the common
> practice of setting the DMA mask uncondionally, according to the
> capabilities of the IP block itself rather than to its integration into
> the chip.
> 
> It is currently unclear what the correct fix is here. We could hack around
> it by only setting the DMA mask if it deviates from its default value of
> DMA_BIT_MASK(32). However, this makes it impossible for the bus layer to
> use DMA_BIT_MASK(32) as the bus limit, and so it appears that a more
> comprehensive approach is required to take DMA limits imposed by the
> SoC as a whole into account.
> 
> In the mean time, let's limit the DMA mask to 40 bits. Given that there
> is currently only one SoC that incorporates this IP, this is a reasonable
> approach that can be backported to -stable and buys us some time to come
> up with a proper fix going forward.

It's a little bit of a dodge, but until another SoC comes along with 
different requirements I agree it's the reasonable thing to do.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Jassi Brar <jaswinder.singh@linaro.org>
> Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Please cc to -stable (v4.16+)
> 
>   drivers/net/ethernet/socionext/netsec.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/socionext/netsec.c b/drivers/net/ethernet/socionext/netsec.c
> index f4c0b02ddad8..59fbf74dcada 100644
> --- a/drivers/net/ethernet/socionext/netsec.c
> +++ b/drivers/net/ethernet/socionext/netsec.c
> @@ -1674,8 +1674,8 @@ static int netsec_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto unreg_napi;
>   
> -	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64)))
> -		dev_warn(&pdev->dev, "Failed to enable 64-bit DMA\n");
> +	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(40)))
> +		dev_warn(&pdev->dev, "Failed to set DMA mask\n");
>   
>   	ret = register_netdev(ndev);
>   	if (ret) {
> 

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Jassi Brar]

On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> The netsec network controller IP can drive 64 address bits for DMA, and
> the DMA mask is set accordingly in the driver. However, the SynQuacer
> SoC, which is the only silicon incorporating this IP at the moment,
> integrates this IP in a manner that leaves address bits [63:40]
> unconnected.
>
> Up until now, this has not resulted in any problems, given that the DDR
> controller doesn't decode those bits to begin with. However, recent
> firmware updates for platforms incorporating this SoC allow the IOMMU
> to be enabled, which does decode address bits [47:40], and allocates
> top down from the IOVA space, producing DMA addresses that have bits
> set that have been left unconnected.
>
> Both the DT and ACPI (IORT) descriptions of the platform take this into
> account, and only describe a DMA address space of 40 bits (using either
> dma-ranges DT properties, or DMA address limits in IORT named component
> nodes). However, even though our IOMMU and bus layers may take such
> limitations into account by setting a narrower DMA mask when creating
> the platform device, the netsec probe() entrypoint follows the common
> practice of setting the DMA mask uncondionally, according to the
> capabilities of the IP block itself rather than to its integration into
> the chip.
>
> It is currently unclear what the correct fix is here. We could hack around
> it by only setting the DMA mask if it deviates from its default value of
> DMA_BIT_MASK(32). However, this makes it impossible for the bus layer to
> use DMA_BIT_MASK(32) as the bus limit, and so it appears that a more
> comprehensive approach is required to take DMA limits imposed by the
> SoC as a whole into account.
>
> In the mean time, let's limit the DMA mask to 40 bits. Given that there
> is currently only one SoC that incorporates this IP, this is a reasonable
> approach that can be backported to -stable and buys us some time to come
> up with a proper fix going forward.
>
I am sure you already thought about it, but why not let the platform
specify the bit mask for the driver (via some "bus-width" property),
to override the default 64 bit mask?

Cheers!

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Robin Murphy]

On Sat, 26 May 2018 00:33:05 +0530
Jassi Brar <jaswinder.singh@linaro.org> wrote:

> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > The netsec network controller IP can drive 64 address bits for DMA,
> > and the DMA mask is set accordingly in the driver. However, the
> > SynQuacer SoC, which is the only silicon incorporating this IP at
> > the moment, integrates this IP in a manner that leaves address bits
> > [63:40] unconnected.
> >
> > Up until now, this has not resulted in any problems, given that the
> > DDR controller doesn't decode those bits to begin with. However,
> > recent firmware updates for platforms incorporating this SoC allow
> > the IOMMU to be enabled, which does decode address bits [47:40],
> > and allocates top down from the IOVA space, producing DMA addresses
> > that have bits set that have been left unconnected.
> >
> > Both the DT and ACPI (IORT) descriptions of the platform take this
> > into account, and only describe a DMA address space of 40 bits
> > (using either dma-ranges DT properties, or DMA address limits in
> > IORT named component nodes). However, even though our IOMMU and bus
> > layers may take such limitations into account by setting a narrower
> > DMA mask when creating the platform device, the netsec probe()
> > entrypoint follows the common practice of setting the DMA mask
> > uncondionally, according to the capabilities of the IP block itself
> > rather than to its integration into the chip.
> >
> > It is currently unclear what the correct fix is here. We could hack
> > around it by only setting the DMA mask if it deviates from its
> > default value of DMA_BIT_MASK(32). However, this makes it
> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
> > limit, and so it appears that a more comprehensive approach is
> > required to take DMA limits imposed by the SoC as a whole into
> > account.
> >
> > In the mean time, let's limit the DMA mask to 40 bits. Given that
> > there is currently only one SoC that incorporates this IP, this is
> > a reasonable approach that can be backported to -stable and buys us
> > some time to come up with a proper fix going forward.
> >  
> I am sure you already thought about it, but why not let the platform
> specify the bit mask for the driver (via some "bus-width" property),
> to override the default 64 bit mask?

Because lack of a property to describe the integration is not the
problem. There are already at least two ways: the general DT/IORT
properties for describing DMA addressing - which it would be a bit
ungainly for a driver to parse for this reason, but not impossible -
and inferring it from a SoC-specific compatible - which is more
appropriate, and what we happen to be able to do here.

Robin.

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Jassi Brar]

On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@arm.com> wrote:
> On Sat, 26 May 2018 00:33:05 +0530
> Jassi Brar <jaswinder.singh@linaro.org> wrote:
>
>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>> > The netsec network controller IP can drive 64 address bits for DMA,
>> > and the DMA mask is set accordingly in the driver. However, the
>> > SynQuacer SoC, which is the only silicon incorporating this IP at
>> > the moment, integrates this IP in a manner that leaves address bits
>> > [63:40] unconnected.
>> >
>> > Up until now, this has not resulted in any problems, given that the
>> > DDR controller doesn't decode those bits to begin with. However,
>> > recent firmware updates for platforms incorporating this SoC allow
>> > the IOMMU to be enabled, which does decode address bits [47:40],
>> > and allocates top down from the IOVA space, producing DMA addresses
>> > that have bits set that have been left unconnected.
>> >
>> > Both the DT and ACPI (IORT) descriptions of the platform take this
>> > into account, and only describe a DMA address space of 40 bits
>> > (using either dma-ranges DT properties, or DMA address limits in
>> > IORT named component nodes). However, even though our IOMMU and bus
>> > layers may take such limitations into account by setting a narrower
>> > DMA mask when creating the platform device, the netsec probe()
>> > entrypoint follows the common practice of setting the DMA mask
>> > uncondionally, according to the capabilities of the IP block itself
>> > rather than to its integration into the chip.
>> >
>> > It is currently unclear what the correct fix is here. We could hack
>> > around it by only setting the DMA mask if it deviates from its
>> > default value of DMA_BIT_MASK(32). However, this makes it
>> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
>> > limit, and so it appears that a more comprehensive approach is
>> > required to take DMA limits imposed by the SoC as a whole into
>> > account.
>> >
>> > In the mean time, let's limit the DMA mask to 40 bits. Given that
>> > there is currently only one SoC that incorporates this IP, this is
>> > a reasonable approach that can be backported to -stable and buys us
>> > some time to come up with a proper fix going forward.
>> >
>> I am sure you already thought about it, but why not let the platform
>> specify the bit mask for the driver (via some "bus-width" property),
>> to override the default 64 bit mask?
>
> Because lack of a property to describe the integration is not the
> problem. There are already at least two ways: the general DT/IORT
> properties for describing DMA addressing - which it would be a bit
> ungainly for a driver to parse for this reason, but not impossible -
....


> and inferring it from a SoC-specific compatible - which is more
> appropriate, and what we happen to be able to do here.
>
Sorry, I am not sure I follow. This patch changes from 64-bits default
to 40-bits capability without checking for the parent SoC. If the next
generation implements the full 64-bit or just 32-bit bus, we'll be
back in the pit again. No?

Thanks.

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Jassi Brar]

On 26 May 2018 at 08:56, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@arm.com> wrote:
>> On Sat, 26 May 2018 00:33:05 +0530
>> Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>
>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> wrote:
>>> > The netsec network controller IP can drive 64 address bits for DMA,
>>> > and the DMA mask is set accordingly in the driver. However, the
>>> > SynQuacer SoC, which is the only silicon incorporating this IP at
>>> > the moment, integrates this IP in a manner that leaves address bits
>>> > [63:40] unconnected.
>>> >
>>> > Up until now, this has not resulted in any problems, given that the
>>> > DDR controller doesn't decode those bits to begin with. However,
>>> > recent firmware updates for platforms incorporating this SoC allow
>>> > the IOMMU to be enabled, which does decode address bits [47:40],
>>> > and allocates top down from the IOVA space, producing DMA addresses
>>> > that have bits set that have been left unconnected.
>>> >
>>> > Both the DT and ACPI (IORT) descriptions of the platform take this
>>> > into account, and only describe a DMA address space of 40 bits
>>> > (using either dma-ranges DT properties, or DMA address limits in
>>> > IORT named component nodes). However, even though our IOMMU and bus
>>> > layers may take such limitations into account by setting a narrower
>>> > DMA mask when creating the platform device, the netsec probe()
>>> > entrypoint follows the common practice of setting the DMA mask
>>> > uncondionally, according to the capabilities of the IP block itself
>>> > rather than to its integration into the chip.
>>> >
>>> > It is currently unclear what the correct fix is here. We could hack
>>> > around it by only setting the DMA mask if it deviates from its
>>> > default value of DMA_BIT_MASK(32). However, this makes it
>>> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
>>> > limit, and so it appears that a more comprehensive approach is
>>> > required to take DMA limits imposed by the SoC as a whole into
>>> > account.
>>> >
>>> > In the mean time, let's limit the DMA mask to 40 bits. Given that
>>> > there is currently only one SoC that incorporates this IP, this is
>>> > a reasonable approach that can be backported to -stable and buys us
>>> > some time to come up with a proper fix going forward.
>>> >
>>> I am sure you already thought about it, but why not let the platform
>>> specify the bit mask for the driver (via some "bus-width" property),
>>> to override the default 64 bit mask?
>>
>> Because lack of a property to describe the integration is not the
>> problem. There are already at least two ways: the general DT/IORT
>> properties for describing DMA addressing - which it would be a bit
>> ungainly for a driver to parse for this reason, but not impossible -
> ....
>
>
>> and inferring it from a SoC-specific compatible - which is more
>> appropriate, and what we happen to be able to do here.
>>
> Sorry, I am not sure I follow. This patch changes from 64-bits default
> to 40-bits capability without checking for the parent SoC. If the next
> generation implements the full 64-bit or just 32-bit bus, we'll be
> back in the pit again. No?
>
Probably you meant we'll change the ethernet compatible string for
differently capable SoC. OK, but here it is more of integration issue
than controller version.

Which makes me realise the extant compatible property for netsec is
not so correct (it embeds the platform name). So I am ok either way.

Thanks.

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Ard Biesheuvel]

On 26 May 2018 at 05:44, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 26 May 2018 at 08:56, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>> On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On Sat, 26 May 2018 00:33:05 +0530
>>> Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>
>>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> wrote:
>>>> > The netsec network controller IP can drive 64 address bits for DMA,
>>>> > and the DMA mask is set accordingly in the driver. However, the
>>>> > SynQuacer SoC, which is the only silicon incorporating this IP at
>>>> > the moment, integrates this IP in a manner that leaves address bits
>>>> > [63:40] unconnected.
>>>> >
>>>> > Up until now, this has not resulted in any problems, given that the
>>>> > DDR controller doesn't decode those bits to begin with. However,
>>>> > recent firmware updates for platforms incorporating this SoC allow
>>>> > the IOMMU to be enabled, which does decode address bits [47:40],
>>>> > and allocates top down from the IOVA space, producing DMA addresses
>>>> > that have bits set that have been left unconnected.
>>>> >
>>>> > Both the DT and ACPI (IORT) descriptions of the platform take this
>>>> > into account, and only describe a DMA address space of 40 bits
>>>> > (using either dma-ranges DT properties, or DMA address limits in
>>>> > IORT named component nodes). However, even though our IOMMU and bus
>>>> > layers may take such limitations into account by setting a narrower
>>>> > DMA mask when creating the platform device, the netsec probe()
>>>> > entrypoint follows the common practice of setting the DMA mask
>>>> > uncondionally, according to the capabilities of the IP block itself
>>>> > rather than to its integration into the chip.
>>>> >
>>>> > It is currently unclear what the correct fix is here. We could hack
>>>> > around it by only setting the DMA mask if it deviates from its
>>>> > default value of DMA_BIT_MASK(32). However, this makes it
>>>> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
>>>> > limit, and so it appears that a more comprehensive approach is
>>>> > required to take DMA limits imposed by the SoC as a whole into
>>>> > account.
>>>> >
>>>> > In the mean time, let's limit the DMA mask to 40 bits. Given that
>>>> > there is currently only one SoC that incorporates this IP, this is
>>>> > a reasonable approach that can be backported to -stable and buys us
>>>> > some time to come up with a proper fix going forward.
>>>> >
>>>> I am sure you already thought about it, but why not let the platform
>>>> specify the bit mask for the driver (via some "bus-width" property),
>>>> to override the default 64 bit mask?
>>>
>>> Because lack of a property to describe the integration is not the
>>> problem. There are already at least two ways: the general DT/IORT
>>> properties for describing DMA addressing - which it would be a bit
>>> ungainly for a driver to parse for this reason, but not impossible -
>> ....
>>
>>
>>> and inferring it from a SoC-specific compatible - which is more
>>> appropriate, and what we happen to be able to do here.
>>>
>> Sorry, I am not sure I follow. This patch changes from 64-bits default
>> to 40-bits capability without checking for the parent SoC. If the next
>> generation implements the full 64-bit or just 32-bit bus, we'll be
>> back in the pit again. No?
>>
> Probably you meant we'll change the ethernet compatible string for
> differently capable SoC. OK, but here it is more of integration issue
> than controller version.
>
> Which makes me realise the extant compatible property for netsec is
> not so correct (it embeds the platform name). So I am ok either way.
>

The platform in question has a dma-ranges DT property at the root
level that only describes 40 bits' worth of DMA. Also, the ACPI
description in the IORT table of the IOMMU integration of the netsec
controller limits DMA to 40 bits. In the latter case, we actually
enter netsec_probe() with the correct value already assigned to the
DMA mask fields. (In the former case, the DMA limit is ignored
entirely)

In other words, we can already describe these SoC limitations and
distinguish them from device limitations. The problem is that drivers
ignore the existing values of DMA mask.

Robin has volunteered to look into fixing this, but this cannot be
done in a way that is suitable for -stable. In the mean time, we have
a single platform using this network IP in the field that cannot
upgrade its firmware to a version that describes the IOMMU, because
the existing DMA layer code will start driving address bits that are
correctly described as unconnected by the DT/ACPI tables.

So as a a workaround, until Robin fixes things properly, let's reduce
the DMA mask to 40 bits.

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[Jassi Brar]

On 26 May 2018 at 11:46, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 26 May 2018 at 05:44, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>> On 26 May 2018 at 08:56, Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>> On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@arm.com> wrote:
>>>> On Sat, 26 May 2018 00:33:05 +0530
>>>> Jassi Brar <jaswinder.singh@linaro.org> wrote:
>>>>
>>>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>> wrote:
>>>>> > The netsec network controller IP can drive 64 address bits for DMA,
>>>>> > and the DMA mask is set accordingly in the driver. However, the
>>>>> > SynQuacer SoC, which is the only silicon incorporating this IP at
>>>>> > the moment, integrates this IP in a manner that leaves address bits
>>>>> > [63:40] unconnected.
>>>>> >
>>>>> > Up until now, this has not resulted in any problems, given that the
>>>>> > DDR controller doesn't decode those bits to begin with. However,
>>>>> > recent firmware updates for platforms incorporating this SoC allow
>>>>> > the IOMMU to be enabled, which does decode address bits [47:40],
>>>>> > and allocates top down from the IOVA space, producing DMA addresses
>>>>> > that have bits set that have been left unconnected.
>>>>> >
>>>>> > Both the DT and ACPI (IORT) descriptions of the platform take this
>>>>> > into account, and only describe a DMA address space of 40 bits
>>>>> > (using either dma-ranges DT properties, or DMA address limits in
>>>>> > IORT named component nodes). However, even though our IOMMU and bus
>>>>> > layers may take such limitations into account by setting a narrower
>>>>> > DMA mask when creating the platform device, the netsec probe()
>>>>> > entrypoint follows the common practice of setting the DMA mask
>>>>> > uncondionally, according to the capabilities of the IP block itself
>>>>> > rather than to its integration into the chip.
>>>>> >
>>>>> > It is currently unclear what the correct fix is here. We could hack
>>>>> > around it by only setting the DMA mask if it deviates from its
>>>>> > default value of DMA_BIT_MASK(32). However, this makes it
>>>>> > impossible for the bus layer to use DMA_BIT_MASK(32) as the bus
>>>>> > limit, and so it appears that a more comprehensive approach is
>>>>> > required to take DMA limits imposed by the SoC as a whole into
>>>>> > account.
>>>>> >
>>>>> > In the mean time, let's limit the DMA mask to 40 bits. Given that
>>>>> > there is currently only one SoC that incorporates this IP, this is
>>>>> > a reasonable approach that can be backported to -stable and buys us
>>>>> > some time to come up with a proper fix going forward.
>>>>> >
>>>>> I am sure you already thought about it, but why not let the platform
>>>>> specify the bit mask for the driver (via some "bus-width" property),
>>>>> to override the default 64 bit mask?
>>>>
>>>> Because lack of a property to describe the integration is not the
>>>> problem. There are already at least two ways: the general DT/IORT
>>>> properties for describing DMA addressing - which it would be a bit
>>>> ungainly for a driver to parse for this reason, but not impossible -
>>> ....
>>>
>>>
>>>> and inferring it from a SoC-specific compatible - which is more
>>>> appropriate, and what we happen to be able to do here.
>>>>
>>> Sorry, I am not sure I follow. This patch changes from 64-bits default
>>> to 40-bits capability without checking for the parent SoC. If the next
>>> generation implements the full 64-bit or just 32-bit bus, we'll be
>>> back in the pit again. No?
>>>
>> Probably you meant we'll change the ethernet compatible string for
>> differently capable SoC. OK, but here it is more of integration issue
>> than controller version.
>>
>> Which makes me realise the extant compatible property for netsec is
>> not so correct (it embeds the platform name). So I am ok either way.
>>
>
> The platform in question has a dma-ranges DT property at the root
> level that only describes 40 bits' worth of DMA. Also, the ACPI
> description in the IORT table of the IOMMU integration of the netsec
> controller limits DMA to 40 bits. In the latter case, we actually
> enter netsec_probe() with the correct value already assigned to the
> DMA mask fields. (In the former case, the DMA limit is ignored
> entirely)
>
> In other words, we can already describe these SoC limitations and
> distinguish them from device limitations. The problem is that drivers
> ignore the existing values of DMA mask.
>
> Robin has volunteered to look into fixing this, but this cannot be
> done in a way that is suitable for -stable. In the mean time, we have
> a single platform using this network IP in the field that cannot
> upgrade its firmware to a version that describes the IOMMU, because
> the existing DMA layer code will start driving address bits that are
> correctly described as unconnected by the DT/ACPI tables.
>
> So as a a workaround, until Robin fixes things properly, let's reduce
> the DMA mask to 40 bits.
>
Yeah no point in introducing another dt property if this hack is
temporary until the core is fixed.
FWIW ... Acked-by: Jassi Brar <jaswinder.singh@linaro.org>

Thanks.

Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

[David Miller]

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 25 May 2018 14:50:37 +0200

> The netsec network controller IP can drive 64 address bits for DMA, and
> the DMA mask is set accordingly in the driver. However, the SynQuacer
> SoC, which is the only silicon incorporating this IP at the moment,
> integrates this IP in a manner that leaves address bits [63:40]
> unconnected.
> 
> Up until now, this has not resulted in any problems, given that the DDR
> controller doesn't decode those bits to begin with. However, recent
> firmware updates for platforms incorporating this SoC allow the IOMMU
> to be enabled, which does decode address bits [47:40], and allocates
> top down from the IOVA space, producing DMA addresses that have bits
> set that have been left unconnected.
> 
> Both the DT and ACPI (IORT) descriptions of the platform take this into
> account, and only describe a DMA address space of 40 bits (using either
> dma-ranges DT properties, or DMA address limits in IORT named component
> nodes). However, even though our IOMMU and bus layers may take such
> limitations into account by setting a narrower DMA mask when creating
> the platform device, the netsec probe() entrypoint follows the common
> practice of setting the DMA mask uncondionally, according to the
> capabilities of the IP block itself rather than to its integration into
> the chip.
> 
> It is currently unclear what the correct fix is here. We could hack around
> it by only setting the DMA mask if it deviates from its default value of
> DMA_BIT_MASK(32). However, this makes it impossible for the bus layer to
> use DMA_BIT_MASK(32) as the bus limit, and so it appears that a more
> comprehensive approach is required to take DMA limits imposed by the
> SoC as a whole into account.
> 
> In the mean time, let's limit the DMA mask to 40 bits. Given that there
> is currently only one SoC that incorporates this IP, this is a reasonable
> approach that can be backported to -stable and buys us some time to come
> up with a proper fix going forward.
> 
> Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Applied and queued up for -stable.