lkml.org 
[lkml]   [2017]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 0/4] ACPI: DMA ranges management
From
Date
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 <will.deacon@arm.com>
>> Cc: Hanjun Guo <hanjun.guo@linaro.org>
>> Cc: Feng Kan <fkan@apm.com>
>> Cc: Jon Masters <jcm@redhat.com>
>> Cc: Robert Moore <robert.moore@intel.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Zhang Rui <rui.zhang@intel.com>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>>
>> 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(-)
>>
>

\
 
 \ /
  Last update: 2017-07-26 17:06    [W:0.217 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site