[Xen-devel] [PATCH] x86/pv: Fix `global-pages` to match the documentation

Andrew Cooper posted 1 patch 4 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20191216140227.19234-1-andrew.cooper3@citrix.com
docs/misc/xen-command-line.pandoc | 24 +++++++++++++++---------
xen/arch/x86/pv/domain.c          |  4 ++--
2 files changed, 17 insertions(+), 11 deletions(-)
[Xen-devel] [PATCH] x86/pv: Fix `global-pages` to match the documentation
Posted by Andrew Cooper 4 years, 4 months ago
c/s 5de961d9c09 "x86: do not enable global pages when virtualized on AMD or
Hygon hardware" in fact does.  Fix the calculation in pge_init().

While fixing this, adjust the command line documenation, first to use the
newer style, and to expand the description to discuss cases where the option
might be useful to use, but Xen can't account for by default.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 docs/misc/xen-command-line.pandoc | 24 +++++++++++++++---------
 xen/arch/x86/pv/domain.c          |  4 ++--
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 7a1be84ca9..cb54a000fc 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1087,18 +1087,24 @@ value settable via Xen tools.
 
 Dom0 is using this value for sizing its maptrack table.
 
-### global-pages (x86)
-> `= <boolean>`
+### global-pages
+    = <boolean>
+
+    Applicability: x86
+    Default: true unless running virtualized on AMD or Hygon hardware
 
-> Default: `true` unless running virtualized on AMD or Hygon hardware
+Control whether to use global pages for PV guests, and thus the need to
+perform TLB flushes by writing to CR4.  This is a performance trade-off.
 
-Set whether the PGE bit in CR4 will be enabled for PV guests. This controls the
-usage of global pages, and thus the need to perform tlb flushes by writing to
-CR4.
+AMD SVM does not support selective trapping of CR4 writes, which means that a
+global TLB flush (two CR4 writes) takes two VMExits, and massively outweigh
+the benefit of using global pages to begin with.  This case is easy for Xen to
+spot, and is accounted for in the default setting.
 
-Note it's disabled by default when running virtualized on AMD or Hygon hardware
-since AMD SVM doesn't support selective trapping of CR4, so global pages are
-not enabled in order to reduce the overhead of TLB flushes.
+Other cases where this option might be a benefit is on VT-x hardware when
+selective CR4 writes are not supported/enabled by the hypervisor, or in any
+virtualised case using shadow paging.  These are not easy for Xen to spot, so
+are not accounted for in the default setting.
 
 ### guest_loglvl
 > `= <level>[/<rate-limited level>]` where level is `none | error | warning | info | debug | all`
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index e6e1c51548..ed5111fc47 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -125,8 +125,8 @@ static int __init pge_init(void)
 {
     if ( opt_global_pages == -1 )
         opt_global_pages = !cpu_has_hypervisor ||
-                           (boot_cpu_data.x86_vendor &
-                            (X86_VENDOR_AMD | X86_VENDOR_HYGON));
+                           !(boot_cpu_data.x86_vendor &
+                             (X86_VENDOR_AMD | X86_VENDOR_HYGON));
 
     return 0;
 }
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Fix `global-pages` to match the documentation
Posted by Roger Pau Monné 4 years, 4 months ago
On Mon, Dec 16, 2019 at 02:02:27PM +0000, Andrew Cooper wrote:
> c/s 5de961d9c09 "x86: do not enable global pages when virtualized on AMD or
> Hygon hardware" in fact does.  Fix the calculation in pge_init().
> 
> While fixing this, adjust the command line documenation, first to use the
> newer style, and to expand the description to discuss cases where the option
> might be useful to use, but Xen can't account for by default.
> 

Fixes: 5de961d9c09 ('x86: do not enable global pages when virtualized on AMD or Hygon hardware')

Would be helpful for backport reasons if someone picks up the other
change (or at least I've been trying to use it in order to help
downstreams that might cherry-pick stuff).

> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Fix `global-pages` to match the documentation
Posted by Jan Beulich 4 years, 4 months ago
On 16.12.2019 16:38, Roger Pau Monné wrote:
> On Mon, Dec 16, 2019 at 02:02:27PM +0000, Andrew Cooper wrote:
>> c/s 5de961d9c09 "x86: do not enable global pages when virtualized on AMD or
>> Hygon hardware" in fact does.  Fix the calculation in pge_init().
>>
>> While fixing this, adjust the command line documenation, first to use the
>> newer style, and to expand the description to discuss cases where the option
>> might be useful to use, but Xen can't account for by default.
>>
> 
> Fixes: 5de961d9c09 ('x86: do not enable global pages when virtualized on AMD or Hygon hardware')
> 
> Would be helpful for backport reasons if someone picks up the other
> change (or at least I've been trying to use it in order to help
> downstreams that might cherry-pick stuff).
> 
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel