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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.10.2024 11:36, Roger Pau Monné wrote:
> 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.

Done so. Despite the tag I'm uncertain whether to backport.

Jan