Hello,

The attempt to fix destroy_xen_mappings() so that L2 tables are
consistently freed uncovered some errors in the memory management code.
The following series attempts to fix them.

All patches except for 4/4 are new in v2, and 4/4 has no change from v1,
hence kept Jan's Reviewed-by tag in 4/4.

Thanks, Roger.

Roger Pau Monne (4):
  x86/mm: introduce helpers to detect super page alignment
  x86/mm: skip super-page alignment checks for non-present entries
  x86/setup: remove bootstrap_map_addr() usage of destroy_xen_mappings()
  x86/mm: ensure L2 is always freed if empty

 xen/arch/x86/mm.c    | 33 ++++++++++++++++++++++-----------
 xen/arch/x86/setup.c |  4 +++-
 2 files changed, 25 insertions(+), 12 deletions(-)

-- 
2.46.0

On 14/11/2024 2:57 pm, Roger Pau Monne wrote:
> Hello,
>
> The attempt to fix destroy_xen_mappings() so that L2 tables are
> consistently freed uncovered some errors in the memory management code.
> The following series attempts to fix them.
>
> All patches except for 4/4 are new in v2, and 4/4 has no change from v1,
> hence kept Jan's Reviewed-by tag in 4/4.
>
> Thanks, Roger.
>
> Roger Pau Monne (4):
>   x86/mm: introduce helpers to detect super page alignment
>   x86/mm: skip super-page alignment checks for non-present entries
>   x86/setup: remove bootstrap_map_addr() usage of destroy_xen_mappings()
>   x86/mm: ensure L2 is always freed if empty

Still broken.  This happened to be a Cascade Lake:

(XEN) 3... 2... 1...
(XEN) Xen is relinquishing VGA console.
(XEN) ----[ Xen-4.20.0-4-d  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) RIP:    e008:[<ffff82d04022cdc8>]
common/page_alloc.c#free_heap_pages+0x52/0x753
<snip>
(XEN) Xen call trace:
(XEN)    [<ffff82d04022cdc8>] R
common/page_alloc.c#free_heap_pages+0x52/0x753
(XEN)    [<ffff82d04022e98f>] F free_domheap_pages+0x445/0x447
(XEN)    [<ffff82d040320f88>] F free_xen_pagetable+0x2f/0x31
(XEN)    [<ffff82d04032193d>] F map_pages_to_xen+0x5df/0x1028
(XEN)    [<ffff82d040238ab1>] F vunmap+0x27/0xde
(XEN)    [<ffff82d040624b61>] F vesa_endboot+0x86/0xb3
(XEN)    [<ffff82d0406244c1>] F video_endboot+0x93/0x261
(XEN)    [<ffff82d040616e3d>] F console_endboot+0x92/0x120
(XEN)    [<ffff82d0406407e2>] F __start_xen+0x2139/0x2307
(XEN)    [<ffff82d0402033ae>] F __high_start+0x8e/0x90
(XEN)
(XEN) Pagetable walk from ffff82c000205001:
(XEN)  L4[0x105] = 0000000060431063 ffffffffffffffff
(XEN)  L3[0x100] = 000000006fffd063 ffffffffffffffff
(XEN)  L2[0x001] = fffffffffffff000 ffffffffffffffff
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=0000]
(XEN) Faulting linear address: ffff82c000205001
(XEN) ****************************************

Note the definitely bogus L2e, which I'm guessing is an issue with the
use of INVALID_MFN.

But the same crash is seen on BDX, SKX, SPR, Naples, Rome and Milan, all
including the bad L2e.

ICX crashed but there was no serial console.


Genoa was weird.  It was:

(XEN) ----[ Xen-4.20.0-4-d  x86_64  debug=y  Not tainted ]----
(XEN) CPU:    24
(XEN) RIP:    e008:[<ffff82d040330509>] __node_distance+0x20/0x62
<snip>
(XEN) Xen call trace:
(XEN)    [<ffff82d040330509>] R __node_distance+0x20/0x62
(XEN)    [<ffff82d040240ee5>] F do_sysctl+0xcf8/0x140e
(XEN)    [<ffff82d0402f6b21>] F pv_hypercall+0x59b/0x62b
(XEN)    [<ffff82d0402012d3>] F lstar_enter+0x143/0x150
(XEN)
(XEN) Pagetable walk from ffff82c000203024:
(XEN)  L4[0x105] = 000000005fc31063 ffffffffffffffff
(XEN)  L3[0x100] = 00000000677cd063 ffffffffffffffff
(XEN)  L2[0x001] = 000000207a79a063 ffffffffffffffff
(XEN)  L1[0x003] = 0000000000000000 ffffffffffffffff
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 24:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=0000]
(XEN) Faulting linear address: ffff82c000203024
(XEN) ****************************************

while sat at the login prompt, which means it had survived a full
install and first boot.

The range of the change was 64d2290cf96bb^..d2c214ede05ca (starting from
Jan's fix to _xvrealloc()) but there are no other obvious candidates.

This crash isn't obviously related to the others, but there's nothing
else like it seen before.  Memory corruption isn't out of the question.

~Andrew

On 15.11.2024 01:55, Andrew Cooper wrote:
> On 14/11/2024 2:57 pm, Roger Pau Monne wrote:
>> Hello,
>>
>> The attempt to fix destroy_xen_mappings() so that L2 tables are
>> consistently freed uncovered some errors in the memory management code.
>> The following series attempts to fix them.
>>
>> All patches except for 4/4 are new in v2, and 4/4 has no change from v1,
>> hence kept Jan's Reviewed-by tag in 4/4.
>>
>> Thanks, Roger.
>>
>> Roger Pau Monne (4):
>>   x86/mm: introduce helpers to detect super page alignment
>>   x86/mm: skip super-page alignment checks for non-present entries
>>   x86/setup: remove bootstrap_map_addr() usage of destroy_xen_mappings()
>>   x86/mm: ensure L2 is always freed if empty
> 
> Still broken.  This happened to be a Cascade Lake:
> 
> (XEN) 3... 2... 1...
> (XEN) Xen is relinquishing VGA console.
> (XEN) ----[ Xen-4.20.0-4-d  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<ffff82d04022cdc8>]
> common/page_alloc.c#free_heap_pages+0x52/0x753
> <snip>
> (XEN) Xen call trace:
> (XEN)    [<ffff82d04022cdc8>] R
> common/page_alloc.c#free_heap_pages+0x52/0x753
> (XEN)    [<ffff82d04022e98f>] F free_domheap_pages+0x445/0x447
> (XEN)    [<ffff82d040320f88>] F free_xen_pagetable+0x2f/0x31
> (XEN)    [<ffff82d04032193d>] F map_pages_to_xen+0x5df/0x1028
> (XEN)    [<ffff82d040238ab1>] F vunmap+0x27/0xde
> (XEN)    [<ffff82d040624b61>] F vesa_endboot+0x86/0xb3
> (XEN)    [<ffff82d0406244c1>] F video_endboot+0x93/0x261
> (XEN)    [<ffff82d040616e3d>] F console_endboot+0x92/0x120
> (XEN)    [<ffff82d0406407e2>] F __start_xen+0x2139/0x2307
> (XEN)    [<ffff82d0402033ae>] F __high_start+0x8e/0x90
> (XEN)
> (XEN) Pagetable walk from ffff82c000205001:
> (XEN)  L4[0x105] = 0000000060431063 ffffffffffffffff
> (XEN)  L3[0x100] = 000000006fffd063 ffffffffffffffff
> (XEN)  L2[0x001] = fffffffffffff000 ffffffffffffffff
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) FATAL PAGE FAULT
> (XEN) [error_code=0000]
> (XEN) Faulting linear address: ffff82c000205001
> (XEN) ****************************************
> 
> Note the definitely bogus L2e, which I'm guessing is an issue with the
> use of INVALID_MFN.

Why bogus? That's precisely what vunmap() requests:

    map_pages_to_xen(addr, INVALID_MFN, pages, _PAGE_NONE);

What I agree with is that INVALID_MFN better wouldn't spill into the
upper non-address part of the PTE, but that doesn't look to be important
here.

I can't, however, explain why free_heap_pages() would access VMAP space.
It's all page / MFN based and shouldn't even have a notion of the VA
space underlying vmap(). Is that address in CR2 inside the range that
was mapping the frame buffer (i.e. part of what vunmap() is in the
process of removing)?

What I also don't understand: How did this pass CI then? Are all tests
there perhaps done without vga= designating a VESA mode to use?

Jan