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

[Tom Lendacky]

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.