Re: [PATCH v4 1/3] arm,smmu: switch to using iommu_fwspec functions

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 17/07/2021 00:59, Stefano Stabellini wrote:
> diff --git a/xen/drivers/passthrough/device_tree.c 
> b/xen/drivers/passthrough/device_tree.c
> index 999b831d90..911f82a561 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -140,8 +140,12 @@ int iommu_add_dt_device(struct dt_device_node *np)
>       if ( !ops )
>           return -EINVAL;
>   
> +    /*
> +     * Device already added. It can happen when both iommus and
> +     * mmu-masters are present.
> +     */
This is common code. So I would suggest to write a generic comment to 
avoid any misunderstanding. The one added just after the call in 
iommu_do_domctl() would seem more suitable.
>       if ( dev_iommu_fwspec_get(dev) )
> -        return -EEXIST;
> +        return 0;
There are a few things to mention here:

 1) The change is not explained in the commit message
 2) One of the caller was checking -EEXIST. As you dropped the only 
place where -EEXIST should be returned, can you drop the check in the 
caller?
Ideally this should be in a separate patch because this change is not 
entirely related to this patch.
Cheers,

--
Julien Grall