arm64: iort: take _DMA methods into account for named components
Subsystems (2)
PATCH 1/1 arm64: iort: take _DMA methods into account for named components
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver, _DMA methods in AML can express more complex topologies, involving DMA translation in particular.
Currently, we only take this _DMA method into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.
--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/acpi/arm64/iort.c +4 -7
@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)else size =1ULL <<32 ; -if (dev_is_pci (dev)) { - ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); -if (ret == -ENODEV ) - ret =rc_dma_get_range (dev, &size); - }else { - ret =nc_dma_get_range (dev, &size); - } + ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); +if (ret == -ENODEV ) + ret =dev_is_pci (dev) ?rc_dma_get_range (dev, &size) + :nc_dma_get_range (dev, &size);if (!ret) {/*
Signature · 7 lines
2.17.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 9 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.
ACPI spec v6.3, 6.2.4 _DMA:
"_DMA is only defined under devices that represent buses"
This should probably be updated and _DMA usage clarified - we can't leave it open to interpretation.
Thanks,
Lorenzo
Quoted 33 lines
Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)drivers/acpi/arm64/iort.c +4
-7@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)else size =1ULL <<32 ; -if (dev_is_pci (dev)) { - ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); -if (ret == -ENODEV ) - ret =rc_dma_get_range (dev, &size); - }else { - ret =nc_dma_get_range (dev, &size); - } + ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); +if (ret == -ENODEV ) + ret =dev_is_pci (dev) ?rc_dma_get_range (dev, &size) + :nc_dma_get_range (dev, &size);if (!ret) {/* Signature · 1 line
2.17.1
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.
Quoted 9 lines
Quoted 4 lines
Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"
Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Quoted 3 lines
This should probably be updated and
_DMAusage clarified - we can't leave it open to interpretation.
Clarification is always better.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 14 lines · Ard Biesheuvel
Quoted 12 lines · Lorenzo Pieralisi
Quoted 9 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.
Quoted 8 lines
Quoted 4 lines
ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem to _require_ a _CRS to be considered valid.
It is an example. Better to make the _DMA definition more robust and linked to ACPI0004, for instance.
Quoted 5 lines
Quoted 3 lines
This should probably be updated and
_DMAusage clarified - we can't leave it open to interpretation.Clarification is always better.
Yes, we should be able to get this in as an Errata, better to be clear given that it is something that will be used extensively across platforms.
Thanks !
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 21 lines · Lorenzo Pieralisi
Quoted 19 lines · Ard Biesheuvel
Quoted 16 lines · Lorenzo Pieralisi
Quoted 9 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"
Quoted 7 lines
Quoted 3 lines
Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Yes.
_DMAis valid only if a_CRSis present, ACPI0004 does not seem to _require_ a_CRSto be considered valid.
How is that relevant? Any node that has a _DMA must have a _CRS as well. Some nodes that don't have a _DMA may not have a _CRS either. But that does not disqualify a ACPI0004 that *does* have both from being considered a bus node, no? Or is that not what you are saying?
Quoted 3 lines
It is an example. Better to make the
_DMAdefinition more robust and linked to ACPI0004, for instance.
If there is wording in the spec that says that only APCI0004 or PNP0A03/PNP0A08 should be considered to be bus nodes, then we should probably do that. But I wonder if that is really the intent, and whether vendors could denote bus nodes using their own HID namespace instead.
Quoted 10 lines
Quoted 5 lines
Quoted 3 lines
This should probably be updated and
_DMAusage clarified - we can't leave it open to interpretation.Clarification is always better.
Yes, we should be able to get this in as an Errata, better to be clear given that it is something that will be used extensively across platforms.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 27 lines · Ard Biesheuvel
Quoted 25 lines · Lorenzo Pieralisi
Quoted 22 lines · Ard Biesheuvel
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Quoted 9 lines
Quoted 4 lines
Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Quoted 8 lines
Quoted 3 lines
Yes.
_DMAis valid only if a_CRSis present, ACPI0004 does not seem to _require_ a_CRSto be considered valid.How is that relevant? Any node that has a
_DMAmust have a_CRSas well. Some nodes that don't have a_DMAmay not have a_CRSeither. But that does not disqualify a ACPI0004 that *does* have both from being considered a bus node, no? Or is that not what you are saying?
I am just trying to prevent firmware developers from adding ACPI0004 nodes with *just* a _DMA object (because the _CRS is optional) which will become unusable in OS context (where we do check the _CRS presence), that's all I am saying.
Just trying to make specs foolproof :)
Thanks,
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 38 lines · Lorenzo Pieralisi
Quoted 27 lines · Ard Biesheuvel
Quoted 25 lines · Lorenzo Pieralisi
Quoted 22 lines · Ard Biesheuvel
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Quoted 9 lines
Quoted 4 lines
Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Quoted 8 lines
Quoted 3 lines
Yes.
_DMAis valid only if a_CRSis present, ACPI0004 does not seem to _require_ a_CRSto be considered valid.How is that relevant? Any node that has a
_DMAmust have a_CRSas well. Some nodes that don't have a_DMAmay not have a_CRSeither. But that does not disqualify a ACPI0004 that *does* have both from being considered a bus node, no? Or is that not what you are saying?
Quoted 7 lines
I am just trying to prevent firmware developers from adding ACPI0004 nodes with *just* a
_DMAobject (because the_CRSis optional) which will become unusable in OS context (where we do check the_CRSpresence), that's all I am saying.Just trying to make specs foolproof :)
Ah ok, fair enough.
Note that acpi_dma_get_range() already checks this, but on the firmware validation side, adding a rule like this would certainly help as well.
I think the window for new ACPI material is closing atm - I'll check internally whether we can get someone to slip this in (i.e., a clarification added to '9.12 Module Device' that _DMA methods are permitted but only if _CRS is defined as well)
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 51 lines · Ard Biesheuvel
Quoted 38 lines · Lorenzo Pieralisi
Quoted 27 lines · Ard Biesheuvel
Quoted 25 lines · Lorenzo Pieralisi
Quoted 22 lines · Ard Biesheuvel
Quoted 7 lines · Lorenzo Pieralisi
Quoted 5 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Quoted 9 lines
Quoted 4 lines
Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.ACPI spec v6.3, 6.2.4
_DMA:"
_DMAis only defined under devices that represent buses"Sure. But ACPI0004 module devices are also bus nodes, so that statement does not exclude named components that are defined under such a module device.
Quoted 8 lines
Quoted 3 lines
Yes.
_DMAis valid only if a_CRSis present, ACPI0004 does not seem to _require_ a_CRSto be considered valid.How is that relevant? Any node that has a
_DMAmust have a_CRSas well. Some nodes that don't have a_DMAmay not have a_CRSeither. But that does not disqualify a ACPI0004 that *does* have both from being considered a bus node, no? Or is that not what you are saying?Quoted 7 lines
I am just trying to prevent firmware developers from adding ACPI0004 nodes with *just* a
_DMAobject (because the_CRSis optional) which will become unusable in OS context (where we do check the_CRSpresence), that's all I am saying.Just trying to make specs foolproof :)
Ah ok, fair enough.
Quoted 8 lines
Note that
acpi_dma_get_range()already checks this, but on the firmware validation side, adding a rule like this would certainly help as well.I think the window for new ACPI material is closing atm - I'll check internally whether we can get someone to slip this in (i.e., a clarification added to '9.12 Module Device' that
_DMAmethods are permitted but only if_CRSis defined as well)
So what is the verdict on this patch? We agree that the ACPI spec permits ACPI0004 containers with _CRS/_DMA method pairs, and child named components that rely on the described DMA translation to be honored by the OS. IOW, we should provide guidance to ensure that firmware providers don't get it wrong, but if they do get it right, we still need this change in order to take the translation into account.
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 10 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.
Quoted 7 lines
Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Only question is whether there is FW in the field with _DMA methods that now we would start parsing (and hopefully everything will still work) but for that the only choice is applying this patch and see, so:
Quoted 25 lines
drivers/acpi/arm64/iort.c +4
-7@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)else size =1ULL <<32 ; -if (dev_is_pci (dev)) { - ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); -if (ret == -ENODEV ) - ret =rc_dma_get_range (dev, &size); - }else { - ret =nc_dma_get_range (dev, &size); - } + ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); +if (ret == -ENODEV ) + ret =dev_is_pci (dev) ?rc_dma_get_range (dev, &size) + :nc_dma_get_range (dev, &size);if (!ret) {/* Signature · 1 line
2.17.1
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 20 lines · Lorenzo Pieralisi
Quoted 10 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.Quoted 7 lines
Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Quoted 4 lines
Only question is whether there is FW in the field with
_DMAmethods that now we would start parsing (and hopefully everything will still work) but for that the only choice is applying this patch and see, so:
Perhaps it would be better to call acpi_dma_get_range() on dev->parent then?
Quoted 22 lines
Quoted 20 lines
drivers/acpi/arm64/iort.c +4
-7@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)else size =1ULL <<32 ; -if (dev_is_pci (dev)) { - ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); -if (ret == -ENODEV ) - ret =rc_dma_get_range (dev, &size); - }else { - ret =nc_dma_get_range (dev, &size); - } + ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); +if (ret == -ENODEV ) + ret =dev_is_pci (dev) ?rc_dma_get_range (dev, &size) + :nc_dma_get_range (dev, &size);
Quoted 5 lines
Quoted 5 lines
drivers/acpi/arm64/iort.c +0
-1if (!ret) {/* -- 2.17.1
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 22 lines · Ard Biesheuvel
Quoted 20 lines · Lorenzo Pieralisi
Quoted 17 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Quoted 6 lines
Quoted 4 lines
Only question is whether there is FW in the field with
_DMAmethods that now we would start parsing (and hopefully everything will still work) but for that the only choice is applying this patch and see, so:Perhaps it would be better to call
acpi_dma_get_range()ondev->parentthen?
I think it is fine as it is - maybe we can hold off sending it all the way to stable kernels until we are confident it does not cause unintended breakage ?
Anyway, thanks for putting it together.
Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:" just to keep logs uniform.
Thanks,
Lorenzo
Quoted 22 lines
Quoted 22 lines
Quoted 20 lines
drivers/acpi/arm64/iort.c +4
-7@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)else size =1ULL <<32 ; -if (dev_is_pci (dev)) { - ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); -if (ret == -ENODEV ) - ret =rc_dma_get_range (dev, &size); - }else { - ret =nc_dma_get_range (dev, &size); - } + ret =acpi_dma_get_range (dev, &dmaaddr, &offset, &size); +if (ret == -ENODEV ) + ret =dev_is_pci (dev) ?rc_dma_get_range (dev, &size) + :nc_dma_get_range (dev, &size);
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/acpi/arm64/iort.c +0
-1if (!ret) {/* -- 2.17.1
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 31 lines · Lorenzo Pieralisi
Quoted 22 lines · Ard Biesheuvel
Quoted 20 lines · Lorenzo Pieralisi
Quoted 17 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)Quoted 6 lines
Quoted 4 lines
Only question is whether there is FW in the field with
_DMAmethods that now we would start parsing (and hopefully everything will still work) but for that the only choice is applying this patch and see, so:Perhaps it would be better to call
acpi_dma_get_range()ondev->parentthen?
Quoted 9 lines
I think it is fine as it is - maybe we can hold off sending it all the way to stable kernels until we are confident it does not cause unintended breakage ?
Anyway, thanks for putting it together.
Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:" just to keep logs uniform.
OK, I'll respin and resend, with the ACPI folks on cc this time.
Thanks
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Will Deacon
maintainerQuoted 37 lines · Ard Biesheuvel
Quoted 35 lines · Lorenzo Pieralisi
Quoted 22 lines · Ard Biesheuvel
Quoted 20 lines · Lorenzo Pieralisi
Quoted 17 lines · Ard Biesheuvel
Where IORT nodes for named components can describe simple DMA limits expressed as the number of address bits a device can driver,
_DMAmethods in AML can express more complex topologies, involving DMA translation in particular.Currently, we only take this
_DMAmethod into account if it appears on a ACPI device node describing a PCIe root complex, but it is perfectly acceptable to attach them to named components as well, so let's ensure we take them into account in those cases too.Reported-by Andrei Warkentin <[email protected]>Fixes 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")Cc <[email protected]> # v4.14+Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/acpi/arm64/iort.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)Quoted 6 lines
Quoted 4 lines
Only question is whether there is FW in the field with
_DMAmethods that now we would start parsing (and hopefully everything will still work) but for that the only choice is applying this patch and see, so:Perhaps it would be better to call
acpi_dma_get_range()ondev->parentthen?I think it is fine as it is - maybe we can hold off sending it all the way to stable kernels until we are confident it does not cause unintended breakage ?
Quoted 7 lines
Quoted 5 lines
Anyway, thanks for putting it together.
Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:" just to keep logs uniform.
OK, I'll respin and resend, with the ACPI folks on cc this time.
Thanks. I'm happy to queue this in the arm64 tree with the CC: stable dropped for now, so please keep me on cc for v2.
Will
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel