Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751554AbdGZPGB (ORCPT ); Wed, 26 Jul 2017 11:06:01 -0400 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:34108 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750867AbdGZPF7 (ORCPT ); Wed, 26 Jul 2017 11:05:59 -0400 Subject: Re: [PATCH 0/4] ACPI: DMA ranges management To: Nate Watterson , Lorenzo Pieralisi , linux-acpi@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Will Deacon , Hanjun Guo , Feng Kan , Jon Masters , Robert Moore , Zhang Rui , "Rafael J. Wysocki" References: <20170720144517.32529-1-lorenzo.pieralisi@arm.com> <7df11172-5a40-685f-5202-9a0bceb6e19b@codeaurora.org> From: Robin Murphy Message-ID: Date: Wed, 26 Jul 2017 16:05:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <7df11172-5a40-685f-5202-9a0bceb6e19b@codeaurora.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4163 Lines: 100 Hi Nate, On 26/07/17 15:46, Nate Watterson wrote: > Hi Lorenzo, > > On 7/20/2017 10:45 AM, Lorenzo Pieralisi wrote: >> As reported in: >> >> http://lkml.kernel.org/r/CAL85gmA_SSCwM80TKdkZqEe+S1beWzDEvdki1kpkmUTDRmSP7g@mail.gmail.com >> >> >> the bus connecting devices to an IOMMU bus can be smaller in size than >> the IOMMU input address bits which results in devices DMA HW bugs in >> particular related to IOVA allocation (ie chopping of higher address >> bits owing to system bus HW capabilities mismatch with the IOMMU). >> >> Fortunately this problem can be solved through an already present but >> never >> used ACPI 6.2 firmware bindings (ie _DMA object) allowing to define >> the DMA >> window for a specific bus in ACPI and therefore all upstream devices >> connected to it. >> >> This small patch series enables _DMA parsing in ACPI core code and >> use it in ACPI IORT code in order to detect DMA ranges for devices and >> update their data structures to make them work with their related DMA >> addressing restrictions. > > I tested the patches and unfortunately it seems like the DMA addressing > restrictions are not really enforced for devices that attempt to set > their own dma_mask based on controller capabilities. For instance, > consider the following from the ahci_platform driver: > > if (hpriv->cap & HOST_CAP_64) { > rc = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); > [...] > } > > Prior to the check, I can see that the device dma_mask respects the > limits enumerated in the _DMA object, however it is then clobbered by > the call to dma_coerce_mask_and_coherent(). Interestingly, if > HOST_CAP_64 was not set and the _DMA object for the device (or its > parent) indicated support for > 32-bit addrs, the host controller > could end up getting programmed with addresses beyond what it actually > supports. That is more a bug with the ahci_platform driver assuming a > default 32-bit dma_mask, but I would not be surprised to find other > drivers that rely on the same assumption. Yup, you've hit upon the more general problem, which applies equally to DT "dma-ranges" too. I'm working on arm64 DMA stuff at the moment, and have the patch to actually enforce the firmware-described limit when drivers update their masks, but that depends on everyone passing the correct information to arch_setup_dma_ops() in the first place (I think DT needs more fixing than ACPI does). > To ensure that dma_set_mask() and friends actually respect _DMA, would > you consider introducing a dma_supported() callback to check the input > dma_mask against the FW defined limits? This would end up aggressively > clipping the dma_mask to 32-bits for devices like the above if the _DMA > limit was less than 64-bits, but that is probably preferable to the > controller accessing unintended addresses. > > Also, how would you feel about adding support for the IORT named_node > memory_address_limit field? We will certainly need that for some platform devices, so if you fancy giving it a go before Lorenzo or I get there, feel free! Robin. > -Nate >> >> Cc: Will Deacon >> Cc: Hanjun Guo >> Cc: Feng Kan >> Cc: Jon Masters >> Cc: Robert Moore >> Cc: Robin Murphy >> Cc: Zhang Rui >> Cc: "Rafael J. Wysocki" >> >> Lorenzo Pieralisi (4): >> ACPI: Allow _DMA method in walk resources >> ACPI: Make acpi_dev_get_resources() method agnostic >> ACPI: Introduce DMA ranges parsing >> ACPI: Make acpi_dma_configure() DMA regions aware >> >> drivers/acpi/acpica/rsxface.c | 7 ++-- >> drivers/acpi/arm64/iort.c | 27 +++++++++++- >> drivers/acpi/resource.c | 83 >> ++++++++++++++++++++++++++++--------- >> drivers/acpi/scan.c | 95 >> +++++++++++++++++++++++++++++++++++++++---- >> include/acpi/acnames.h | 1 + >> include/acpi/acpi_bus.h | 2 + >> include/linux/acpi.h | 8 ++++ >> include/linux/acpi_iort.h | 5 ++- >> 8 files changed, 194 insertions(+), 34 deletions(-) >> >