Hi Stewart,
On 07/06/2023 04:02, Stewart Hildebrand wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Move code for processing DT IOMMU specifier to a separate helper.
This helper will be re-used for adding PCI devices by the subsequent
patches as we will need exact the same actions for processing
DT PCI-IOMMU specifier.
While at it introduce NO_IOMMU to avoid magic "1".
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
---
v3->v4:
* make dt_phandle_args *iommu_spec const
* move !ops->add_device check to helper
v2->v3:
* no change
v1->v2:
* no change
downstream->v1:
* trivial rebase
* s/dt_iommu_xlate/iommu_dt_xlate/
(cherry picked from commit c26bab0415ca303df86aba1d06ef8edc713734d3 from
the downstream branch poc/pci-passthrough from
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
---
xen/drivers/passthrough/device_tree.c | 47 ++++++++++++++++++---------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/xen/drivers/passthrough/device_tree.c
b/xen/drivers/passthrough/device_tree.c
index c60e78eaf556..ff9e66ebf92a 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -127,15 +127,42 @@ int iommu_release_dt_devices(struct domain *d)
return 0;
}
+/* This correlation must not be altered */
Please expand why.
+#define NO_IOMMU 1
Given that the value is returned, should not this be moved to an header
and used by the callers?
+
+static int iommu_dt_xlate(struct device *dev,
+ const struct dt_phandle_args *iommu_spec)
+{
+ const struct iommu_ops *ops = iommu_get_ops();
+ int rc;
+
+ if ( !ops->dt_xlate )
+ return -EINVAL;
+
+ if ( !dt_device_is_available(iommu_spec->np) )
+ return NO_IOMMU;
+
+ rc = iommu_fwspec_init(dev, &iommu_spec->np->dev);
+ if ( rc )
+ return rc;
+
+ /*
+ * Provide DT IOMMU specifier which describes the IOMMU master
+ * interfaces of that device (device IDs, etc) to the driver.
+ * The driver is responsible to decide how to interpret them.
+ */
+ return ops->dt_xlate(dev, iommu_spec);
+}
+
int iommu_add_dt_device(struct dt_device_node *np)
{
const struct iommu_ops *ops = iommu_get_ops();
struct dt_phandle_args iommu_spec;
struct device *dev = dt_to_dev(np);
- int rc = 1, index = 0;
+ int rc = NO_IOMMU, index = 0;
if ( !iommu_enabled )
- return 1;
+ return NO_IOMMU;
if ( !ops )
return -EINVAL;
@@ -163,22 +190,10 @@ int iommu_add_dt_device(struct dt_device_node *np)
* The driver which supports generic IOMMU DT bindings must have
* these callback implemented.
The 's' was missing to callback. But now, I think you want to replace
'these' with 'this' as you move the second check.
*/
- if ( !ops->add_device || !ops->dt_xlate )
+ if ( !ops->add_device )
return -EINVAL;
- if ( !dt_device_is_available(iommu_spec.np) )
- break;
-
- rc = iommu_fwspec_init(dev, &iommu_spec.np->dev);
- if ( rc )
- break;
-
- /*
- * Provide DT IOMMU specifier which describes the IOMMU master
- * interfaces of that device (device IDs, etc) to the driver.
- * The driver is responsible to decide how to interpret them.
- */
- rc = ops->dt_xlate(dev, &iommu_spec);
+ rc = iommu_dt_xlate(dev, &iommu_spec);
if ( rc )
break;
Cheers,
--
Julien Grall
