[PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

Commit 7113a45451a9f656deeff070e47672043ed83664 (kexec/x86: do not map
crash kernel area) causes fatal page faults when loading a crash
image.  The attempt to zero the first control page allocated from the
crash region will fault as the VA return by map_domain_page() has no
mapping.

The fault will occur on non-debug builds of Xen when the crash area is
below 5 TiB (which will be most systems).

The assumption that the crash area mapping was not used is incorrect.
map_domain_page() is used when loading an image and building the
image's page tables to temporarily map the crash area, thus the
mapping is required if the crash area is in the direct map area.

Reintroduce the mapping, but only the portions of the crash area that
are within the direct map area.

Reported-by: Don Slutz <dslutz@verizon.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Daniel Kiper <daniel.kiper@oracle.com>
---
This fixes a Xen crash so is an important fix for the 4.4 release..

Changes in v2:
- merge patches into one
- add check for e > s before mapping
---
 xen/arch/x86/setup.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4833ca3..b49256d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1098,6 +1098,17 @@ void __init __start_xen(unsigned long mbi_p)
                          PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
     }
 
+    if ( kexec_crash_area.size )
+    {
+        unsigned long s = PFN_DOWN(kexec_crash_area.start);
+        unsigned long e = min(s + PFN_UP(kexec_crash_area.size),
+                              PFN_UP(__pa(HYPERVISOR_VIRT_END - 1)));
+
+        if ( e > s ) 
+            map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
+                             s, e - s, PAGE_HYPERVISOR);
+    }
+
     xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) &
                    ~((1UL << L2_PAGETABLE_SHIFT) - 1);
     destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE);
-- 
1.7.2.5

Re: [PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[Daniel Kiper]

On Wed, Jan 08, 2014 at 06:35:19PM +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> Commit 7113a45451a9f656deeff070e47672043ed83664 (kexec/x86: do not map
> crash kernel area) causes fatal page faults when loading a crash
> image.  The attempt to zero the first control page allocated from the
> crash region will fault as the VA return by map_domain_page() has no
> mapping.
>
> The fault will occur on non-debug builds of Xen when the crash area is
> below 5 TiB (which will be most systems).
>
> The assumption that the crash area mapping was not used is incorrect.
> map_domain_page() is used when loading an image and building the
> image's page tables to temporarily map the crash area, thus the
> mapping is required if the crash area is in the direct map area.
>
> Reintroduce the mapping, but only the portions of the crash area that
> are within the direct map area.
>
> Reported-by: Don Slutz <dslutz@verizon.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> This fixes a Xen crash so is an important fix for the 4.4 release..

Thanks. It looks quite good for me but I would like to do some tests.
I will send you results by the end of this week.

Daniel

Re: [PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[Don Slutz]

On 01/08/14 13:35, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> Commit 7113a45451a9f656deeff070e47672043ed83664 (kexec/x86: do not map
> crash kernel area) causes fatal page faults when loading a crash
> image.  The attempt to zero the first control page allocated from the
> crash region will fault as the VA return by map_domain_page() has no
> mapping.
>
> The fault will occur on non-debug builds of Xen when the crash area is
> below 5 TiB (which will be most systems).
>
> The assumption that the crash area mapping was not used is incorrect.
> map_domain_page() is used when loading an image and building the
> image's page tables to temporarily map the crash area, thus the
> mapping is required if the crash area is in the direct map area.
>
> Reintroduce the mapping, but only the portions of the crash area that
> are within the direct map area.
>
> Reported-by: Don Slutz <dslutz@verizon.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> This fixes a Xen crash so is an important fix for the 4.4 release..
>
> Changes in v2:
> - merge patches into one
> - add check for e > s before mapping
> ---
>   xen/arch/x86/setup.c |   11 +++++++++++
>   1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 4833ca3..b49256d 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1098,6 +1098,17 @@ void __init __start_xen(unsigned long mbi_p)
>                            PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
>       }
>   
> +    if ( kexec_crash_area.size )
> +    {
> +        unsigned long s = PFN_DOWN(kexec_crash_area.start);
> +        unsigned long e = min(s + PFN_UP(kexec_crash_area.size),
> +                              PFN_UP(__pa(HYPERVISOR_VIRT_END - 1)));
> +
> +        if ( e > s )
> +            map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
> +                             s, e - s, PAGE_HYPERVISOR);
> +    }
> +
>       xen_virt_end = ((unsigned long)_end + (1UL << L2_PAGETABLE_SHIFT) - 1) &
>                      ~((1UL << L2_PAGETABLE_SHIFT) - 1);
>       destroy_xen_mappings(xen_virt_end, XEN_VIRT_START + BOOTSTRAP_MAP_BASE);


4.4.0-rc1 + this patch works for me.  So you can add:

Tested-by: Don Slutz <dslutz@verizon.com>

    -Don Slutz

Re: [PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[Daniel Kiper]

On Wed, Jan 08, 2014 at 06:35:19PM +0000, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
>
> Commit 7113a45451a9f656deeff070e47672043ed83664 (kexec/x86: do not map
> crash kernel area) causes fatal page faults when loading a crash
> image.  The attempt to zero the first control page allocated from the
> crash region will fault as the VA return by map_domain_page() has no
> mapping.
>
> The fault will occur on non-debug builds of Xen when the crash area is
> below 5 TiB (which will be most systems).
>
> The assumption that the crash area mapping was not used is incorrect.
> map_domain_page() is used when loading an image and building the
> image's page tables to temporarily map the crash area, thus the
> mapping is required if the crash area is in the direct map area.
>
> Reintroduce the mapping, but only the portions of the crash area that
> are within the direct map area.
>
> Reported-by: Don Slutz <dslutz@verizon.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> This fixes a Xen crash so is an important fix for the 4.4 release..

This really fixes page fault found by Don. After deeper testing I can confirm
that issue introduced by commit 7113a45451a9f656deeff070e47672043ed83664
is revealed only when Xen is compiled with debug option disabled.

By the way, why map_domain_page() behavior depends on debug option?
It is not nice because we could be trapped by this in the future in
more serious places. Could map_domain_page() work in the same way
with or without debug option?

Anyway...

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Tested-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[Andrew Cooper]

On 09/01/2014 19:50, Daniel Kiper wrote:
> By the way, why map_domain_page() behavior depends on debug option?
> It is not nice because we could be trapped by this in the future in
> more serious places. Could map_domain_page() work in the same way
> with or without debug option?

With a debug build of Xen, map_domain_page() always mutates the
pagetables and hands out virtual addresses from the mapcache region. 
This is to test map_domain_page() itself, as well as making domain
mapping leaks more obvious (as the mapcache is under heavier load).

For a non-debug build of Xen, any map_domain_page() calls which can be
satisfied by returning a virtual address from the direct map region
(i.e. for pages below the 5TiB boundary which is basically all of them
unless you have more money than sense) are, which avoided excessive use
of the mapcache, and avoids a TLB shootdown/flush on unmap.

~Andrew

Re: [PATCHv2] x86: map portion of kexec crash area that is within the direct map area

[Daniel Kiper]

On Thu, Jan 09, 2014 at 11:15:04PM +0000, Andrew Cooper wrote:
> On 09/01/2014 19:50, Daniel Kiper wrote:
> > By the way, why map_domain_page() behavior depends on debug option?
> > It is not nice because we could be trapped by this in the future in
> > more serious places. Could map_domain_page() work in the same way
> > with or without debug option?
>
> With a debug build of Xen, map_domain_page() always mutates the
> pagetables and hands out virtual addresses from the mapcache region.
> This is to test map_domain_page() itself, as well as making domain
> mapping leaks more obvious (as the mapcache is under heavier load).
>
> For a non-debug build of Xen, any map_domain_page() calls which can be
> satisfied by returning a virtual address from the direct map region
> (i.e. for pages below the 5TiB boundary which is basically all of them
> unless you have more money than sense) are, which avoided excessive use
> of the mapcache, and avoids a TLB shootdown/flush on unmap.

Make sens. Thanks for explanation.

Daniel