Hi Stewart,
On 18/05/2023 23:06, 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> # rename
> ---
> 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 | 42 +++++++++++++++++----------
> 1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/xen/drivers/passthrough/device_tree.c
> b/xen/drivers/passthrough/device_tree.c
> index b5bd13393b56..1b50f4670944 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -127,15 +127,39 @@ int iommu_release_dt_devices(struct domain *d)
> return 0;
> }
>
> +/* This correlation must not be altered */
> +#define NO_IOMMU 1
> +
> +static int iommu_dt_xlate(struct device *dev,
> + struct dt_phandle_args *iommu_spec)
I think iommu_spec can be const.
> +{
> + const struct iommu_ops *ops = iommu_get_ops();
> + int rc;
> +
> + 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);
Wouldn't it be better to move the check (!ops->dt_xlate) from
iommu_add_dt_device to this helper?
After all it is the only function that calls dt_xlate so for me it would be a
natural placement.
Looking at the next patch it will also reduce the similar check in
iommu_add_dt_pci_sideband_ids.
~Michal
