diff mbox

x86/pagewalk: Remove opt_allow_superpage check from guest_can_use_l2_superpages()

Message ID 1500994813-8407-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper July 25, 2017, 3 p.m. UTC
The purpose of guest_walk_tables() is to match the behaviour of real hardware.

A PV guest can have 2M superpages in its pagetables, via the M2P and the
initial initrd mapping, even if it isn't permitted to create arbitrary 2M
superpage mappings.

guest_can_use_l2_superpages() checking opt_allow_superpage is a piece of PV
guest policy enforcement, rather than its intended purpose of meaning "would
hardware tolerate finding an L2 superpage with these control settings?"

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/asm-x86/guest_pt.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Tim Deegan July 25, 2017, 3:27 p.m. UTC | #1
At 16:00 +0100 on 25 Jul (1500998413), Andrew Cooper wrote:
> The purpose of guest_walk_tables() is to match the behaviour of real hardware.
> 
> A PV guest can have 2M superpages in its pagetables, via the M2P and the
> initial initrd mapping, even if it isn't permitted to create arbitrary 2M
> superpage mappings.

Can the domain builder (or Xen?) really give a guest superpage
mappings for its initrd?  Wouldn't that cause problems for live
migration?

In any case this patch looks correct: the presence of superpages in PV
pagetables is decided by the PV MM rules, so the walker should accept them.

Reviewed-by: Tim Deegan <tim@xen.org>

> guest_can_use_l2_superpages() checking opt_allow_superpage is a piece of PV
> guest policy enforcement, rather than its intended purpose of meaning "would
> hardware tolerate finding an L2 superpage with these control settings?"
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Tim Deegan <tim@xen.org>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/include/asm-x86/guest_pt.h | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
> index 72126d5..08031c8 100644
> --- a/xen/include/asm-x86/guest_pt.h
> +++ b/xen/include/asm-x86/guest_pt.h
> @@ -205,15 +205,17 @@ static inline guest_l4e_t guest_l4e_from_gfn(gfn_t gfn, u32 flags)
>  static inline bool guest_can_use_l2_superpages(const struct vcpu *v)
>  {
>      /*
> +     * PV guests use Xen's paging settings.  Being 4-level, 2M
> +     * superpages are unconditionally supported.
> +     *
>       * The L2 _PAGE_PSE bit must be honoured in HVM guests, whenever
>       * CR4.PSE is set or the guest is in PAE or long mode.
>       * It's also used in the dummy PT for vcpus with CR0.PG cleared.
>       */
> -    return (is_pv_vcpu(v)
> -            ? opt_allow_superpage
> -            : (GUEST_PAGING_LEVELS != 2
> -               || !hvm_paging_enabled(v)
> -               || (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE)));
> +    return (is_pv_vcpu(v) ||
> +            GUEST_PAGING_LEVELS != 2 ||
> +            !hvm_paging_enabled(v) ||
> +            (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE));
>  }
>  
>  static inline bool guest_can_use_l3_superpages(const struct domain *d)
> -- 
> 2.1.4
>
Wei Liu July 25, 2017, 4:05 p.m. UTC | #2
On Tue, Jul 25, 2017 at 04:00:13PM +0100, Andrew Cooper wrote:
> The purpose of guest_walk_tables() is to match the behaviour of real hardware.
> 
> A PV guest can have 2M superpages in its pagetables, via the M2P and the
> initial initrd mapping, even if it isn't permitted to create arbitrary 2M
> superpage mappings.
> 
> guest_can_use_l2_superpages() checking opt_allow_superpage is a piece of PV
> guest policy enforcement, rather than its intended purpose of meaning "would
> hardware tolerate finding an L2 superpage with these control settings?"
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Wei Liu July 25, 2017, 4:06 p.m. UTC | #3
On Tue, Jul 25, 2017 at 04:27:00PM +0100, Tim Deegan wrote:
> At 16:00 +0100 on 25 Jul (1500998413), Andrew Cooper wrote:
> > The purpose of guest_walk_tables() is to match the behaviour of real hardware.
> > 
> > A PV guest can have 2M superpages in its pagetables, via the M2P and the
> > initial initrd mapping, even if it isn't permitted to create arbitrary 2M
> > superpage mappings.
> 
> Can the domain builder (or Xen?) really give a guest superpage
> mappings for its initrd?  Wouldn't that cause problems for live
> migration?
> 

I can see superpage mapping when constructing Dom0 (kernel or initrd,
haven't checked).
Andrew Cooper July 25, 2017, 4:12 p.m. UTC | #4
On 25/07/17 16:27, Tim Deegan wrote:
> At 16:00 +0100 on 25 Jul (1500998413), Andrew Cooper wrote:
>> The purpose of guest_walk_tables() is to match the behaviour of real hardware.
>>
>> A PV guest can have 2M superpages in its pagetables, via the M2P and the
>> initial initrd mapping, even if it isn't permitted to create arbitrary 2M
>> superpage mappings.
> Can the domain builder (or Xen?) really give a guest superpage
> mappings for its initrd?  Wouldn't that cause problems for live
> migration?

From Wei's work, dom0 definitely ends up calling put_page_type() on a L2
superpage.

Looking at the dom0 construction code more closely, it appears that Xen
creates the initial p2m in the guest using superpages.

I will s/initrd/initial p2m/ in the commit message.  I don't believe the
domain builder has any way of creating such mappings for a domU, so the
migration aspect is less important.

>
> In any case this patch looks correct: the presence of superpages in PV
> pagetables is decided by the PV MM rules, so the walker should accept them.
>
> Reviewed-by: Tim Deegan <tim@xen.org>

Thanks.
diff mbox

Patch

diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
index 72126d5..08031c8 100644
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -205,15 +205,17 @@  static inline guest_l4e_t guest_l4e_from_gfn(gfn_t gfn, u32 flags)
 static inline bool guest_can_use_l2_superpages(const struct vcpu *v)
 {
     /*
+     * PV guests use Xen's paging settings.  Being 4-level, 2M
+     * superpages are unconditionally supported.
+     *
      * The L2 _PAGE_PSE bit must be honoured in HVM guests, whenever
      * CR4.PSE is set or the guest is in PAE or long mode.
      * It's also used in the dummy PT for vcpus with CR0.PG cleared.
      */
-    return (is_pv_vcpu(v)
-            ? opt_allow_superpage
-            : (GUEST_PAGING_LEVELS != 2
-               || !hvm_paging_enabled(v)
-               || (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE)));
+    return (is_pv_vcpu(v) ||
+            GUEST_PAGING_LEVELS != 2 ||
+            !hvm_paging_enabled(v) ||
+            (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE));
 }
 
 static inline bool guest_can_use_l3_superpages(const struct domain *d)