Re: [Xen-devel] [PATCH v6 09/10] iommu: tidy up iommu_use_hap_pt() and need_iommu_pt_sync() macros

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.08.2019 19:20, Paul Durrant wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -102,8 +102,10 @@ static int __init parse_iommu_param(const char *s)
>              iommu_hwdom_passthrough = val;
>          else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
>              iommu_hwdom_strict = val;
> +#ifndef CONFIG_ARM
>          else if ( (val = parse_boolean("sharept", s, ss)) >= 0 )
>              iommu_hap_pt_share = val;
> +#endif
>          else
>              rc = -EINVAL;

I think you/we should go further here: Arm should #define this to
true, and here we should have "#ifndef iommu_hap_pt_share".

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -268,6 +268,17 @@ struct domain_iommu {
>  #define iommu_set_feature(d, f)   set_bit(f, dom_iommu(d)->features)
>  #define iommu_clear_feature(d, f) clear_bit(f, dom_iommu(d)->features)
>  
> +/* Are we using the domain P2M table as its IOMMU pagetable? */
> +#define iommu_use_hap_pt(d) \
> +    (hap_enabled(d) && is_iommu_enabled(d) && iommu_hap_pt_share)
> +
> +/* Does the IOMMU pagetable need to be kept synchronized with the P2M */
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +#define need_iommu_pt_sync(d)     (dom_iommu(d)->need_sync)
> +#else
> +#define need_iommu_pt_sync(d)     false

I think you'd better evaluate d here; one (somewhat in risk of
opposition) variant would be

#define need_iommu_pt_sync(d)     (!(d))

Jan

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