Re: [PATCH RESEND v3 2/3] arm,smmu: restructure code in preparation to new bindings support

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 13/04/2021 18:59, Stefano Stabellini wrote:
> From: Brian Woods <brian.woods@xxxxxxxxxx>
>
> Restructure some of the code and add supporting functions for adding
> generic device tree (DT) binding support.  This will allow for using
> current Linux device trees with just modifying the chosen field to
> enable Xen.
>
> Signed-off-by: Brian Woods <brian.woods@xxxxxxxxxx>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

> ---
>   xen/drivers/passthrough/arm/smmu.c | 62 +++++++++++++++++-------------
>   1 file changed, 35 insertions(+), 27 deletions(-)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c 
> b/xen/drivers/passthrough/arm/smmu.c
> index ac75e23268..f949c110ad 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -810,52 +810,36 @@ static int insert_smmu_master(struct arm_smmu_device 
> *smmu,
>         return 0;
>   }
>   
> -static int register_smmu_master(struct arm_smmu_device *smmu,
> -                               struct device *dev,
> -                               struct of_phandle_args *masterspec)
> +static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu,
> +                                        struct device *dev,
> +                                        struct iommu_fwspec *fwspec)
>   {
> -       int i, ret = 0;
> +       int i;
>         struct arm_smmu_master *master;
> -       struct iommu_fwspec *fwspec;
> +       struct device_node *dev_node = dev_get_dev_node(dev);
>   
> -	master = find_smmu_master(smmu, masterspec->np);
> +       master = find_smmu_master(smmu, dev_node);
>         if (master) {
>                 dev_err(dev,
>                         "rejecting multiple registrations for master device 
> %s\n",
> -                       masterspec->np->name);
> +                       dev_node->name);
>                 return -EBUSY;
>         }
>   
>   	master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
>         if (!master)
>                 return -ENOMEM;
> -       master->of_node = masterspec->np;
> -
> -       ret = iommu_fwspec_init(&master->of_node->dev, smmu->dev);
> -       if (ret) {
> -               kfree(master);
> -               return ret;
> -       }
> -       fwspec = dev_iommu_fwspec_get(dev);
> -
> -       /* adding the ids here */
> -       ret = iommu_fwspec_add_ids(&masterspec->np->dev,
> -                                  masterspec->args,
> -                                  masterspec->args_count);
> -       if (ret)
> -               return ret;
> +       master->of_node = dev_node;
>   
>   	/* Xen: Let Xen know that the device is protected by an SMMU */
> -       dt_device_set_protected(masterspec->np);
> +       dt_device_set_protected(dev_node);
>   
>   	for (i = 0; i < fwspec->num_ids; ++i) {
> -               u16 streamid = masterspec->args[i];
> -
>                 if (!(smmu->features & ARM_SMMU_FEAT_STREAM_MATCH) &&
> -                    (streamid >= smmu->num_mapping_groups)) {
> +                    (fwspec->ids[i] >= smmu->num_mapping_groups)) {
>                         dev_err(dev,
>                                 "stream ID for master device %s greater than maximum 
> allowed (%d)\n",
> -                               masterspec->np->name, smmu->num_mapping_groups);
> +                               dev_node->name, smmu->num_mapping_groups);
>                         return -ERANGE;
>                 }
>                 master->cfg.smendx[i] = INVALID_SMENDX;
> @@ -864,6 +848,30 @@ static int register_smmu_master(struct arm_smmu_device 
> *smmu,
>         return insert_smmu_master(smmu, master);
>   }
>   
> +static int register_smmu_master(struct arm_smmu_device *smmu,
> +                               struct device *dev,
> +                               struct of_phandle_args *masterspec)
> +{
> +       int ret = 0;
> +       struct iommu_fwspec *fwspec;
> +
> +       ret = iommu_fwspec_init(&masterspec->np->dev, smmu->dev);
> +       if (ret)
> +               return ret;
> +
> +       fwspec = dev_iommu_fwspec_get(&masterspec->np->dev);
> +
> +       ret = iommu_fwspec_add_ids(&masterspec->np->dev,
> +                                  masterspec->args,
> +                                  masterspec->args_count);
> +       if (ret)
> +               return ret;
> +
> +       return arm_smmu_dt_add_device_legacy(smmu,
> +                                            &masterspec->np->dev,
> +                                            fwspec);
> +}
> +
>   static struct arm_smmu_device *find_smmu_for_device(struct device *dev)
>   {
>         struct arm_smmu_device *smmu;
--
Julien Grall