Date:   Sat, 26 May 2018 09:14:19 +0530
From:   Jassi Brar <jaswinder.singh@...aro.org>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Ard Biesheuvel <ard.biesheuvel@...aro.org>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Masahisa Kojima <masahisa.kojima@...aro.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>, nd@....com
Subject: Re: [PATCH] net: netsec: reduce DMA mask to 40 bits

On 26 May 2018 at 08:56, Jassi Brar <jaswinder.singh@...aro.org> wrote:
> On 26 May 2018 at 01:07, Robin Murphy <robin.murphy@....com> wrote:
>> On Sat, 26 May 2018 00:33:05 +0530
>> Jassi Brar <jaswinder.singh@...aro.org> wrote:
>>
>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheuvel@...aro.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.

Powered by blists - more mailing lists