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

RE: [PATCH v6 02/12] IOMMU/x86: new command line option to suppress use of superpage mappings


  • To: Pau Monné, Roger <roger.pau@xxxxxxxxxx>, "Beulich, Jan" <JBeulich@xxxxxxxx>
  • From: "Tian, Kevin" <kevin.tian@xxxxxxxxx>
  • Date: Wed, 29 Jun 2022 08:56:02 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=CPmbE+BRc69ZfHF3vhM7wH1H0i6SmKhoV5Y0PgygeC8=; b=CH6hVjzbeCaZvZsPcYqWA9Zrtcn1AQkBD5epSjZJ/tuLiXb8r09OPgwEH76oWcjxXaF2y6F0mHzhXpVNR3RAMwMfPHssLE2Qqj4PnBt3CXxYZ5cxB/3hQ8IupZBm98m+poMQ7rAyFU8Pl/b9DyndaiIscnFo7KfaFfv3CTJD+WN1FTiyiGu1ZyrOFnY/Ay99k13yUBC3CBC2sMiUbVK/5ZJJKgOB1iRksMwUp4nzk7oFdDyFsIdLcTxVrBboI/QJblCAwK/pgaCnhvYRGoGpmHp6OQXFqziRZus0wRWVgsk1vcyZbZXqX30a4FHbiPHbXjy2Dqoafhhw9v2iN4mQRQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=F3TY7PdeoLzgBjg80lojYIUgOnRDYus7XvXMmz1aw4Y3pCphLaGJ/qUlQDcgydTb+UcTAOL5Jc82vnO5H+N2TyU+ViXmqsmQl8C16HRH/eScJSVm/e3uEuihiYwQhkXbPvw0KE6I1DE9AxOWTJXRSbuqgfu1fwPZey/b5AoWrS0SaFGRMQBjLFKb86UzOd7EI4BnuMHMLz5I98hNPM88DdAdJMxXAbWY2iLmGAr/UsXyW3HdjoPVbXzdArmQUc6EWHyF2yX1Vc89HnpXWgYyTgNpDN08PVoNW+Ght1QIyklqyLuJ2sP7sUktY//iMMVLVRzguwWml0NzbmmYVyDQGg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "Cooper, Andrew" <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Wed, 29 Jun 2022 08:56:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYe+oq9bL7IByuf0q0gcGGaDvoUK1k4/GAgAFO9LA=
  • Thread-topic: [PATCH v6 02/12] IOMMU/x86: new command line option to suppress use of superpage mappings

> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Sent: Tuesday, June 28, 2022 8:52 PM
> 
> On Thu, Jun 09, 2022 at 12:17:23PM +0200, Jan Beulich wrote:
> > Before actually enabling their use, provide a means to suppress it in
> > case of problems. Note that using the option can also affect the sharing
> > of page tables in the VT-d / EPT combination: If EPT would use large
> > page mappings but the option is in effect, page table sharing would be
> > suppressed (to properly fulfill the admin request).
> >
> > Requested-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > v6: New.
> >
> > --- a/docs/misc/xen-command-line.pandoc
> > +++ b/docs/misc/xen-command-line.pandoc
> > @@ -1405,7 +1405,7 @@ detection of systems known to misbehave
> >
> >  ### iommu
> >      = List of [ <bool>, verbose, debug, force, required, 
> > quarantine[=scratch-
> page],
> > -                sharept, intremap, intpost, crash-disable,
> > +                sharept, superpages, intremap, intpost, crash-disable,
> >                  snoop, qinval, igfx, amd-iommu-perdev-intremap,
> >                  dom0-{passthrough,strict} ]
> >
> > @@ -1481,6 +1481,12 @@ boolean (e.g. `iommu=no`) can override t
> >
> >      This option is ignored on ARM, and the pagetables are always shared.
> >
> > +*   The `superpages` boolean controls whether superpage mappings may
> be used
> > +    in IOMMU page tables.  If using this option is necessary to fix an 
> > issue,
> > +    please report a bug.
> > +
> > +    This option is only valid on x86.
> > +
> >  *   The `intremap` boolean controls the Interrupt Remapping sub-feature,
> and
> >      is active by default on compatible hardware.  On x86 systems, the first
> >      generation of IOMMUs only supported DMA remapping, and Interrupt
> Remapping
> > --- a/xen/arch/x86/include/asm/iommu.h
> > +++ b/xen/arch/x86/include/asm/iommu.h
> > @@ -132,7 +132,7 @@ extern bool untrusted_msi;
> >  int pi_update_irte(const struct pi_desc *pi_desc, const struct pirq *pirq,
> >                     const uint8_t gvec);
> >
> > -extern bool iommu_non_coherent;
> > +extern bool iommu_non_coherent, iommu_superpages;
> >
> >  static inline void iommu_sync_cache(const void *addr, unsigned int size)
> >  {
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -88,6 +88,8 @@ static int __init cf_check parse_iommu_p
> >              iommu_igfx = val;
> >          else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
> >              iommu_qinval = val;
> > +        else if ( (val = parse_boolean("superpages", s, ss)) >= 0 )
> > +            iommu_superpages = val;
> >  #endif
> >          else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
> >              iommu_verbose = val;
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -2213,7 +2213,8 @@ static bool __init vtd_ept_page_compatib
> >      if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 )
> >          return false;
> >
> > -    return (ept_has_2mb(ept_cap) && opt_hap_2mb) <=
> cap_sps_2mb(vtd_cap) &&
> > +    return iommu_superpages &&
> > +           (ept_has_2mb(ept_cap) && opt_hap_2mb) <=
> cap_sps_2mb(vtd_cap) &&
> >             (ept_has_1gb(ept_cap) && opt_hap_1gb) <= cap_sps_1gb(vtd_cap);
> 
> Isn't this too strict in requesting iommu superpages to be enabled
> regardless of whether EPT has superpage support?
> 
> Shouldn't this instead be:
> 
> return iommu_superpages ? ((ept_has_2mb(ept_cap) && opt_hap_2mb) <=
> cap_sps_2mb(vtd_cap) &&
>                            (ept_has_1gb(ept_cap) && opt_hap_1gb) <=
> cap_sps_1gb(vtd_cap))
>                         : !((ept_has_2mb(ept_cap) && opt_hap_2mb) ||
>                             (ept_has_1gb(ept_cap) && opt_hap_1gb));
> 
> I think we want to introduce some local variables to store EPT
> superpage availability, as the lines are too long.
> 

Or to be pair with ept side checks:

    return (ept_has_2mb(ept_cap) && opt_hap_2mb) <=
           (cap_sps_2mb(vtd_cap) && iommu_superpages) &&
           (ept_has_1gb(ept_cap) && opt_hap_1gb) <=
           (cap_sps_1gb(vtd_cap) && iommu_superpages);

 


Rackspace

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