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: <733302e0-e8c2-458e-a4b2-dfd10e065036@amd.com>
Date: Wed, 31 Jan 2024 09:30:10 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Kevin Loughlin <kevinloughlin@...gle.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, Nick Desaulniers <ndesaulniers@...gle.com>,
 Justin Stitt <justinstitt@...gle.com>, Pankaj Gupta <pankaj.gupta@....com>,
 Hou Wenlong <houwenlong.hwl@...group.com>, Ard Biesheuvel <ardb@...nel.org>,
 Dionna Glaze <dionnaglaze@...gle.com>, Brijesh Singh
 <brijesh.singh@....com>, Michael Roth <michael.roth@....com>,
 "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
 linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
 linux-coco@...ts.linux.dev, Ashish Kalra <ashish.kalra@....com>,
 Andi Kleen <ak@...ux.intel.com>
Cc: Adam Dunlap <acdunlap@...gle.com>, Peter Gonda <pgonda@...gle.com>,
 Jacob Xu <jacobhxu@...gle.com>, Sidharth Telang <sidtelang@...gle.com>
Subject: Re: [PATCH v3 2/2] x86/head64: Replace pointer fixups with
 RIP_RELATIVE_ADDR()

On 1/30/24 16:08, Kevin Loughlin wrote:
> Now that we have RIP_RELATIVE_ADDR(), we can replace the "fixup_*()"
> family of functions in head64.c with the new macro for cleaner code.

If this series is purely for backporting, this specific patch isn't 
needed, right? Since this all works today with clang?

Thanks,
Tom

> 
> Signed-off-by: Kevin Loughlin <kevinloughlin@...gle.com>
> ---
>   arch/x86/kernel/head64.c | 63 +++++++++++++++-------------------------
>   1 file changed, 24 insertions(+), 39 deletions(-)
> 
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index d159239997f4..e782149eefc4 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -85,23 +85,8 @@ static struct desc_ptr startup_gdt_descr __initdata = {
>   	.address = 0,
>   };
>   
> -static void __head *fixup_pointer(void *ptr, unsigned long physaddr)
> -{
> -	return ptr - (void *)_text + (void *)physaddr;
> -}
> -
> -static unsigned long __head *fixup_long(void *ptr, unsigned long physaddr)
> -{
> -	return fixup_pointer(ptr, physaddr);
> -}
> -
>   #ifdef CONFIG_X86_5LEVEL
> -static unsigned int __head *fixup_int(void *ptr, unsigned long physaddr)
> -{
> -	return fixup_pointer(ptr, physaddr);
> -}
> -
> -static bool __head check_la57_support(unsigned long physaddr)
> +static bool __head check_la57_support(void)
>   {
>   	/*
>   	 * 5-level paging is detected and enabled at kernel decompression
> @@ -110,17 +95,17 @@ static bool __head check_la57_support(unsigned long physaddr)
>   	if (!(native_read_cr4() & X86_CR4_LA57))
>   		return false;
>   
> -	*fixup_int(&__pgtable_l5_enabled, physaddr) = 1;
> -	*fixup_int(&pgdir_shift, physaddr) = 48;
> -	*fixup_int(&ptrs_per_p4d, physaddr) = 512;
> -	*fixup_long(&page_offset_base, physaddr) = __PAGE_OFFSET_BASE_L5;
> -	*fixup_long(&vmalloc_base, physaddr) = __VMALLOC_BASE_L5;
> -	*fixup_long(&vmemmap_base, physaddr) = __VMEMMAP_BASE_L5;
> +	*((unsigned int *)RIP_RELATIVE_ADDR(__pgtable_l5_enabled)) = 1;
> +	*((unsigned int *)RIP_RELATIVE_ADDR(pgdir_shift)) = 48;
> +	*((unsigned int *)RIP_RELATIVE_ADDR(ptrs_per_p4d)) = 512;
> +	*((unsigned long *)RIP_RELATIVE_ADDR(page_offset_base)) = __PAGE_OFFSET_BASE_L5;
> +	*((unsigned long *)RIP_RELATIVE_ADDR(vmalloc_base)) = __VMALLOC_BASE_L5;
> +	*((unsigned long *)RIP_RELATIVE_ADDR(vmemmap_base)) = __VMEMMAP_BASE_L5;
>   
>   	return true;
>   }
>   #else
> -static bool __head check_la57_support(unsigned long physaddr)
> +static bool __head check_la57_support(void)
>   {
>   	return false;
>   }
> @@ -175,8 +160,8 @@ static unsigned long __head sme_postprocess_startup(struct boot_params *bp, pmdv
>    * Code in __startup_64() can be relocated during execution, but the compiler
>    * doesn't have to generate PC-relative relocations when accessing globals from
>    * that function. Clang actually does not generate them, which leads to
> - * boot-time crashes. To work around this problem, every global pointer must
> - * be adjusted using fixup_pointer() or RIP_RELATIVE_ADDR().
> + * boot-time crashes. To work around this problem, every global variable access
> + * must be adjusted using RIP_RELATIVE_ADDR().
>    */
>   unsigned long __head __startup_64(unsigned long physaddr,
>   				  struct boot_params *bp)
> @@ -194,7 +179,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   	int i;
>   	unsigned int *next_pgt_ptr;
>   
> -	la57 = check_la57_support(physaddr);
> +	la57 = check_la57_support();
>   
>   	/* Is the address too large? */
>   	if (physaddr >> MAX_PHYSMEM_BITS)
> @@ -215,7 +200,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   
>   	/* Fixup the physical addresses in the page table */
>   
> -	pgd = fixup_pointer(early_top_pgt, physaddr);
> +	pgd = RIP_RELATIVE_ADDR(early_top_pgt);
>   	p = pgd + pgd_index(__START_KERNEL_map);
>   	if (la57)
>   		*p = (unsigned long)level4_kernel_pgt;
> @@ -224,15 +209,15 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   	*p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
>   
>   	if (la57) {
> -		p4d = fixup_pointer(level4_kernel_pgt, physaddr);
> +		p4d = RIP_RELATIVE_ADDR(level4_kernel_pgt);
>   		p4d[511] += load_delta;
>   	}
>   
> -	pud = fixup_pointer(level3_kernel_pgt, physaddr);
> +	pud = RIP_RELATIVE_ADDR(level3_kernel_pgt);
>   	pud[510] += load_delta;
>   	pud[511] += load_delta;
>   
> -	pmd = fixup_pointer(level2_fixmap_pgt, physaddr);
> +	pmd = RIP_RELATIVE_ADDR(level2_fixmap_pgt);
>   	for (i = FIXMAP_PMD_TOP; i > FIXMAP_PMD_TOP - FIXMAP_PMD_NUM; i--)
>   		pmd[i] += load_delta;
>   
> @@ -243,8 +228,8 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   	 * it avoids problems around wraparound.
>   	 */
>   
> -	next_pgt_ptr = fixup_pointer(&next_early_pgt, physaddr);
> -	early_dynamic_pgts_ptr = fixup_pointer(early_dynamic_pgts, physaddr);
> +	early_dynamic_pgts_ptr = RIP_RELATIVE_ADDR(early_dynamic_pgts);
> +	next_pgt_ptr = RIP_RELATIVE_ADDR(next_early_pgt);
>   	pud = (pudval_t *) early_dynamic_pgts_ptr[(*next_pgt_ptr)++];
>   	pmd = (pmdval_t *) early_dynamic_pgts_ptr[(*next_pgt_ptr)++];
>   
> @@ -272,7 +257,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   
>   	pmd_entry = __PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL;
>   	/* Filter out unsupported __PAGE_KERNEL_* bits: */
> -	mask_ptr = fixup_pointer(&__supported_pte_mask, physaddr);
> +	mask_ptr = RIP_RELATIVE_ADDR(__supported_pte_mask);
>   	pmd_entry &= *mask_ptr;
>   	pmd_entry += sme_me_mask_fixed_up;
>   	pmd_entry +=  physaddr;
> @@ -299,7 +284,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   	 * error, causing the BIOS to halt the system.
>   	 */
>   
> -	pmd = fixup_pointer(level2_kernel_pgt, physaddr);
> +	pmd = RIP_RELATIVE_ADDR(level2_kernel_pgt);
>   
>   	/* invalidate pages before the kernel image */
>   	for (i = 0; i < pmd_index((unsigned long)_text); i++)
> @@ -318,7 +303,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
>   	 * Fixup phys_base - remove the memory encryption mask to obtain
>   	 * the true physical address.
>   	 */
> -	*fixup_long(&phys_base, physaddr) += load_delta - sme_me_mask_fixed_up;
> +	*((unsigned long *)RIP_RELATIVE_ADDR(phys_base)) += load_delta - sme_me_mask_fixed_up;
>   
>   	return sme_postprocess_startup(bp, pmd);
>   }
> @@ -594,15 +579,15 @@ static void set_bringup_idt_handler(gate_desc *idt, int n, void *handler)
>   /* This runs while still in the direct mapping */
>   static void __head startup_64_load_idt(unsigned long physbase)
>   {
> -	struct desc_ptr *desc = fixup_pointer(&bringup_idt_descr, physbase);
> -	gate_desc *idt = fixup_pointer(bringup_idt_table, physbase);
> +	struct desc_ptr *desc = RIP_RELATIVE_ADDR(bringup_idt_descr);
> +	gate_desc *idt = RIP_RELATIVE_ADDR(bringup_idt_table);
>   
>   
>   	if (IS_ENABLED(CONFIG_AMD_MEM_ENCRYPT)) {
>   		void *handler;
>   
>   		/* VMM Communication Exception */
> -		handler = fixup_pointer(vc_no_ghcb, physbase);
> +		handler = RIP_RELATIVE_ADDR(vc_no_ghcb);
>   		set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
>   	}
>   
> @@ -629,7 +614,7 @@ void early_setup_idt(void)
>   void __head startup_64_setup_env(unsigned long physbase)
>   {
>   	/* Load GDT */
> -	startup_gdt_descr.address = (unsigned long)fixup_pointer(startup_gdt, physbase);
> +	startup_gdt_descr.address = (unsigned long)RIP_RELATIVE_ADDR(startup_gdt);
>   	native_load_gdt(&startup_gdt_descr);
>   
>   	/* New GDT is live - reload data segment registers */

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ