Re: [PATCH v2 4/4] x86/64/mm: Map all kernel memory into trampoline_pgd

[Dave Hansen <dave.hansen@intel.com>]

On 9/29/21 7:55 AM, Joerg Roedel wrote:
...
> The reason this does not crash always is only that kernel mappings are
> global and the CR3 switch does not flush those mappings. But if theses
> mappings are not in the TLB already, the above code will crash before it
> can jump to the real-mode stub.

This would have been nice to have in the cover letter.  The whole
purpose for this series wasn't totally apparent until I read this.

> diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
> index 0cfe1046cec9..792cb9ca9b29 100644
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -91,6 +91,7 @@ static void __init setup_real_mode(void)
>  #ifdef CONFIG_X86_64
>  	u64 *trampoline_pgd;
>  	u64 efer;
> +	int i;
>  #endif
>  
>  	base = (unsigned char *)real_mode_header;
> @@ -147,8 +148,17 @@ static void __init setup_real_mode(void)
>  	trampoline_header->flags = 0;
>  
>  	trampoline_pgd = (u64 *) __va(real_mode_header->trampoline_pgd);
> +
> +	/*
> +	 * Map all of kernel memory into the trampoline PGD so that it includes
> +	 * the direct mapping and vmalloc space. This is needed to keep the
> +	 * stack and real_mode_header mapped when switching to this page table.
> +	 */

This comment's mention of the direct map and vmalloc() makes a lot of
sense in the context of this patch where you're adding them.  But, it
doesn't mention the pgd[511] stuff.

Maybe just make it more generic:

	Include the entirety of the kernel mapping into the trampoline
	PGD.  This way, all mappings present in the normal kernel page
	tables are usable while running on trampoline_pgd.


> +	for (i = pgd_index(__PAGE_OFFSET); i < PTRS_PER_PGD; i++)
> +		trampoline_pgd[i] = init_top_pgt[i].pgd;
> +
> +	/* Map the real mode stub as virtual == physical */
>  	trampoline_pgd[0] = trampoline_pgd_entry.pgd;
> -	trampoline_pgd[511] = init_top_pgt[511].pgd;
>  #endif

Nit: can we preserve the order, please?

	/* Map the real mode stub as virtual == physical */
  	trampoline_pgd[0] = trampoline_pgd_entry.pgd;

	for (i = pgd_index(__PAGE_OFFSET); i < PTRS_PER_PGD; i++)
		trampoline_pgd[i] = init_top_pgt[i].pgd;