On Mon, Jan 04, 2021 at 04:16:18PM +0100, Jan Beulich wrote: > On 04.01.2021 15:56, Roger Pau Monné wrote: > > On Mon, Jan 04, 2021 at 02:43:52PM +0100, Jan Beulich wrote: > >> On 28.12.2020 11:54, Roger Pau Monné wrote: > >>> On Mon, Nov 09, 2020 at 11:54:09AM +0100, Jan Beulich wrote: > >>>> Now that the IOMMU for guests can't be enabled "on demand" anymore, > >>>> there's also no reason to expose the related CPUID bit "just in case". > >>>> > >>>> Signed-off-by: Jan Beulich <jbeul...@suse.com> > >>> > >>> I'm not sure this is helpful from a guest PoV. > >>> > >>> How does the guest know whether it has pass through devices, and thus > >>> whether it needs to check if this flag is present or not in order to > >>> safely pass foreign mapped pages (or grants) to the underlying devices? > >>> > >>> Ie: prior to this change I would just check whether the flag is > >>> present in CPUID to know whether FreeBSD needs to use a bounce buffer > >>> in blkback and netback when running as a domU. If this is now > >>> conditionally set only when the IOMMU is enabled for the guest I > >>> also need to figure a way to know whether the domU has any passed > >>> through device or not, which doesn't seem trivial. > >> > >> I'm afraid I don't understand your concern and/or description of > >> the scenario. Prior to the change, the bit was set unconditionally. > >> To me, _that_ was making the bit useless - no point in checking > >> something which is always set anyway (leaving aside old Xen > >> versions). > > > > This bit was used to differentiate between versions of Xen that don't > > create IOMMU mappings for grants/foreign maps (and so are broken) vs > > versions of Xen that do create such mappings. If the bit is not set > > HVM domains need a bounce buffer in blkback/netback in order to avoid > > sending grants to devices. > > Neither the comment in cpuid.h nor that in traps.c have any mention > of this, and the constant's name also doesn't imply such. > > > Now it's my understand that with this change sometimes the bit might > > not be set not because we are running on an unfixed Xen version, but > > because there's no IOMMU assigned to the domain, so the guest will > > fallback to use a bounce buffer. > > ... if it expects to ever map a foreign domain's pages. > > I can see that reverting the change is one way to address the issue. > Such a revert shouldn't be a plain one then, but one adjusting one > or both of the the comments to indicate the _real_ purpose of this > flag. (We probably better don't rename the constant, as we can't > easily drop the old name from the public interface anyway.)
I'm happy to send the revert, but do you have any suggestion about the fixed comments? Maybe adding something like: /* * Unditionally set the flag to notice this version of Xen has been * fixed to create IOMMU mappings for grant/foreign maps. */ Thanks, Roger.
