On Wed, Jul 26, 2017 at 04:05:55PM +0100, Robin Murphy wrote: > 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@xxxxxxxxxxxxxx > >> > >> > >> 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! I can do it for v2 but I would like to understand why using _DMA is not good enough for named components - having two bindings describing the same thing is not ideal and I'd rather avoid it - if there is a reason I am happy to add the necessary code. Thanks, Lorenzo > Robin. > > > -Nate > >> > >> Cc: Will Deacon <will.deacon@xxxxxxx> > >> Cc: Hanjun Guo <hanjun.guo@xxxxxxxxxx> > >> Cc: Feng Kan <fkan@xxxxxxx> > >> Cc: Jon Masters <jcm@xxxxxxxxxx> > >> Cc: Robert Moore <robert.moore@xxxxxxxxx> > >> Cc: Robin Murphy <robin.murphy@xxxxxxx> > >> Cc: Zhang Rui <rui.zhang@xxxxxxxxx> > >> Cc: "Rafael J. Wysocki" <rjw@xxxxxxxxxxxxx> > >> > >> 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(-) > >> > > > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
