Re: [Xen-devel] [PATCH v6 07/10] use is_iommu_enabled() where appropriate...

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.08.2019 19:19, Paul Durrant wrote:
> ...rather than testing the global iommu_enabled flag and ops pointer.
> 
> Now that there is a per-domain flag indicating whether the domain is
> permitted to use the IOMMU (which determines whether the ops pointer will
> be set), many tests of the global iommu_enabled flag and ops pointer can
> be translated into tests of the per-domain flag. Some of the other tests of
> purely the global iommu_enabled flag can also be translated into tests of
> the per-domain flag.
> 
> NOTE: The comment in iommu_share_p2m_table() is also fixed; need_iommu()
>       disappeared some time ago. Also, whilst the style of the 'if' in
>       flask_iommu_resource_use_perm() is fixed, I have not translated any
>       instances of u32 into uint32_t to keep consistency. IMO such a
>       translation would be better done globally for the source module in
>       a separate patch.
>       The change in the initialization of the 'hd' variable in iommu_map()
>       and iommu_unmap() is done to keep the PV shim build happy. Without
>       this change it will fail to compile with errors of the form:
> 
> iommu.c:361:32: error: unused variable ‘hd’ [-Werror=unused-variable]
>      const struct domain_iommu *hd = dom_iommu(d);
>                                      ^~

Why would that be? Afaict there's no short-circuiting of
is_iommu_enabled() (albeit there could be, as we don't mean the shim
to have/use an IOMMU). Oh - I guess I see it: Instead of this change
what I think we want is for x86's iommu_call() to evaluate its 1st
argument. Otherwise we're liable to run into the same issue elsewhere
again.

> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -886,7 +886,7 @@ static int flask_map_domain_msi (struct domain *d, int 
> irq, const void *data,
>  #endif
>  }
>  
> -static u32 flask_iommu_resource_use_perm(void)
> +static u32 flask_iommu_resource_use_perm(struct domain *d)

const?

With these adjustments
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel