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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b726e0d7-7dfb-d902-652f-8aab4bf43e89@amd.com>
Date:   Wed, 2 Dec 2020 11:58:15 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Borislav Petkov <bp@...en8.de>,
        Arvind Sankar <nivedita@...m.mit.edu>
Cc:     x86@...nel.org, Kim Phillips <kim.phillips@....com>,
        Yazen Ghannam <yazen.ghannam@....com>, Pu Wen <puwen@...on.cn>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/cpu/amd: Remove dead code for TSEG region remapping

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.
>