Re: [PATCH] x86/cpu/amd: Remove dead code for TSEG region remapping

[Tom Lendacky <thomas.lendacky@amd.com>]

On 11/27/20 11:27 AM, Borislav Petkov wrote:
> On Fri, Nov 27, 2020 at 12:13:24PM -0500, Arvind Sankar wrote:
>> Commit
>>    26bfa5f89486 ("x86, amd: Cleanup init_amd")
>> moved the code that remaps the TSEG region using 4k pages from
>> init_amd() to bsp_init_amd().
>>
>> However, bsp_init_amd() is executed well before the direct mapping is
>> actually created:
>>
>>    setup_arch()
>>      -> early_cpu_init()
>>        -> early_identify_cpu()
>>          -> this_cpu->c_bsp_init()
>> 	  -> bsp_init_amd()
>>      ...
>>      -> init_mem_mapping()
>>
>> So the change effectively disabled the 4k remapping, because
>> pfn_range_is_mapped() is always false at this point.
>>
>> It has been over six years since the commit, and no-one seems to have
>> noticed this, so just remove the code. The original code was also
>> incomplete, since it doesn't check how large the TSEG address range
>> actually is, so it might remap only part of it in any case.
> 
> Yah, and the patch which added this:
> 
> 6c62aa4a3c12 ("x86: make amd.c have 64bit support code")
> 
> does not say what for (I'm not surprised, frankly).
> 
> So if AMD folks on Cc don't have any need for actually fixing this
> properly, yap, we can zap it.

I believe this is geared towards performance. If the TSEG base address is 
not 2MB aligned, then hardware has to break down a 2MB TLB entry if the OS 
references the memory within the 2MB page that is before the TSEG base 
address. This can occur whenever the 2MB TLB entry is re-installed because 
of TLB flushes, etc.

I would hope that newer BIOSes are 2MB aligning the TSEG base address, but 
if not, then this can help.

So moving it back wouldn't be a bad thing. It should probably only do the 
set_memory_4k() if the TSEG base address is not 2MB aligned, which I think 
is covered by the pfn_range_is_mapped() call?

Thanks,
Tom

> 
> Thx.
>