diff mbox

[RFC,09/11] drivers: acpi: implement acpi_dma_configure

Message ID 1460654743-7896-10-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Lorenzo Pieralisi April 14, 2016, 5:25 p.m. UTC
On DT based systems, the of_dma_configure() API implements DMA configuration
for a given device. On ACPI systems an API equivalent to of_dma_configure()
is missing which implies that it is currently not possible to set-up DMA
operations for devices through the ACPI generic kernel layer.

This patch fills the gap by introducing acpi_dma_configure/deconfigure()
calls, that carry out IOMMU configuration through IORT (on systems where
it is present) and call arch_setup_dma_ops(...) with the retrieved
parameters.

The DMA range size passed to arch_setup_dma_ops() is sized according
to the device coherent_dma_mask (starting at address 0x0), mirroring the
DT probing path behaviour when a dma-ranges property is not provided
for the device being probed; this changes the current arch_setup_dma_ops()
call parameters in the ACPI probing case, but since arch_setup_dma_ops()
is a NOP on all architectures but ARM/ARM64 this patch does not change
the current kernel behaviour on them.

This patch updates ACPI and PCI core code to use the newly introduced
acpi_dma_configure function, providing the same functionality
as of_dma_configure on ARM systems and leaving behaviour unchanged
for all other arches.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Tomasz Nowicki <tn@semihalf.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
---
 drivers/acpi/glue.c     |  4 +--
 drivers/acpi/iort.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c     | 29 +++++++++++++++++
 drivers/pci/probe.c     |  3 +-
 include/acpi/acpi_bus.h |  2 ++
 include/linux/acpi.h    |  5 +++
 include/linux/iort.h    |  9 ++++++
 7 files changed, 133 insertions(+), 4 deletions(-)

Comments

Bjorn Helgaas April 15, 2016, 4:14 p.m. UTC | #1
On Thu, Apr 14, 2016 at 06:25:41PM +0100, Lorenzo Pieralisi wrote:
> On DT based systems, the of_dma_configure() API implements DMA configuration
> for a given device. On ACPI systems an API equivalent to of_dma_configure()
> is missing which implies that it is currently not possible to set-up DMA
> operations for devices through the ACPI generic kernel layer.
> 
> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> calls, that carry out IOMMU configuration through IORT (on systems where
> it is present) and call arch_setup_dma_ops(...) with the retrieved
> parameters.
> 
> The DMA range size passed to arch_setup_dma_ops() is sized according
> to the device coherent_dma_mask (starting at address 0x0), mirroring the
> DT probing path behaviour when a dma-ranges property is not provided
> for the device being probed; this changes the current arch_setup_dma_ops()
> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> is a NOP on all architectures but ARM/ARM64 this patch does not change
> the current kernel behaviour on them.
> 
> This patch updates ACPI and PCI core code to use the newly introduced
> acpi_dma_configure function, providing the same functionality
> as of_dma_configure on ARM systems and leaving behaviour unchanged
> for all other arches.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>

There's only a tiny PCI change in this series, so I assume somebody
else will merge all this.  Here's my ack for the PCI part:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# for drivers/pci/probe.c change

One question on use of pci_for_each_dma_alias() below.

> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> +{
> +	u32 *rid = data;
> +
> +	*rid = alias;
> +	return 0;
> +}
> +
> +/**
> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> + *
> + * @dev: device that requires IOMMU set-up
> + *
> + * Returns: iommu_ops pointer on configuration success
> + *          NULL on configuration failure
> + */
> +struct iommu_ops *iort_iommu_configure(struct device *dev)
> +{
> +	struct acpi_iort_node *node, *parent;
> +	struct iommu_ops *ops = NULL;
> +	struct iommu_fwspec fwspec;
> +	struct iort_iommu_node *iommu_node;
> +	u32 rid = 0, devid = 0;
> +
> +	if (dev_is_pci(dev)) {
> +		struct pci_bus *bus = to_pci_dev(dev)->bus;
> +
> +		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> +				       &rid);

You end up with only the last DMA alias in "rid".  Is it really true
that you only need to call iort_dev_map_rid() for one of the aliases?

> +		node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> +				      iort_find_dev_callback, &bus->dev);
> +	} else
> +		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> +				      iort_find_dev_callback, dev);
> +
> +	if (!node)
> +		return NULL;
> +
> +	iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> +
> +	parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
> +
> +	if (!parent)
> +		return NULL;
> +
> +	iommu_node = iort_iommu_get_node(parent);
> +	ops = iommu_node->ops;
> +
> +	fwspec.fwnode = iommu_node->fwnode;
> +	fwspec.param_count = 1;
> +	fwspec.param[0] = devid;
> +
> +	if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> +		return NULL;
> +
> +	return ops;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robin Murphy April 15, 2016, 4:31 p.m. UTC | #2
Hi Bjorn,

On 15/04/16 17:14, Bjorn Helgaas wrote:
> On Thu, Apr 14, 2016 at 06:25:41PM +0100, Lorenzo Pieralisi wrote:
>> On DT based systems, the of_dma_configure() API implements DMA configuration
>> for a given device. On ACPI systems an API equivalent to of_dma_configure()
>> is missing which implies that it is currently not possible to set-up DMA
>> operations for devices through the ACPI generic kernel layer.
>>
>> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
>> calls, that carry out IOMMU configuration through IORT (on systems where
>> it is present) and call arch_setup_dma_ops(...) with the retrieved
>> parameters.
>>
>> The DMA range size passed to arch_setup_dma_ops() is sized according
>> to the device coherent_dma_mask (starting at address 0x0), mirroring the
>> DT probing path behaviour when a dma-ranges property is not provided
>> for the device being probed; this changes the current arch_setup_dma_ops()
>> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
>> is a NOP on all architectures but ARM/ARM64 this patch does not change
>> the current kernel behaviour on them.
>>
>> This patch updates ACPI and PCI core code to use the newly introduced
>> acpi_dma_configure function, providing the same functionality
>> as of_dma_configure on ARM systems and leaving behaviour unchanged
>> for all other arches.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Tomasz Nowicki <tn@semihalf.com>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>
> There's only a tiny PCI change in this series, so I assume somebody
> else will merge all this.  Here's my ack for the PCI part:
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>	# for drivers/pci/probe.c change
>
> One question on use of pci_for_each_dma_alias() below.
>
>> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> +	u32 *rid = data;
>> +
>> +	*rid = alias;
>> +	return 0;
>> +}
>> +
>> +/**
>> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
>> + *
>> + * @dev: device that requires IOMMU set-up
>> + *
>> + * Returns: iommu_ops pointer on configuration success
>> + *          NULL on configuration failure
>> + */
>> +struct iommu_ops *iort_iommu_configure(struct device *dev)
>> +{
>> +	struct acpi_iort_node *node, *parent;
>> +	struct iommu_ops *ops = NULL;
>> +	struct iommu_fwspec fwspec;
>> +	struct iort_iommu_node *iommu_node;
>> +	u32 rid = 0, devid = 0;
>> +
>> +	if (dev_is_pci(dev)) {
>> +		struct pci_bus *bus = to_pci_dev(dev)->bus;
>> +
>> +		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> +				       &rid);
>
> You end up with only the last DMA alias in "rid".  Is it really true
> that you only need to call iort_dev_map_rid() for one of the aliases?

Indeed - all we care about is what things look like by the time they 
come out of the root complex on their way to the the IOMMU, so whatever 
intermediate aliasing _within_ the PCI bus might happen along the way 
doesn't actually matter.

Robin.

>> +		node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> +				      iort_find_dev_callback, &bus->dev);
>> +	} else
>> +		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> +				      iort_find_dev_callback, dev);
>> +
>> +	if (!node)
>> +		return NULL;
>> +
>> +	iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
>> +
>> +	parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
>> +
>> +	if (!parent)
>> +		return NULL;
>> +
>> +	iommu_node = iort_iommu_get_node(parent);
>> +	ops = iommu_node->ops;
>> +
>> +	fwspec.fwnode = iommu_node->fwnode;
>> +	fwspec.param_count = 1;
>> +	fwspec.param[0] = devid;
>> +
>> +	if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
>> +		return NULL;
>> +
>> +	return ops;
>> +}
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Timur Tabi April 15, 2016, 6:29 p.m. UTC | #3
On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> +{
> +       struct iommu_ops *iommu;
> +
> +       iommu = iort_iommu_configure(dev);
> +
> +       /*
> +        * Assume dma valid range starts at 0 and covers the whole
> +        * coherent_dma_mask.
> +        */
> +       arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
> +                          attr == DEV_DMA_COHERENT);
> +}

I have a network driver that is impacted by this code, so thank you
for posting this. (See
https://www.mail-archive.com/netdev@vger.kernel.org/msg106249.html).

One one SOC, the driver needs to set the mask to 32 bits.  On another
SOC, it needs to set it to 64 bits.  On device tree, the driver will
use dma-ranges.

In your patches, where is coherent_dma_mask initialized?  I found this
code in add_smmu_platform_device(), but I think this is setting the
mask for the IOMMU driver, not the individual devices.  Either way, I
don't understand where the correct value is going to be overridden.

+       /*
+        * Set default dma mask value for the table walker,
+        * to be overridden on probing with correct value.
+        */
+       *pdev->dev.dma_mask = DMA_BIT_MASK(32);
+       pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
Lorenzo Pieralisi April 18, 2016, 10:30 a.m. UTC | #4
On Fri, Apr 15, 2016 at 01:29:14PM -0500, Timur Tabi wrote:
> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
> > +{
> > +       struct iommu_ops *iommu;
> > +
> > +       iommu = iort_iommu_configure(dev);
> > +
> > +       /*
> > +        * Assume dma valid range starts at 0 and covers the whole
> > +        * coherent_dma_mask.
> > +        */
> > +       arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
> > +                          attr == DEV_DMA_COHERENT);
> > +}
> 
> I have a network driver that is impacted by this code, so thank you
> for posting this. (See
> https://www.mail-archive.com/netdev@vger.kernel.org/msg106249.html).
> 
> One one SOC, the driver needs to set the mask to 32 bits.  On another
> SOC, it needs to set it to 64 bits.  On device tree, the driver will
> use dma-ranges.

First off I think we agree this patch does not change current behaviour
as far as the devices default dma_mask are concerned. They are
initialized in PCI/ACPI core code:

- pci_setup_device()
- acpi_create_platform_device()

As for ACPI DT-dma-ranges equivalent I have to check if I can use
the _DMA method for that so that we can put in place the same
mechanism as DT to override the default masks, other than that it is
up to the drivers to set-up the dma mask accordingly, that's not
something this patchset is changing anyway.

> In your patches, where is coherent_dma_mask initialized?  I found this
> code in add_smmu_platform_device(), but I think this is setting the
> mask for the IOMMU driver, not the individual devices.  Either way, I
> don't understand where the correct value is going to be overridden.

For the ARM SMMU table walker:

arm_smmu_device_cfg_probe() - dma_set_mask_and_coherent()

For other devices see above.

Thanks,
Lorenzo

> 
> +       /*
> +        * Set default dma mask value for the table walker,
> +        * to be overridden on probing with correct value.
> +        */
> +       *pdev->dev.dma_mask = DMA_BIT_MASK(32);
> +       pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
> 
> -- 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robin Murphy April 18, 2016, 10:43 a.m. UTC | #5
On 15/04/16 19:29, Timur Tabi wrote:
> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
>> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
>> +{
>> +       struct iommu_ops *iommu;
>> +
>> +       iommu = iort_iommu_configure(dev);
>> +
>> +       /*
>> +        * Assume dma valid range starts at 0 and covers the whole
>> +        * coherent_dma_mask.
>> +        */
>> +       arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
>> +                          attr == DEV_DMA_COHERENT);
>> +}
>
> I have a network driver that is impacted by this code, so thank you
> for posting this. (See
> https://www.mail-archive.com/netdev@vger.kernel.org/msg106249.html).
>
> One one SOC, the driver needs to set the mask to 32 bits.  On another
> SOC, it needs to set it to 64 bits.  On device tree, the driver will
> use dma-ranges.

That's the wrong way to look at it - the driver isn't _using_ 
dma-ranges, you're merely relying on the OF code setting the _default_ 
DMA mask differently based on the property. If your driver is in the 
minority of those which actually care about DMA masks, then it should be 
calling dma_set_mask_and_coherent() appropriately and not relying on the 
default.

> In your patches, where is coherent_dma_mask initialized?  I found this
> code in add_smmu_platform_device(), but I think this is setting the
> mask for the IOMMU driver, not the individual devices.

Yes, that's for the SMMU itself as a device (i.e. the page table walker) 
- as a handy example of "drivers which actually care about DMA masks", 
it specifically needs to avoid a too-small DMA mask because 
bounce-buffering the page tables tends to make things go horribly wrong.

Robin.

>  Either way, I
> don't understand where the correct value is going to be overridden.
>
> +       /*
> +        * Set default dma mask value for the table walker,
> +        * to be overridden on probing with correct value.
> +        */
> +       *pdev->dev.dma_mask = DMA_BIT_MASK(32);
> +       pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko April 21, 2016, 10:45 p.m. UTC | #6
On Thu, Apr 14, 2016 at 8:25 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On DT based systems, the of_dma_configure() API implements DMA configuration
> for a given device. On ACPI systems an API equivalent to of_dma_configure()
> is missing which implies that it is currently not possible to set-up DMA
> operations for devices through the ACPI generic kernel layer.
>
> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> calls, that carry out IOMMU configuration through IORT (on systems where
> it is present) and call arch_setup_dma_ops(...) with the retrieved
> parameters.
>
> The DMA range size passed to arch_setup_dma_ops() is sized according
> to the device coherent_dma_mask (starting at address 0x0), mirroring the
> DT probing path behaviour when a dma-ranges property is not provided
> for the device being probed; this changes the current arch_setup_dma_ops()
> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> is a NOP on all architectures but ARM/ARM64 this patch does not change
> the current kernel behaviour on them.
>
> This patch updates ACPI and PCI core code to use the newly introduced
> acpi_dma_configure function, providing the same functionality
> as of_dma_configure on ARM systems and leaving behaviour unchanged
> for all other arches.
>

Nitpicks below.

> --- a/drivers/acpi/iort.c
> +++ b/drivers/acpi/iort.c
> @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
>         return 0;
>  }
>
> +/**
> + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
> + *
> + * @node: IORT table node to be looked-up
> + *
> + * Returns: iort_iommu_node pointer on success
> + *          NULL on failure
> + */
> +static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
> +{
> +       struct iort_iommu_node *iommu_node;
> +
> +       spin_lock(&iort_iommu_lock);
> +       list_for_each_entry(iommu_node, &iort_iommu_list, list) {
> +               if (iommu_node->node == node)
> +                       goto found;
> +       }
> +
> +       iommu_node = NULL;
> +found:
> +       spin_unlock(&iort_iommu_lock);
> +
> +       return iommu_node;

Ouch, and why not to

strut iommu_node = NULL;

lock
list for each() {
 if ()
  break;
}
unlock

return iommu_node;

?

> +}


> +/**
> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> + *
> + * @dev: device that requires IOMMU set-up
> + *
> + * Returns: iommu_ops pointer on configuration success
> + *          NULL on configuration failure
> + */
> +struct iommu_ops *iort_iommu_configure(struct device *dev)
> +{
> +       struct acpi_iort_node *node, *parent;
> +       struct iommu_ops *ops = NULL;
> +       struct iommu_fwspec fwspec;
> +       struct iort_iommu_node *iommu_node;
> +       u32 rid = 0, devid = 0;
> +
> +       if (dev_is_pci(dev)) {
> +               struct pci_bus *bus = to_pci_dev(dev)->bus;
> +
> +               pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> +                                      &rid);
> +
> +               node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> +                                     iort_find_dev_callback, &bus->dev);

> +       } else

checkpatch.pl ?

> +               node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> +                                     iort_find_dev_callback, dev);
> +
> +       if (!node)
> +               return NULL;
> +
> +       iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> +
> +       parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);

> +

Redundant.

> +       if (!parent)
> +               return NULL;
> +
> +       iommu_node = iort_iommu_get_node(parent);
> +       ops = iommu_node->ops;
> +
> +       fwspec.fwnode = iommu_node->fwnode;
> +       fwspec.param_count = 1;
> +       fwspec.param[0] = devid;
> +
> +       if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> +               return NULL;
> +
> +       return ops;
> +}
> +
Lorenzo Pieralisi April 22, 2016, 10:57 a.m. UTC | #7
Hi Andy,

On Fri, Apr 22, 2016 at 01:45:38AM +0300, Andy Shevchenko wrote:
> On Thu, Apr 14, 2016 at 8:25 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On DT based systems, the of_dma_configure() API implements DMA configuration
> > for a given device. On ACPI systems an API equivalent to of_dma_configure()
> > is missing which implies that it is currently not possible to set-up DMA
> > operations for devices through the ACPI generic kernel layer.
> >
> > This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> > calls, that carry out IOMMU configuration through IORT (on systems where
> > it is present) and call arch_setup_dma_ops(...) with the retrieved
> > parameters.
> >
> > The DMA range size passed to arch_setup_dma_ops() is sized according
> > to the device coherent_dma_mask (starting at address 0x0), mirroring the
> > DT probing path behaviour when a dma-ranges property is not provided
> > for the device being probed; this changes the current arch_setup_dma_ops()
> > call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> > is a NOP on all architectures but ARM/ARM64 this patch does not change
> > the current kernel behaviour on them.
> >
> > This patch updates ACPI and PCI core code to use the newly introduced
> > acpi_dma_configure function, providing the same functionality
> > as of_dma_configure on ARM systems and leaving behaviour unchanged
> > for all other arches.
> >
> 
> Nitpicks below.

Thanks for having a look.

> > --- a/drivers/acpi/iort.c
> > +++ b/drivers/acpi/iort.c
> > @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
> >         return 0;
> >  }
> >
> > +/**
> > + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
> > + *
> > + * @node: IORT table node to be looked-up
> > + *
> > + * Returns: iort_iommu_node pointer on success
> > + *          NULL on failure
> > + */
> > +static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
> > +{
> > +       struct iort_iommu_node *iommu_node;
> > +
> > +       spin_lock(&iort_iommu_lock);
> > +       list_for_each_entry(iommu_node, &iort_iommu_list, list) {
> > +               if (iommu_node->node == node)
> > +                       goto found;
> > +       }
> > +
> > +       iommu_node = NULL;
> > +found:
> > +       spin_unlock(&iort_iommu_lock);
> > +
> > +       return iommu_node;
> 
> Ouch, and why not to
> 
> strut iommu_node = NULL;
> 
> lock
> list for each() {
>  if ()
>   break;
> }
> unlock
> 
> return iommu_node;

To make sure iommu_node is NULL if no node is found, but this list handling
function needs updating anyway (both locking and list handling), so I will
rework it and take your suggestion into account, I agree it is not that
readable (or safe to begin with).

> ?
> 
> > +}
> 
> 
> > +/**
> > + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> > + *
> > + * @dev: device that requires IOMMU set-up
> > + *
> > + * Returns: iommu_ops pointer on configuration success
> > + *          NULL on configuration failure
> > + */
> > +struct iommu_ops *iort_iommu_configure(struct device *dev)
> > +{
> > +       struct acpi_iort_node *node, *parent;
> > +       struct iommu_ops *ops = NULL;
> > +       struct iommu_fwspec fwspec;
> > +       struct iort_iommu_node *iommu_node;
> > +       u32 rid = 0, devid = 0;
> > +
> > +       if (dev_is_pci(dev)) {
> > +               struct pci_bus *bus = to_pci_dev(dev)->bus;
> > +
> > +               pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> > +                                      &rid);
> > +
> > +               node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> > +                                     iort_find_dev_callback, &bus->dev);
> 
> > +       } else
> 
> checkpatch.pl ?

checkpatch.pl --strict does not even barf at it. I will add the braces
and go check why checkpatch.pl is quiet :)

> > +               node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> > +                                     iort_find_dev_callback, dev);
> > +
> > +       if (!node)
> > +               return NULL;
> > +
> > +       iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
> > +
> > +       parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
> 
> > +
> 
> Redundant.

Ok.

Thanks,
Lorenzo

> 

> > +       if (!parent)
> > +               return NULL;
> > +
> > +       iommu_node = iort_iommu_get_node(parent);
> > +       ops = iommu_node->ops;
> > +
> > +       fwspec.fwnode = iommu_node->fwnode;
> > +       fwspec.param_count = 1;
> > +       fwspec.param[0] = devid;
> > +
> > +       if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
> > +               return NULL;
> > +
> > +       return ops;
> > +}
> > +
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Nowicki May 16, 2016, 3:15 p.m. UTC | #8
On 18.04.2016 12:43, Robin Murphy wrote:
> On 15/04/16 19:29, Timur Tabi wrote:
>> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>>> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
>>> +{
>>> +       struct iommu_ops *iommu;
>>> +
>>> +       iommu = iort_iommu_configure(dev);
>>> +
>>> +       /*
>>> +        * Assume dma valid range starts at 0 and covers the whole
>>> +        * coherent_dma_mask.
>>> +        */
>>> +       arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
>>> +                          attr == DEV_DMA_COHERENT);
>>> +}
>>
>> I have a network driver that is impacted by this code, so thank you
>> for posting this. (See
>> https://www.mail-archive.com/netdev@vger.kernel.org/msg106249.html).
>>
>> One one SOC, the driver needs to set the mask to 32 bits.  On another
>> SOC, it needs to set it to 64 bits.  On device tree, the driver will
>> use dma-ranges.
>
> That's the wrong way to look at it - the driver isn't _using_
> dma-ranges, you're merely relying on the OF code setting the _default_
> DMA mask differently based on the property. If your driver is in the
> minority of those which actually care about DMA masks, then it should be
> calling dma_set_mask_and_coherent() appropriately and not relying on the
> default.

I don't see the clear strategy for setting DMA mask as well.

Lets consider DT boot method example:
1. SMMUv2 supports 48bit translation and 1:1 address map
dma-ranges = <0x0 0x0 0x0 0x0 0x00010000 0x0>;
and we are adding PCI device:

pci_device_add -> DMA_BIT_MASK(32) by default
   pci_dma_configure
     of_dma_configure -> reads dma-ranges and calculates 48bit DMA mask, 
but it picks minimum, we stay with DMA_BIT_MASK(32)

now PCI dev turns out to be e1000e NIC:
e1000_probe
   dma_set_mask_and_coherent -> tries to set DMA_BIT_MASK(64)
     dma_set_mask -> there is no set_dma_mask ops for SMMUv2 so we let 
it be DMA_BIT_MASK(64). From that point on, we let to use memory which 
SMMUv2 cannot work with.

Does lack of set_dma_mask is the only missing thing here?

Thanks,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Nowicki May 16, 2016, 3:26 p.m. UTC | #9
On 16.05.2016 17:15, Tomasz Nowicki wrote:
> On 18.04.2016 12:43, Robin Murphy wrote:
>> On 15/04/16 19:29, Timur Tabi wrote:
>>> On Thu, Apr 14, 2016 at 12:25 PM, Lorenzo Pieralisi
>>> <lorenzo.pieralisi@arm.com> wrote:
>>>> +void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
>>>> +{
>>>> +       struct iommu_ops *iommu;
>>>> +
>>>> +       iommu = iort_iommu_configure(dev);
>>>> +
>>>> +       /*
>>>> +        * Assume dma valid range starts at 0 and covers the whole
>>>> +        * coherent_dma_mask.
>>>> +        */
>>>> +       arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
>>>> +                          attr == DEV_DMA_COHERENT);
>>>> +}
>>>
>>> I have a network driver that is impacted by this code, so thank you
>>> for posting this. (See
>>> https://www.mail-archive.com/netdev@vger.kernel.org/msg106249.html).
>>>
>>> One one SOC, the driver needs to set the mask to 32 bits.  On another
>>> SOC, it needs to set it to 64 bits.  On device tree, the driver will
>>> use dma-ranges.
>>
>> That's the wrong way to look at it - the driver isn't _using_
>> dma-ranges, you're merely relying on the OF code setting the _default_
>> DMA mask differently based on the property. If your driver is in the
>> minority of those which actually care about DMA masks, then it should be
>> calling dma_set_mask_and_coherent() appropriately and not relying on the
>> default.
>
> I don't see the clear strategy for setting DMA mask as well.
>
> Lets consider DT boot method example:
> 1. SMMUv2 supports 48bit translation and 1:1 address map
> dma-ranges = <0x0 0x0 0x0 0x0 0x00010000 0x0>;
> and we are adding PCI device:
>
> pci_device_add -> DMA_BIT_MASK(32) by default
>    pci_dma_configure
>      of_dma_configure -> reads dma-ranges and calculates 48bit DMA mask,
> but it picks minimum, we stay with DMA_BIT_MASK(32)
>
> now PCI dev turns out to be e1000e NIC:
> e1000_probe
>    dma_set_mask_and_coherent -> tries to set DMA_BIT_MASK(64)
>      dma_set_mask -> there is no set_dma_mask ops for SMMUv2 so we let

Sorry, there is no .set_dma_mask ops for ARM64 (not SMMUv2)

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Nowicki May 17, 2016, 8:07 a.m. UTC | #10
Hi Lorenzo,

On 14.04.2016 19:25, Lorenzo Pieralisi wrote:
> On DT based systems, the of_dma_configure() API implements DMA configuration
> for a given device. On ACPI systems an API equivalent to of_dma_configure()
> is missing which implies that it is currently not possible to set-up DMA
> operations for devices through the ACPI generic kernel layer.
>
> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> calls, that carry out IOMMU configuration through IORT (on systems where
> it is present) and call arch_setup_dma_ops(...) with the retrieved
> parameters.
>
> The DMA range size passed to arch_setup_dma_ops() is sized according
> to the device coherent_dma_mask (starting at address 0x0), mirroring the
> DT probing path behaviour when a dma-ranges property is not provided
> for the device being probed; this changes the current arch_setup_dma_ops()
> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
> is a NOP on all architectures but ARM/ARM64 this patch does not change
> the current kernel behaviour on them.
>
> This patch updates ACPI and PCI core code to use the newly introduced
> acpi_dma_configure function, providing the same functionality
> as of_dma_configure on ARM systems and leaving behaviour unchanged
> for all other arches.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
>   drivers/acpi/glue.c     |  4 +--
>   drivers/acpi/iort.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/acpi/scan.c     | 29 +++++++++++++++++
>   drivers/pci/probe.c     |  3 +-
>   include/acpi/acpi_bus.h |  2 ++
>   include/linux/acpi.h    |  5 +++
>   include/linux/iort.h    |  9 ++++++
>   7 files changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
> index 5ea5dc2..f8d6564 100644
> --- a/drivers/acpi/glue.c
> +++ b/drivers/acpi/glue.c
> @@ -227,8 +227,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
>
>   	attr = acpi_get_dma_attr(acpi_dev);
>   	if (attr != DEV_DMA_NOT_SUPPORTED)
> -		arch_setup_dma_ops(dev, 0, 0, NULL,
> -				   attr == DEV_DMA_COHERENT);
> +		acpi_dma_configure(dev, attr);
>
>   	acpi_physnode_link_name(physical_node_name, node_id);
>   	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
> @@ -251,6 +250,7 @@ int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
>   	return 0;
>
>    err:
> +	acpi_dma_deconfigure(dev);
>   	ACPI_COMPANION_SET(dev, NULL);
>   	put_device(dev);
>   	put_device(&acpi_dev->dev);
> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
> index 2b5ce65..b1bb8fb 100644
> --- a/drivers/acpi/iort.c
> +++ b/drivers/acpi/iort.c
> @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
>   	return 0;
>   }
>
> +/**
> + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
> + *
> + * @node: IORT table node to be looked-up
> + *
> + * Returns: iort_iommu_node pointer on success
> + *          NULL on failure
> + */
> +static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
> +{
> +	struct iort_iommu_node *iommu_node;
> +
> +	spin_lock(&iort_iommu_lock);
> +	list_for_each_entry(iommu_node, &iort_iommu_list, list) {
> +		if (iommu_node->node == node)
> +			goto found;
> +	}
> +
> +	iommu_node = NULL;
> +found:
> +	spin_unlock(&iort_iommu_lock);
> +
> +	return iommu_node;
> +}
> +
>   typedef acpi_status (*iort_find_node_callback)
>   	(struct acpi_iort_node *node, void *context);
>
> @@ -405,6 +430,66 @@ iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
>   	return domain_handle;
>   }
>
> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> +{
> +	u32 *rid = data;
> +
> +	*rid = alias;
> +	return 0;
> +}
> +
> +/**
> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
> + *
> + * @dev: device that requires IOMMU set-up
> + *
> + * Returns: iommu_ops pointer on configuration success
> + *          NULL on configuration failure
> + */
> +struct iommu_ops *iort_iommu_configure(struct device *dev)
> +{
> +	struct acpi_iort_node *node, *parent;
> +	struct iommu_ops *ops = NULL;
> +	struct iommu_fwspec fwspec;
> +	struct iort_iommu_node *iommu_node;
> +	u32 rid = 0, devid = 0;
> +
> +	if (dev_is_pci(dev)) {
> +		struct pci_bus *bus = to_pci_dev(dev)->bus;
> +
> +		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> +				       &rid);
> +
> +		node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
> +				      iort_find_dev_callback, &bus->dev);
> +	} else
> +		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
> +				      iort_find_dev_callback, dev);

I think this will not work for finding host bridge stream ID. We still 
need to use ACPI_IORT_NODE_PCI_ROOT_COMPLEX but 'dev' is not PCI device, 
right?

Thanks,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Nowicki May 17, 2016, 12:32 p.m. UTC | #11
On 17.05.2016 10:07, Tomasz Nowicki wrote:
> Hi Lorenzo,
>
> On 14.04.2016 19:25, Lorenzo Pieralisi wrote:
>> On DT based systems, the of_dma_configure() API implements DMA
>> configuration
>> for a given device. On ACPI systems an API equivalent to
>> of_dma_configure()
>> is missing which implies that it is currently not possible to set-up DMA
>> operations for devices through the ACPI generic kernel layer.
>>
>> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
>> calls, that carry out IOMMU configuration through IORT (on systems where
>> it is present) and call arch_setup_dma_ops(...) with the retrieved
>> parameters.
>>
>> The DMA range size passed to arch_setup_dma_ops() is sized according
>> to the device coherent_dma_mask (starting at address 0x0), mirroring the
>> DT probing path behaviour when a dma-ranges property is not provided
>> for the device being probed; this changes the current
>> arch_setup_dma_ops()
>> call parameters in the ACPI probing case, but since arch_setup_dma_ops()
>> is a NOP on all architectures but ARM/ARM64 this patch does not change
>> the current kernel behaviour on them.
>>
>> This patch updates ACPI and PCI core code to use the newly introduced
>> acpi_dma_configure function, providing the same functionality
>> as of_dma_configure on ARM systems and leaving behaviour unchanged
>> for all other arches.
>>
>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Tomasz Nowicki <tn@semihalf.com>
>> Cc: Joerg Roedel <joro@8bytes.org>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> ---
>>   drivers/acpi/glue.c     |  4 +--
>>   drivers/acpi/iort.c     | 85
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/acpi/scan.c     | 29 +++++++++++++++++
>>   drivers/pci/probe.c     |  3 +-
>>   include/acpi/acpi_bus.h |  2 ++
>>   include/linux/acpi.h    |  5 +++
>>   include/linux/iort.h    |  9 ++++++
>>   7 files changed, 133 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
>> index 5ea5dc2..f8d6564 100644
>> --- a/drivers/acpi/glue.c
>> +++ b/drivers/acpi/glue.c
>> @@ -227,8 +227,7 @@ int acpi_bind_one(struct device *dev, struct
>> acpi_device *acpi_dev)
>>
>>       attr = acpi_get_dma_attr(acpi_dev);
>>       if (attr != DEV_DMA_NOT_SUPPORTED)
>> -        arch_setup_dma_ops(dev, 0, 0, NULL,
>> -                   attr == DEV_DMA_COHERENT);
>> +        acpi_dma_configure(dev, attr);
>>
>>       acpi_physnode_link_name(physical_node_name, node_id);
>>       retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
>> @@ -251,6 +250,7 @@ int acpi_bind_one(struct device *dev, struct
>> acpi_device *acpi_dev)
>>       return 0;
>>
>>    err:
>> +    acpi_dma_deconfigure(dev);
>>       ACPI_COMPANION_SET(dev, NULL);
>>       put_device(dev);
>>       put_device(&acpi_dev->dev);
>> diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
>> index 2b5ce65..b1bb8fb 100644
>> --- a/drivers/acpi/iort.c
>> +++ b/drivers/acpi/iort.c
>> @@ -72,6 +72,31 @@ int iort_iommu_set_node(struct iommu_ops *ops,
>> struct acpi_iort_node *node,
>>       return 0;
>>   }
>>
>> +/**
>> + * iort_iommu_get_node - Retrieve iort_iommu_node associated with an
>> IORT node.
>> + *
>> + * @node: IORT table node to be looked-up
>> + *
>> + * Returns: iort_iommu_node pointer on success
>> + *          NULL on failure
>> + */
>> +static struct iort_iommu_node *iort_iommu_get_node(struct
>> acpi_iort_node *node)
>> +{
>> +    struct iort_iommu_node *iommu_node;
>> +
>> +    spin_lock(&iort_iommu_lock);
>> +    list_for_each_entry(iommu_node, &iort_iommu_list, list) {
>> +        if (iommu_node->node == node)
>> +            goto found;
>> +    }
>> +
>> +    iommu_node = NULL;
>> +found:
>> +    spin_unlock(&iort_iommu_lock);
>> +
>> +    return iommu_node;
>> +}
>> +
>>   typedef acpi_status (*iort_find_node_callback)
>>       (struct acpi_iort_node *node, void *context);
>>
>> @@ -405,6 +430,66 @@ iort_pci_get_domain(struct pci_dev *pdev, u32
>> req_id)
>>       return domain_handle;
>>   }
>>
>> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> +    u32 *rid = data;
>> +
>> +    *rid = alias;
>> +    return 0;
>> +}
>> +
>> +/**
>> + * iort_iommu_configure - Set-up IOMMU configuration for a device.
>> + *
>> + * @dev: device that requires IOMMU set-up
>> + *
>> + * Returns: iommu_ops pointer on configuration success
>> + *          NULL on configuration failure
>> + */
>> +struct iommu_ops *iort_iommu_configure(struct device *dev)
>> +{
>> +    struct acpi_iort_node *node, *parent;
>> +    struct iommu_ops *ops = NULL;
>> +    struct iommu_fwspec fwspec;
>> +    struct iort_iommu_node *iommu_node;
>> +    u32 rid = 0, devid = 0;
>> +
>> +    if (dev_is_pci(dev)) {
>> +        struct pci_bus *bus = to_pci_dev(dev)->bus;
>> +
>> +        pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
>> +                       &rid);
>> +
>> +        node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
>> +                      iort_find_dev_callback, &bus->dev);
>> +    } else
>> +        node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
>> +                      iort_find_dev_callback, dev);
>
> I think this will not work for finding host bridge stream ID. We still
> need to use ACPI_IORT_NODE_PCI_ROOT_COMPLEX but 'dev' is not PCI device,
> right?

After private conversation, I think this part is OK.

Thanks,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index 5ea5dc2..f8d6564 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -227,8 +227,7 @@  int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 
 	attr = acpi_get_dma_attr(acpi_dev);
 	if (attr != DEV_DMA_NOT_SUPPORTED)
-		arch_setup_dma_ops(dev, 0, 0, NULL,
-				   attr == DEV_DMA_COHERENT);
+		acpi_dma_configure(dev, attr);
 
 	acpi_physnode_link_name(physical_node_name, node_id);
 	retval = sysfs_create_link(&acpi_dev->dev.kobj, &dev->kobj,
@@ -251,6 +250,7 @@  int acpi_bind_one(struct device *dev, struct acpi_device *acpi_dev)
 	return 0;
 
  err:
+	acpi_dma_deconfigure(dev);
 	ACPI_COMPANION_SET(dev, NULL);
 	put_device(dev);
 	put_device(&acpi_dev->dev);
diff --git a/drivers/acpi/iort.c b/drivers/acpi/iort.c
index 2b5ce65..b1bb8fb 100644
--- a/drivers/acpi/iort.c
+++ b/drivers/acpi/iort.c
@@ -72,6 +72,31 @@  int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
 	return 0;
 }
 
+/**
+ * iort_iommu_get_node - Retrieve iort_iommu_node associated with an IORT node.
+ *
+ * @node: IORT table node to be looked-up
+ *
+ * Returns: iort_iommu_node pointer on success
+ *          NULL on failure
+ */
+static struct iort_iommu_node *iort_iommu_get_node(struct acpi_iort_node *node)
+{
+	struct iort_iommu_node *iommu_node;
+
+	spin_lock(&iort_iommu_lock);
+	list_for_each_entry(iommu_node, &iort_iommu_list, list) {
+		if (iommu_node->node == node)
+			goto found;
+	}
+
+	iommu_node = NULL;
+found:
+	spin_unlock(&iort_iommu_lock);
+
+	return iommu_node;
+}
+
 typedef acpi_status (*iort_find_node_callback)
 	(struct acpi_iort_node *node, void *context);
 
@@ -405,6 +430,66 @@  iort_pci_get_domain(struct pci_dev *pdev, u32 req_id)
 	return domain_handle;
 }
 
+static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
+{
+	u32 *rid = data;
+
+	*rid = alias;
+	return 0;
+}
+
+/**
+ * iort_iommu_configure - Set-up IOMMU configuration for a device.
+ *
+ * @dev: device that requires IOMMU set-up
+ *
+ * Returns: iommu_ops pointer on configuration success
+ *          NULL on configuration failure
+ */
+struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+	struct acpi_iort_node *node, *parent;
+	struct iommu_ops *ops = NULL;
+	struct iommu_fwspec fwspec;
+	struct iort_iommu_node *iommu_node;
+	u32 rid = 0, devid = 0;
+
+	if (dev_is_pci(dev)) {
+		struct pci_bus *bus = to_pci_dev(dev)->bus;
+
+		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
+				       &rid);
+
+		node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX,
+				      iort_find_dev_callback, &bus->dev);
+	} else
+		node = iort_scan_node(ACPI_IORT_NODE_NAMED_COMPONENT,
+				      iort_find_dev_callback, dev);
+
+	if (!node)
+		return NULL;
+
+	iort_dev_map_rid(node, rid, &devid, ACPI_IORT_NODE_SMMU);
+
+	parent = iort_find_parent_node(node, ACPI_IORT_NODE_SMMU);
+
+	if (!parent)
+		return NULL;
+
+	iommu_node = iort_iommu_get_node(parent);
+	ops = iommu_node->ops;
+
+	fwspec.fwnode = iommu_node->fwnode;
+	fwspec.param_count = 1;
+	fwspec.param[0] = devid;
+
+	if (!ops || !ops->fw_xlate || ops->fw_xlate(dev, &fwspec))
+		return NULL;
+
+	return ops;
+}
+
+
 static int __init iort_table_detect(void)
 {
 	acpi_status status;
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5f28cf7..e0fd5e3 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -7,6 +7,7 @@ 
 #include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/acpi.h>
+#include <linux/iort.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
 #include <linux/dmi.h>
@@ -1358,6 +1359,34 @@  enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 		return DEV_DMA_NON_COHERENT;
 }
 
+/**
+ * acpi_dma_configure - Set-up DMA configuration for the device.
+ * @dev: The pointer to the device
+ * @attr: device dma attributes
+ */
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr)
+{
+	struct iommu_ops *iommu;
+
+	iommu = iort_iommu_configure(dev);
+
+	/*
+	 * Assume dma valid range starts at 0 and covers the whole
+	 * coherent_dma_mask.
+	 */
+	arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu,
+			   attr == DEV_DMA_COHERENT);
+}
+
+/**
+ * acpi_dma_deconfigure - Tear-down DMA configuration for the device.
+ * @dev: The pointer to the device
+ */
+void acpi_dma_deconfigure(struct device *dev)
+{
+	arch_teardown_dma_ops(dev);
+}
+
 static void acpi_init_coherency(struct acpi_device *adev)
 {
 	unsigned long long cca = 0;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ef569e8..9cf90b8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1725,8 +1725,7 @@  static void pci_dma_configure(struct pci_dev *dev)
 		if (attr == DEV_DMA_NOT_SUPPORTED)
 			dev_warn(&dev->dev, "DMA not supported.\n");
 		else
-			arch_setup_dma_ops(&dev->dev, 0, 0, NULL,
-					   attr == DEV_DMA_COHERENT);
+			acpi_dma_configure(&dev->dev, attr);
 	}
 
 	pci_put_host_bridge_device(bridge);
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 14362a8..212eff2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -566,6 +566,8 @@  struct acpi_pci_root {
 
 bool acpi_dma_supported(struct acpi_device *adev);
 enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev);
+void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr);
+void acpi_dma_deconfigure(struct device *dev);
 
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
 					   u64 address, bool check_children);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..69b9041 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -683,6 +683,11 @@  static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev)
 	return DEV_DMA_NOT_SUPPORTED;
 }
 
+static inline void acpi_dma_configure(struct device *dev,
+				      enum dev_dma_attr attr) { }
+
+static inline void acpi_dma_deconfigure(struct device *dev) { }
+
 #define ACPI_PTR(_ptr)	(NULL)
 
 #endif	/* !CONFIG_ACPI */
diff --git a/include/linux/iort.h b/include/linux/iort.h
index 766adda..7a7af40 100644
--- a/include/linux/iort.h
+++ b/include/linux/iort.h
@@ -31,6 +31,15 @@  struct fwnode_handle *iort_pci_get_domain(struct pci_dev *pdev, u32 req_id);
 int iort_iommu_set_node(struct iommu_ops *ops, struct acpi_iort_node *node,
 			struct fwnode_handle *fwnode);
 
+#ifdef CONFIG_IORT_TABLE
+struct iommu_ops *iort_iommu_configure(struct device *dev);
+#else
+static inline struct iommu_ops *iort_iommu_configure(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 #define IORT_ACPI_DECLARE(name, table_id, fn)		\
 	ACPI_DECLARE_PROBE_ENTRY(iort, name, table_id, 0, NULL, 0, fn)