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

[PATCHv2] 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 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.
---
 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(
-- 
1.7.2.5

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

[Aurelien Chartier]

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(

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

[David Vrabel]

On 25/07/13 16:46, Aurelien Chartier wrote:
> 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 :

Does this
(http://lists.xen.org/archives/html/xen-devel/2013-07/msg00701.html)
patch work better?

I do think hiding UNUSABLE regions from dom0 is the right thing to do
and will be a more reliable fix going forwards, but Konrad didn't agree.

David

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

[Konrad Rzeszutek Wilk]

David Vrabel <david.vrabel@citrix.com> wrote:
>On 25/07/13 16:46, Aurelien Chartier wrote:
>> 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 :
>
>Does this
>(http://lists.xen.org/archives/html/xen-devel/2013-07/msg00701.html)
>patch work better?
>
>I do think hiding UNUSABLE regions from dom0 is the right thing to do
>and will be a more reliable fix going forwards, but Konrad didn't
>agree.
>
>David

It well might but we need to:
 -  Know why it is still happening after a fix that is tailored towards the fix and still does not work with stable kernels.  What are we missing? 
 -  it is unclear to me how this will affect PV DomU with PCI pass through which use  e820_hole parameter -  which means that the e820 in the guest is similar to the host one. We can't break them. 

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

[Aurelien Chartier]

On 25/07/13 18:33, Konrad Rzeszutek Wilk wrote:
> David Vrabel <david.vrabel@citrix.com> wrote:
>> On 25/07/13 16:46, Aurelien Chartier wrote:
>>> 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 :
>> Does this
>> (http://lists.xen.org/archives/html/xen-devel/2013-07/msg00701.html)
>> patch work better?

This patch was fixing the crash on 3.8.13.x, but I did not test it with
3.11.

I will try to find which commit fixed the crash I reported between those
two versions.


>>
>> I do think hiding UNUSABLE regions from dom0 is the right thing to do
>> and will be a more reliable fix going forwards, but Konrad didn't
>> agree.
>>
>> David
> It well might but we need to:
>  -  Know why it is still happening after a fix that is tailored towards the fix and still does not work with stable kernels.  What are we missing? 
>  -  it is unclear to me how this will affect PV DomU with PCI pass through which use  e820_hole parameter -  which means that the e820 in the guest is similar to the host one. We can't break them.