Message-ID: <CAJe_ZhecUdx5oE8yXAoNRvpUr3WWFbN0jBy8zMsXyRfcGMyfjQ@mail.gmail.com>
Date:   Sat, 26 May 2018 08:56: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 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?

Thanks.

Powered by blists - more mailing lists