Re: [PATCH] x86/mm: ensure L2 is always freed if empty

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/10/2024 11:48 am, Roger Pau Monne wrote:
> The current logic in modify_xen_mappings() allows for fully empty L2 tables to
> not be freed and unhooked from the parent L3 if the last L2 slot is not
> populated.
>
> Ensure that even when an L2 slot is empty the logic to check whether the whole
> L2 can be removed is not skipped.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> I've attempted to find a Fixes tag for this one, but I'm afraid there have 
> been
> many changes in the function, and it's possibly the code that introduced the 
> L2
> freeing (4376c05c31132) the one that failed to originally adjust this case.
> ---
>  xen/arch/x86/mm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index d537a799bced..0f53dcebad95 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5717,7 +5717,7 @@ int modify_xen_mappings(unsigned long s, unsigned long 
> e, unsigned int nf)
>  
>              v += 1UL << L2_PAGETABLE_SHIFT;
>              v &= ~((1UL << L2_PAGETABLE_SHIFT) - 1);
> -            continue;
> +            goto check_l3;
>          }
>  
>          if ( l2e_get_flags(*pl2e) & _PAGE_PSE )

https://gitlab.com/xen-project/hardware/xen/-/jobs/8240163332

That's hitting the BUG_ON(!nr_bootmem_regions); in alloc_boot_pages().

  [<ffff82d04060407a>] R alloc_boot_pages+0x46/0xbc
  [<ffff82d04033a76f>] S alloc_xen_pagetable+0x5e/0x60
  [<ffff82d04033a7af>] F alloc_mapped_pagetable+0xd/0x3e
  [<ffff82d04033a948>] F arch/x86/mm.c#virt_to_xen_l2e+0x78/0xec
  [<ffff82d04033ba71>] F map_pages_to_xen+0xefb/0x1079
  [<ffff82d04063d55c>] F arch/x86/setup#bootstrap_map_addr+0x95/0xff

I think what's happened is you've "freed" l2_bootmap[], causing
bootstrap_map_addr() to now need to allocate memory.

I've reverted for now, per Roger's request on Matrix.

~Andrew