On 26 May 2018 at 11:46, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > On 26 May 2018 at 05:44, Jassi Brar <jaswinder.si...@linaro.org> wrote: >> On 26 May 2018 at 08:56, Jassi Brar <jaswinder.si...@linaro.org> wrote: >>> On 26 May 2018 at 01:07, Robin Murphy <robin.mur...@arm.com> wrote: >>>> On Sat, 26 May 2018 00:33:05 +0530 >>>> Jassi Brar <jaswinder.si...@linaro.org> wrote: >>>> >>>>> On 25 May 2018 at 18:20, Ard Biesheuvel <ard.biesheu...@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.si...@linaro.org>
Thanks.
