From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robin Murphy Subject: Re: [PATCH] net: netsec: reduce DMA mask to 40 bits Date: Fri, 25 May 2018 20:37:30 +0100 Message-ID: <20180525203730.20e8ec72@m750> References: <20180525125037.779-1-ard.biesheuvel@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Ard Biesheuvel , netdev@vger.kernel.org, "David S. Miller" , Masahisa Kojima , Ilias Apalodimas , nd@arm.com To: Jassi Brar Return-path: Received: from mail-he1eur01on0055.outbound.protection.outlook.com ([104.47.0.55]:37248 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S967698AbeEYThc (ORCPT ); Fri, 25 May 2018 15:37:32 -0400 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 26 May 2018 00:33:05 +0530 Jassi Brar wrote: > On 25 May 2018 at 18:20, 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. > > > 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.