On Tue, 3 Mar 2020 at 21:54, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> Commit d367cef0a7f0 ("x86/mm/pat: Fix boot crash when 1GB pages are not
> supported by the CPU") added checking for CPU support for 1G pages
> before using them.
>
> However, when support is not present, nothing is done to map the
> intermediate 1G regions and we go directly to the code that normally
> maps the remainder after 1G mappings have been done. This code can only
> handle mappings that fit inside a single PUD entry, but there is no
> check, and it instead silently produces a corrupted mapping to the end
> of the PUD entry, and no mapping beyond it, but still returns success.
>
> This bug is encountered on EFI machines in mixed mode (32-bit firmware
> with 64-bit kernel), with RAM beyond 2G. The EFI support code
> direct-maps all the RAM, so a memory range from below 1G to above 2G
> triggers the bug and results in no mapping above 2G, and an incorrect
> mapping in the 1G-2G range. If the kernel resides in the 1G-2G range, a
> firmware call does not return correctly, and if it resides above 2G, we
> end up passing addresses that are not mapped in the EFI pagetable.
>
> Fix this by mapping the 1G regions using 2M pages when 1G page support
> is not available.
>
> Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx>
I was trying to test these patches, and while they seem fine from a
regression point of view, I can't seem to reproduce this issue and
make it go away again by applying this patch.
Do you have any detailed instructions how to reproduce this?
> ---
> arch/x86/mm/pat/set_memory.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> index c4aedd00c1ba..d0b7b06253a5 100644
> --- a/arch/x86/mm/pat/set_memory.c
> +++ b/arch/x86/mm/pat/set_memory.c
> @@ -1370,12 +1370,22 @@ static int populate_pud(struct cpa_data *cpa, unsigned long start, p4d_t *p4d,
> /*
> * Map everything starting from the Gb boundary, possibly with 1G pages
> */
> - while (boot_cpu_has(X86_FEATURE_GBPAGES) && end - start >= PUD_SIZE) {
> - set_pud(pud, pud_mkhuge(pfn_pud(cpa->pfn,
> - canon_pgprot(pud_pgprot))));
> + while (end - start >= PUD_SIZE) {
> + if (boot_cpu_has(X86_FEATURE_GBPAGES)) {
> + set_pud(pud, pud_mkhuge(pfn_pud(cpa->pfn,
> + canon_pgprot(pud_pgprot))));
> + cpa->pfn += PUD_SIZE >> PAGE_SHIFT;
> + } else {
> + if (pud_none(*pud))
> + if (alloc_pmd_page(pud))
> + return -1;
> + if (populate_pmd(cpa, start, start + PUD_SIZE,
> + PUD_SIZE >> PAGE_SHIFT,
> + pud, pgprot) < 0)
> + return cur_pages;
> + }
>
> start += PUD_SIZE;
> - cpa->pfn += PUD_SIZE >> PAGE_SHIFT;
> cur_pages += PUD_SIZE >> PAGE_SHIFT;
> pud++;
> }
> --
> 2.24.1
>
