Re: [Xen-devel] [PATCH v8 6/6] introduce a 'passthrough' configuration option to xl.cfg...

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 9/2/19 3:50 PM, Paul Durrant wrote:
> @@ -263,9 +263,17 @@ struct domain_iommu {
>       DECLARE_BITMAP(features, IOMMU_FEAT_count);
>   
>       /*
> -     * Does the guest reqire mappings to be synchonized, to maintain
> -     * the default dfn == pfn map. (See comment on dfn at the top of
> -     * include/xen/mm.h).
> +     * Does the guest share HAP mapping with the IOMMU? This is always
> +     * true for ARM systems and may be true for x86 systems where the
> +     * the hardware is capable.
> +     */
I am worried that such comment may rot over the time. For instance, if 
we either add a new architecture or decide to stop sharing PT on Arm.
I vaguely recall some potential issues with the MSI doorbells that would 
require us to unshare the PT when they will be supported in guest.
I would suggest to either refers to the implementation of 
iommu_use_hap_pt() or drop completely the second sentence.
> +    bool hap_pt_share;
> +
> +    /*
> +     * Does the guest require mappings to be synchronized, to maintain
> +     * the default dfn == pfn map? (See comment on dfn at the top of
> +     * include/xen/mm.h). Note that hap_pt_share == false does not
> +     * necessarily imply this is true.
>        */
>       bool need_sync;
>   };
> @@ -275,8 +283,7 @@ struct domain_iommu {
>   #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)
> +#define iommu_use_hap_pt(d)       (dom_iommu(d)->hap_pt_share)
>   
>   /* Does the IOMMU pagetable need to be kept synchronized with the P2M */
>   #ifdef CONFIG_HAS_PASSTHROUGH
Cheers,

--
Julien Grall

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