[PATCHv5 0/2] x86/xen: fix dom0 boot with tboot

[PATCHv5 0/2] x86/xen: fix dom0 boot with tboot

[David Vrabel]

The first patch (for 3.11 and which should be considered for stable),
fixes boot failures caused by UNUSABLE regions in the e820 machine
memory map.  This typically occurs if tboot is used.

The second patch (for 3.12) is clean up some an inadvertant change to
the mappings setup during early init, as introduced in 3.5.  There is
no need for this to be backported though.

Changes in v5:
- Use the original approach of removing UNUSABLE regions as the
  previous fix did not work when backported to older kernels.

Changes in v4:
- Extend 1:1 mapping region to cover 0 - 1MiB. find_ibft_region()
  scans from 512 KiB and if this overlapped with a reserved region it
  would crash.

Changes in v3:
- Avoid the 1:1 mapping (except for the ISA region) by the early setup
  instead of removing UNUSABLE regions.

Changes in v2:
- Refactor getting the correct memory map.

David

[PATCH 1/2] x86/xen: do not identity map UNUSABLE regions in the machine E820

[David Vrabel]

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

If there are UNUSABLE regions in the machine memory map, dom0 will
attempt to map them 1:1 which is not permitted by Xen and the kernel
will crash.

There isn't anything interesting in the UNUSABLE region that the dom0
kernel needs access to so we can avoid making the 1:1 mapping and
treat it as RAM.

We only do this for dom0, as we do not expect any domU to ever have a
UNUSABLE region in their pseudo-physical map.

This fixes a boot failure on hosts with tboot.

tboot marks a region in the e820 map as unusable and the dom0 kernel
would attempt to map this region and Xen does not permit unusable
regions to be mapped by guests.

  (XEN)  0000000000000000 - 0000000000060000 (usable)
  (XEN)  0000000000060000 - 0000000000068000 (reserved)
  (XEN)  0000000000068000 - 000000000009e000 (usable)
  (XEN)  0000000000100000 - 0000000000800000 (usable)
  (XEN)  0000000000800000 - 0000000000972000 (unusable)

tboot marked this region as unusable.

  (XEN)  0000000000972000 - 00000000cf200000 (usable)
  (XEN)  00000000cf200000 - 00000000cf38f000 (reserved)
  (XEN)  00000000cf38f000 - 00000000cf3ce000 (ACPI data)
  (XEN)  00000000cf3ce000 - 00000000d0000000 (reserved)
  (XEN)  00000000e0000000 - 00000000f0000000 (reserved)
  (XEN)  00000000fe000000 - 0000000100000000 (reserved)
  (XEN)  0000000100000000 - 0000000630000000 (usable)

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/setup.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 056d11f..5a093b7 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -313,6 +313,17 @@ static void xen_align_and_add_e820_region(u64 start, u64 size, int type)
 	e820_add_region(start, end - start, type);
 }
 
+void xen_ignore_unusable(struct e820entry *list, size_t map_size)
+{
+	struct e820entry *entry;
+	unsigned int i;
+
+	for (i = 0, entry = list; i < map_size; i++, entry++) {
+		if (entry->type == E820_UNUSABLE)
+			entry->type = E820_RAM;
+	}
+}
+
 /**
  * machine_specific_memory_setup - Hook for machine specific memory setup.
  **/
@@ -353,6 +364,16 @@ char * __init xen_memory_setup(void)
 	}
 	BUG_ON(rc);
 
+	/*
+	 * Xen won't allow a 1:1 mapping to be created to UNUSABLE
+	 * regions, so if we're using the machine memory map leave the
+	 * region as RAM as it is in the pseudo-physical map.
+	 *
+	 * UNUSABLE regions are not expected in domUs.
+	 */
+	if (xen_initial_domain())
+		xen_ignore_unusable(map, memmap.nr_entries);
+
 	/* Make sure the Xen-supplied memory map is well-ordered. */
 	sanitize_e820_map(map, memmap.nr_entries, &memmap.nr_entries);
 
-- 
1.7.2.5

[PATCH 2/2] x86/xen: during early setup, only 1:1 map the ISA region

[David Vrabel]

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

During early setup, when the reserved regions and MMIO holes are being
setup as 1:1 in the p2m, clear any mappings instead of making them 1:1
(execept for the ISA region which is expected to be mapped).

This reverts a change in behaviour introduced in 3.5 by 83d51ab473dd
(xen/setup: update VA mapping when releasing memory during setup).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/setup.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 5a093b7..081292e 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -215,13 +215,19 @@ static void __init xen_set_identity_and_release_chunk(
 	unsigned long pfn;
 
 	/*
-	 * If the PFNs are currently mapped, the VA mapping also needs
-	 * to be updated to be 1:1.
+	 * If the PFNs are currently mapped, clear the mappings
+	 * (except for the ISA region which must be 1:1 mapped) to
+	 * release the refcounts (in Xen) on the original frames.
 	 */
-	for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++)
+	for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++) {
+		pte_t pte = __pte_ma(0);
+
+		if (pfn < PFN_UP(ISA_END_ADDRESS))
+			pte = mfn_pte(pfn, PAGE_KERNEL_IO);
+
 		(void)HYPERVISOR_update_va_mapping(
-			(unsigned long)__va(pfn << PAGE_SHIFT),
-			mfn_pte(pfn, PAGE_KERNEL_IO), 0);
+			(unsigned long)__va(pfn << PAGE_SHIFT), pte, 0);
+	}
 
 	if (start_pfn < nr_pages)
 		*released += xen_release_chunk(
-- 
1.7.2.5

Re: [PATCHv5 0/2] x86/xen: fix dom0 boot with tboot

[David Vrabel]

On 16/08/13 15:42, David Vrabel wrote:
> The first patch (for 3.11 and which should be considered for stable),
> fixes boot failures caused by UNUSABLE regions in the e820 machine
> memory map.  This typically occurs if tboot is used.
> 
> The second patch (for 3.12) is clean up some an inadvertant change to
> the mappings setup during early init, as introduced in 3.5.  There is
> no need for this to be backported though.

How odd. The copy in my inbox has Cc Konrad, but the copy in the list
does not.

David

Re: [PATCHv5 0/2] x86/xen: fix dom0 boot with tboot

[Ian Campbell]

On Fri, 2013-08-16 at 16:04 +0100, David Vrabel wrote:
> On 16/08/13 15:42, David Vrabel wrote:
> > The first patch (for 3.11 and which should be considered for stable),
> > fixes boot failures caused by UNUSABLE regions in the e820 machine
> > memory map.  This typically occurs if tboot is used.
> > 
> > The second patch (for 3.12) is clean up some an inadvertant change to
> > the mappings setup during early init, as introduced in 3.5.  There is
> > no need for this to be backported though.
> 
> How odd. The copy in my inbox has Cc Konrad, but the copy in the list
> does not.

There's a weird mailman "feature" where you can ask not to get copies of
list posts where you are CC and it will even go as far as to strip cc's
on list posts. Mad I know but there you are.

Ian.

Re: [PATCH 1/2] x86/xen: do not identity map UNUSABLE regions in the machine E820

[Konrad Rzeszutek Wilk]

On Fri, Aug 16, 2013 at 03:42:55PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> If there are UNUSABLE regions in the machine memory map, dom0 will
> attempt to map them 1:1 which is not permitted by Xen and the kernel
> will crash.
> 
> There isn't anything interesting in the UNUSABLE region that the dom0
> kernel needs access to so we can avoid making the 1:1 mapping and
> treat it as RAM.
> 
> We only do this for dom0, as we do not expect any domU to ever have a
> UNUSABLE region in their pseudo-physical map.

Hm, you are going to be disappointed that this is in libxl:

        /* Lastly, convert the RAM to UNSUABLE. Look in the Linux kernel        
           at git commit 2f14ddc3a7146ea4cd5a3d1ecd993f85f2e4f948               
            "xen/setup: Inhibit resource API from using System RAM E820         
           gaps as PCI mem gaps" for full explanation. */                       
        if (end > ram_end)                                                      
            src[i].type = E820_UNUSABLE;                                        
    }                                           
> 
> This fixes a boot failure on hosts with tboot.
> 
> tboot marks a region in the e820 map as unusable and the dom0 kernel
> would attempt to map this region and Xen does not permit unusable
> regions to be mapped by guests.
> 
>   (XEN)  0000000000000000 - 0000000000060000 (usable)
>   (XEN)  0000000000060000 - 0000000000068000 (reserved)
>   (XEN)  0000000000068000 - 000000000009e000 (usable)
>   (XEN)  0000000000100000 - 0000000000800000 (usable)
>   (XEN)  0000000000800000 - 0000000000972000 (unusable)
> 
> tboot marked this region as unusable.
> 
>   (XEN)  0000000000972000 - 00000000cf200000 (usable)
>   (XEN)  00000000cf200000 - 00000000cf38f000 (reserved)
>   (XEN)  00000000cf38f000 - 00000000cf3ce000 (ACPI data)
>   (XEN)  00000000cf3ce000 - 00000000d0000000 (reserved)
>   (XEN)  00000000e0000000 - 00000000f0000000 (reserved)
>   (XEN)  00000000fe000000 - 0000000100000000 (reserved)
>   (XEN)  0000000100000000 - 0000000630000000 (usable)
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  arch/x86/xen/setup.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 056d11f..5a093b7 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -313,6 +313,17 @@ static void xen_align_and_add_e820_region(u64 start, u64 size, int type)
>  	e820_add_region(start, end - start, type);
>  }
>  
> +void xen_ignore_unusable(struct e820entry *list, size_t map_size)
> +{
> +	struct e820entry *entry;
> +	unsigned int i;
> +
> +	for (i = 0, entry = list; i < map_size; i++, entry++) {
> +		if (entry->type == E820_UNUSABLE)
> +			entry->type = E820_RAM;
> +	}
> +}
> +
>  /**
>   * machine_specific_memory_setup - Hook for machine specific memory setup.
>   **/
> @@ -353,6 +364,16 @@ char * __init xen_memory_setup(void)
>  	}
>  	BUG_ON(rc);
>  
> +	/*
> +	 * Xen won't allow a 1:1 mapping to be created to UNUSABLE
> +	 * regions, so if we're using the machine memory map leave the
> +	 * region as RAM as it is in the pseudo-physical map.
> +	 *
> +	 * UNUSABLE regions are not expected in domUs.
> +	 */
> +	if (xen_initial_domain())
> +		xen_ignore_unusable(map, memmap.nr_entries);
> +
>  	/* Make sure the Xen-supplied memory map is well-ordered. */
>  	sanitize_e820_map(map, memmap.nr_entries, &memmap.nr_entries);
>  
> -- 
> 1.7.2.5
> 

Re: [PATCH 2/2] x86/xen: during early setup, only 1:1 map the ISA region

[Konrad Rzeszutek Wilk]

On Fri, Aug 16, 2013 at 03:42:56PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> During early setup, when the reserved regions and MMIO holes are being
> setup as 1:1 in the p2m, clear any mappings instead of making them 1:1
> (execept for the ISA region which is expected to be mapped).
> 
> This reverts a change in behaviour introduced in 3.5 by 83d51ab473dd
> (xen/setup: update VA mapping when releasing memory during setup).

So it won't cause the original issues to reappear which is that we
get this

    (XEN) page_alloc.c:1148:d0 Over-allocation for domain 0: 2097153 > 2097152
    (XEN) memory.c:133:d0 Could not allocate order=0 extent: id=0 memflags=0 (0 of 17)
    
if we boot without dom0_mem_max ?

> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  arch/x86/xen/setup.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
> index 5a093b7..081292e 100644
> --- a/arch/x86/xen/setup.c
> +++ b/arch/x86/xen/setup.c
> @@ -215,13 +215,19 @@ static void __init xen_set_identity_and_release_chunk(
>  	unsigned long pfn;
>  
>  	/*
> -	 * If the PFNs are currently mapped, the VA mapping also needs
> -	 * to be updated to be 1:1.
> +	 * If the PFNs are currently mapped, clear the mappings
> +	 * (except for the ISA region which must be 1:1 mapped) to
> +	 * release the refcounts (in Xen) on the original frames.
>  	 */
> -	for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++)
> +	for (pfn = start_pfn; pfn <= max_pfn_mapped && pfn < end_pfn; pfn++) {
> +		pte_t pte = __pte_ma(0);
> +
> +		if (pfn < PFN_UP(ISA_END_ADDRESS))
> +			pte = mfn_pte(pfn, PAGE_KERNEL_IO);
> +
>  		(void)HYPERVISOR_update_va_mapping(
> -			(unsigned long)__va(pfn << PAGE_SHIFT),
> -			mfn_pte(pfn, PAGE_KERNEL_IO), 0);
> +			(unsigned long)__va(pfn << PAGE_SHIFT), pte, 0);
> +	}
>  
>  	if (start_pfn < nr_pages)
>  		*released += xen_release_chunk(
> -- 
> 1.7.2.5
> 

Re: [PATCH 2/2] x86/xen: during early setup, only 1:1 map the ISA region

[David Vrabel]

On 16/08/13 16:25, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 16, 2013 at 03:42:56PM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> During early setup, when the reserved regions and MMIO holes are being
>> setup as 1:1 in the p2m, clear any mappings instead of making them 1:1
>> (execept for the ISA region which is expected to be mapped).
>>
>> This reverts a change in behaviour introduced in 3.5 by 83d51ab473dd
>> (xen/setup: update VA mapping when releasing memory during setup).
> 
> So it won't cause the original issues to reappear which is that we
> get this
> 
>     (XEN) page_alloc.c:1148:d0 Over-allocation for domain 0: 2097153 > 2097152
>     (XEN) memory.c:133:d0 Could not allocate order=0 extent: id=0 memflags=0 (0 of 17)
>     
> if we boot without dom0_mem_max ?

No, because the mapping is either updated or cleared.

David

Re: [PATCH 1/2] x86/xen: do not identity map UNUSABLE regions in the machine E820

[David Vrabel]

On 16/08/13 16:23, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 16, 2013 at 03:42:55PM +0100, David Vrabel wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> If there are UNUSABLE regions in the machine memory map, dom0 will
>> attempt to map them 1:1 which is not permitted by Xen and the kernel
>> will crash.
>>
>> There isn't anything interesting in the UNUSABLE region that the dom0
>> kernel needs access to so we can avoid making the 1:1 mapping and
>> treat it as RAM.
>>
>> We only do this for dom0, as we do not expect any domU to ever have a
>> UNUSABLE region in their pseudo-physical map.
> 
> Hm, you are going to be disappointed that this is in libxl:

The code is fine though since it only messes with UNUSABLE regions for
the initial domain. Feel free to fix up the comments and commit description.

David