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

[Aurelien Chartier <aurelien.chartier@citrix.com>]

On 22/07/13 15:29, 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 fixes a regression introduced in 3.5 by 83d51ab473dd (xen/setup:
> update VA mapping when releasing memory during setup) which caused
> hosts with tboot to fail to boot.
>
> 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>
> ---
> v2: 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.

I made more extensive testing and I was wrong, the crash I reported has
been fixed upstream. I am able to boot a 3.11-rc1 kernel without any
patch applied. However, I am still seeing errors in the log :

(XEN) mm.c:901:d0 Error getting mfn 800 (pfn 5555555555555555) from L1
entry 0000000000800463 for l1e_owner=0, pg_owner=0

David's patch is fixing those errors.

I also tried applying that patch to 3.8.13.4, but dom0 was still
crashing at boot time :

[    0.000000] init_memory_mapping: [mem 0x00000000-0x373fdfff]
(XEN) mm.c:901:d0 Error getting mfn 800 (pfn 5555555555555555) from L1
entry 0000000000800403 for l1e_owner=0, pg_owner=0
(XEN) mm.c:4976:d0 ptwr_emulate: could not get_page_from_l1e()
[    0.000000] BUG: unable to handle kernel NULL pointer dereference
at   (null)
[    0.000000] IP: [<c16c0e8e>] xen_set_pte_init+0x38/0x3d
[    0.000000] *pdpt = 0000000000000000 *pde = 8bd078326a2f41e0
[    0.000000] Oops: 0003 [#1] SMP
[    0.000000] Modules linked in:
[    0.000000] Pid: 0, comm: swapper Not tainted 3.8.13.4 #4 Dell Inc.
Latitude E6530/07Y85M
[    0.000000] EIP: e019:[<c16c0e8e>] EFLAGS: 00010046 CPU: 0
[    0.000000] EIP is at xen_set_pte_init+0x38/0x3d
[    0.000000] EAX: 00000000 EBX: c0800000 ECX: 00800403 EDX: 00000000
[    0.000000] ESI: c288c000 EDI: 00000800 EBP: c165bdd8 ESP: c165bdd4
[    0.000000]  DS: e021 ES: e021 FS: 00d8 GS: 0000 SS: e021
[    0.000000] CR0: 80050033 CR2: 00000000 CR3: 01739000 CR4: 00002660
[    0.000000] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[    0.000000] DR6: 00000000 DR7: 00000000
[    0.000000] Process swapper (pid: 0, ti=c165a000 task=c1667120
task.ti=c165a000)
[    0.000000] Stack:
[    0.000000]  00000000 c165bde0 c146b438 c165be2c c16d0610 00000801
c288c000 00000000
[    0.000000]  c17b7020 00000004 000373fe 00000000 00800000 c1739018
00000003 00000001
[    0.000000]  00000000 c1739018 00000003 c165be70 00000001 00000001
c165be8c c145d608
[    0.000000] Call Trace: 
[    0.000000]  [<c146b438>] set_pte+0x14/0x16
[    0.000000]  [<c16d0610>] kernel_physical_mapping_init+0x19b/0x262
[    0.000000]  [<c145d608>] init_memory_mapping+0x1d8/0x530
[    0.000000]  [<c16c315b>] setup_arch+0x726/0xcba
[    0.000000]  [<c1009a3e>] ? __raw_callee_save_xen_restore_fl+0x6/0x8
[    0.000000]  [<c1009a38>] ? __raw_callee_save_xen_save_fl+0x8/0x8
[    0.000000]  [<c10494c7>] ? vprintk_emit+0x217/0x4b0
[    0.000000]  [<c146c3a5>] ? printk+0x38/0x3a
[    0.000000]  [<c16bd6e5>] start_kernel+0x75/0x2e8
[    0.000000]  [<c16bd2d8>] i386_start_kernel+0x9b/0xa2
[    0.000000]  [<c16c02ed>] xen_start_kernel+0x5ff/0x60a
[    0.000000] Code: 89 da 25 00 f0 ff ff 81 e2 ff 0f 00 00 0f ac d0 0c
40 74 0f
 8b 06 a8 01 74 0d 83 c8 fd 21 c8 89 c1 eb 04 31 c9 31 db 89 5e 04 5b
<89> 0e 5e
 5d c3 55 89 e5 50 e8 a2 fb 00 00 e8 3f 40 94 ff 83 3d
[    0.000000] EIP: [<c16c0e8e>] xen_set_pte_init+0x38/0x3d SS:ESP
e021:c165bdd4
[    0.000000] CR2: 0000000000000000
[    0.000000] ---[ end trace 75a1f50abddd969d ]---
[    0.000000] Kernel panic - not syncing: Attempted to kill the idle task!
(XEN) Domain 0 crashed: rebooting machine in 5 seconds.

> ---
>  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 94eac5c..9411756 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(