Re: [PATCH 5/5] x86/pvh: Add 64bit relocation page tables

Juergen Gross
2024-05-23T12:11:07Z

[-- Attachment #1.1.1: Type: text/plain, Size: 6613 bytes --]

On 10.04.24 21:48, Jason Andryuk wrote:
> The PVH entry point is 32bit. For a 64bit kernel, the entry point must
> switch to 64bit mode, which requires a set of page tables. In the past,
> PVH used init_top_pgt.
>
> This works fine when the kernel is loaded at LOAD_PHYSICAL_ADDR, as the
> page tables are prebuilt for this address. If the kernel is loaded at a
> different address, they need to be adjusted.
>
> __startup_64() adjusts the prebuilt page tables for the physical load
> address, but it is 64bit code. The 32bit PVH entry code can't call it
> to adjust the page tables, so it can't readily be re-used.
>
> 64bit PVH entry needs page tables set up for identity map, the kernel
> high map and the direct map. pvh_start_xen() enters identity mapped.
> Inside xen_prepare_pvh(), it jumps through a pv_ops function pointer
> into the highmap. The direct map is used for __va() on the initramfs
> and other guest physical addresses.
>
> Add a dedicated set of prebuild page tables for PVH entry. They are
> adjusted in assembly before loading.
>
> Add XEN_ELFNOTE_PHYS32_RELOC to indicate support for relocation
> along with the kernel's loading constraints. The maximum load address,
> KERNEL_IMAGE_SIZE - 1, is determined by a single pvh_level2_ident_pgt
> page. It could be larger with more pages.
>
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Instead of adding 5 pages of prebuilt page tables, they could be
> contructed dynamically in the .bss area. They are then only used for
> PVH entry and until transitioning to init_top_pgt. The .bss is later
> cleared. It's safer to add the dedicated pages, so that is done here.
> ---
> arch/x86/platform/pvh/head.S | 105 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 104 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S
> index c08d08d8cc92..4af3cfbcf2f8 100644
> --- a/arch/x86/platform/pvh/head.S
> +++ b/arch/x86/platform/pvh/head.S
> @@ -21,6 +21,8 @@
> #include <asm/nospec-branch.h>
> #include <xen/interface/elfnote.h>
>
> +#include "../kernel/pgtable_64_helpers.h"
> +
> __HEAD
>
> /*
> @@ -102,8 +104,47 @@ SYM_CODE_START_LOCAL(pvh_start_xen)
> btsl $_EFER_LME, %eax
> wrmsr
>
> + mov %ebp, %ebx
> + subl $LOAD_PHYSICAL_ADDR, %ebx /* offset */
> + jz .Lpagetable_done
> +
> + /* Fixup page-tables for relocation. */
> + leal rva(pvh_init_top_pgt)(%ebp), %edi
> + movl $512, %ecx

Please use PTRS_PER_PGD instead of the literal 512. Similar issue below.

> +2:
> + testl $_PAGE_PRESENT, 0x00(%edi)
> + jz 1f
> + addl %ebx, 0x00(%edi)
> +1:
> + addl $8, %edi
> + decl %ecx
> + jnz 2b
> +
> + /* L3 ident has a single entry. */
> + leal rva(pvh_level3_ident_pgt)(%ebp), %edi
> + addl %ebx, 0x00(%edi)
> +
> + leal rva(pvh_level3_kernel_pgt)(%ebp), %edi
> + addl %ebx, (4096 - 16)(%edi)
> + addl %ebx, (4096 - 8)(%edi)

PAGE_SIZE instead of 4096, please.

> +
> + /* pvh_level2_ident_pgt is fine - large pages */
> +
> + /* pvh_level2_kernel_pgt needs adjustment - large pages */
> + leal rva(pvh_level2_kernel_pgt)(%ebp), %edi
> + movl $512, %ecx
> +2:
> + testl $_PAGE_PRESENT, 0x00(%edi)
> + jz 1f
> + addl %ebx, 0x00(%edi)
> +1:
> + addl $8, %edi
> + decl %ecx
> + jnz 2b
> +
> +.Lpagetable_done:
> /* Enable pre-constructed page tables. */
> - leal rva(init_top_pgt)(%ebp), %eax
> + leal rva(pvh_init_top_pgt)(%ebp), %eax
> mov %eax, %cr3
> mov $(X86_CR0_PG | X86_CR0_PE), %eax
> mov %eax, %cr0
> @@ -197,5 +238,67 @@ SYM_DATA_START_LOCAL(early_stack)
> .fill BOOT_STACK_SIZE, 1, 0
> SYM_DATA_END_LABEL(early_stack, SYM_L_LOCAL, early_stack_end)
>
> +#ifdef CONFIG_X86_64
> +/*
> + * Xen PVH needs a set of identity mapped and kernel high mapping
> + * page tables. pvh_start_xen starts running on the identity mapped
> + * page tables, but xen_prepare_pvh calls into the high mapping.
> + * These page tables need to be relocatable and are only used until
> + * startup_64 transitions to init_top_pgt.
> + */
> +SYM_DATA_START_PAGE_ALIGNED(pvh_init_top_pgt)
> + .quad pvh_level3_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC
> + .org pvh_init_top_pgt + L4_PAGE_OFFSET*8, 0

Please add a space before and after the '*'.

> + .quad pvh_level3_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC
> + .org pvh_init_top_pgt + L4_START_KERNEL*8, 0
> + /* (2^48-(2*1024*1024*1024))/(2^39) = 511 */
> + .quad pvh_level3_kernel_pgt - __START_KERNEL_map + _PAGE_TABLE_NOENC
> +SYM_DATA_END(pvh_init_top_pgt)
> +
> +SYM_DATA_START_PAGE_ALIGNED(pvh_level3_ident_pgt)
> + .quad pvh_level2_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC
> + .fill 511, 8, 0
> +SYM_DATA_END(pvh_level3_ident_pgt)
> +SYM_DATA_START_PAGE_ALIGNED(pvh_level2_ident_pgt)
> + /*
> + * Since I easily can, map the first 1G.
> + * Don't set NX because code runs from these pages.
> + *
> + * Note: This sets _PAGE_GLOBAL despite whether
> + * the CPU supports it or it is enabled. But,
> + * the CPU should ignore the bit.
> + */
> + PMDS(0, __PAGE_KERNEL_IDENT_LARGE_EXEC, PTRS_PER_PMD)
> +SYM_DATA_END(pvh_level2_ident_pgt)
> +SYM_DATA_START_PAGE_ALIGNED(pvh_level3_kernel_pgt)
> + .fill L3_START_KERNEL,8,0

Spaces after the commas.

> + /* (2^48-(2*1024*1024*1024)-((2^39)*511))/(2^30) = 510 */
> + .quad pvh_level2_kernel_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC
> + .quad 0 /* no fixmap */
> +SYM_DATA_END(pvh_level3_kernel_pgt)
> +
> +SYM_DATA_START_PAGE_ALIGNED(pvh_level2_kernel_pgt)
> + /*
> + * Kernel high mapping.
> + *
> + * The kernel code+data+bss must be located below KERNEL_IMAGE_SIZE in
> + * virtual address space, which is 1 GiB if RANDOMIZE_BASE is enabled,
> + * 512 MiB otherwise.
> + *
> + * (NOTE: after that starts the module area, see MODULES_VADDR.)
> + *
> + * This table is eventually used by the kernel during normal runtime.
> + * Care must be taken to clear out undesired bits later, like _PAGE_RW
> + * or _PAGE_GLOBAL in some cases.
> + */
> + PMDS(0, __PAGE_KERNEL_LARGE_EXEC, KERNEL_IMAGE_SIZE/PMD_SIZE)

Spaces around '/'.

> +SYM_DATA_END(pvh_level2_kernel_pgt)
> +
> + ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_RELOC,
> + .long CONFIG_PHYSICAL_ALIGN;
> + .long LOAD_PHYSICAL_ADDR;
> + .long KERNEL_IMAGE_SIZE - 1)
> +#endif
> +
> ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,
> _ASM_PTR (pvh_start_xen - __START_KERNEL_map))

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]