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....@citrix.com>
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
