Re: [PATCH] x86/CPUID: unconditionally set XEN_HVM_CPUID_IOMMU_MAPPINGS

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2021 16:01, Roger Pau Monne wrote:
> This is a revert of f5cfa0985673 plus a rework of the comment that
> accompanies the setting of the flag so we don't forget why it needs to
> be unconditionally set: it's indicating whether the version of Xen has
> the original issue fixed and IOMMU entries are created for
> grant/foreign maps.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Despite my earlier ack I came to think that the description and
comment still don't make it quite clear what was actually wrong
with the prior change, and hence they also don't really guard
against the same getting done again (perhaps even by me). May I
ask that you add a paragraph above and ...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1049,11 +1049,10 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
> uint32_t leaf,
>              res->a |= XEN_HVM_CPUID_X2APIC_VIRT;
>  
>          /*
> -         * Indicate that memory mapped from other domains (either grants or
> -         * foreign pages) has valid IOMMU entries.
> +         * Unconditionally set the flag to indicate this version of Xen has
> +         * been fixed to create IOMMU mappings for grant/foreign maps.
>           */
> -        if ( is_iommu_enabled(d) )
> -            res->a |= XEN_HVM_CPUID_IOMMU_MAPPINGS;
> +        res->a |= XEN_HVM_CPUID_IOMMU_MAPPINGS;

... try to clarify the "Unconditionally" here?

Jan