[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/CPUID: suppress IOMMU related hypervisor leaf data


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 4 Jan 2021 16:45:07 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=lB4bCy0KMXd7RZxUzJMgrHfH3K7xnnmRyz5WRtVWjkY=; b=W60oZLuzS+JXaEpaKRDoTGzgjKh7K516Hlhin6GS68nVA7Z7K2M8lcKu/wH2N3w0THduw+24cbMebrRaVzxKOlWA4qIvjfAa76Hy4yNt57UgI2eA2RFEuqU/Lpp8ZMGjurZ78TR1DNnieuSPjTA1P7CAg6m/2V5CT64PVYa+m+ib01l7mN0Cj5Z+v4Dk7j4qW/nFPcqy0q/UbbPo2DsxUEyVhX6uIZxC2Xb/s18CZ7PNwtg8h6lqlRCStQso1P2TS3TB+yUbGAXvOBnwdZH6r48tuRxvNrFFpRUzKWkdO/Ue4IgahpRgBEWtvezllmd8zq1x0rRQE+oZqNFEMjsrtg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Lr1uyjLxYV6k4jsBk2hPyABPnkywWgpwAfbHZ4QxFewHhBu2ZdY8MrsvPSPQRi3jjuMy8uVoD+BTeYfevTFgkgIrAUt/ANVu5ztL+nd33f6I2wZKKpWBODY6eQ7PO1VRQM1Jc8vqaaUNWwdyn7z/w0jxRNklzU/Lr5S0JC4YDaJSDNFlmFmonGXeOtWwTr5s0DJrxnMT1VhBpuKCdM+AB0hlUByQPBvCjH6Qta8WmE+JaOKmubSRNtZVOT70AdiXa4HYIpiXeSdOwfV/Q0HPsSOqAX7ZJ0e2x9EzUKMoxDje6YZ2YDwf8dDWs+jGnZbAr45+cbOWvRtbEuOSdOO/Pw==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Andrew Cooper" <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Mon, 04 Jan 2021 15:45:20 +0000
  • Ironport-sdr: 8Cd7XwNPsJKHVpjICspoIBJLSyaEsajfqHVsWPb0EH4T0ACMfma1AnRQ7IkRhKZianwgJOtLOj 2EfNDkv1b3DjIDt8xCKHwxXa40ZYXP/UhEvpKw6LsiWlbz+/KEG+cz9OuGnA4scu3EwbK34dZx Qlr4tOcpunpg9T/4MZ262XI+kyw/qK0sfTmOV90ANvDzfM6qlcOi33uSC/sfvG5hM+1QOQL2tH c6jCVgt6jQwJ9WwQxunBMSDXojSztZqcCZRSf84oQEU3Jd0BJuN1KwKTKdbPhmDYLu9VNoZB6q 0OU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>
> >>>
> >>> 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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.