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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Oct 31, 2024 at 10:59:36AM +0100, Jan Beulich wrote:
> On 30.10.2024 12:48, 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>
> 
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> > ---
> > 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.
> 
> Yes, that looks to be the one. It has been a long while, but I think it was
> (wrong) similarity considerations with the corresponding L3E handling (near
> the top of the main loop) that resulted in me not touching that "continue".
> We certainly don't want to ever free L3 tables (vacating L4 entries); since
> the 32-bit case still mattered back then, the "continue" also couldn't have
> been simply replaced (as there that same consideration applied to L2 tables
> and L3 entries, for further extended reasons imposed by PAE).

I agree we don't want to free L3 tables and thus zap L4 entries.

Feel free to add:

Fixes: 4376c05c3113 ('x86-64: use 1GB pages in 1:1 mapping if available')

If you want.

Thanks, Roger.