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 14:08:16 +0100 Message-ID: References: <20180525125037.779-1-ard.biesheuvel@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, Jassi Brar , Masahisa Kojima , Ilias Apalodimas To: Ard Biesheuvel , netdev@vger.kernel.org Return-path: Received: from foss.arm.com ([217.140.101.70]:33680 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752413AbeEYNIT (ORCPT ); Fri, 25 May 2018 09:08:19 -0400 In-Reply-To: <20180525125037.779-1-ard.biesheuvel@linaro.org> Content-Language: en-GB Sender: netdev-owner@vger.kernel.org List-ID: 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 > Fixes: 533dd11a12f6 ("net: socionext: Add Synquacer NetSec driver") > Cc: Robin Murphy > Cc: Jassi Brar > Cc: Masahisa Kojima > Cc: Ilias Apalodimas > Signed-off-by: Ard Biesheuvel > --- > 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) { >