Attachment 230823 Details for Bug 209821 – Updated edited version of the David's patch

[patch] Updated edited version of the David's patch

0001-efi-loader-Inline-copy_finish-function-in-amd64-tram.patch (text/plain), 6.44 KB, created by David Sebek on 2022-01-08 16:27:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Sebek

Created: 2022-01-08 16:27:31 UTC

Size: 6.44 KB

patch

obsolete

>From b24dec8ef88e3476d0eb3d1254dff2dcaa4e59c0 Mon Sep 17 00:00:00 2001
>From: David Sebek <dasebek@gmail.com>
>Date: Fri, 7 Jan 2022 15:18:49 -0500
>Subject: [PATCH] efi: loader: Inline copy_finish function in amd64 trampoline
>
>Instead of calling the efi_copy_finish function from amd64_tramp,
>include the copy instructions in the trampoline code itself.
>This avoids boot hangs and restarts in the cases when
>the efi_copy_finish code happens to be located at a memory
>location that is overwritten during the copy process.
>
>PR:	209821
>---
> stand/efi/loader/arch/amd64/amd64_tramp.S   | 59 ++++++++++++++-------
> stand/efi/loader/arch/amd64/elf64_freebsd.c | 18 ++++---
> stand/efi/loader/copy.c                     |  8 +++
> stand/efi/loader/loader_efi.h               |  1 +
> 4 files changed, 60 insertions(+), 26 deletions(-)
>
>diff --git a/stand/efi/loader/arch/amd64/amd64_tramp.S b/stand/efi/loader/arch/amd64/amd64_tramp.S
>index c102d924358..3ca98a7cf38 100644
>--- a/stand/efi/loader/arch/amd64/amd64_tramp.S
>+++ b/stand/efi/loader/arch/amd64/amd64_tramp.S
>@@ -1,9 +1,11 @@
> /*-
>  * Copyright (c) 2013 The FreeBSD Foundation
>+ * Copyright 2021 David Sebek <dasebek@gmail.com>
>  * All rights reserved.
>  *
>  * This software was developed by Benno Rice under sponsorship from
>  * the FreeBSD Foundation.
>+ *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>  * are met:
>@@ -31,34 +33,53 @@
> #include <machine/asmacros.h>
> 
> 	.text
>-	.globl	amd64_tramp
>+	.globl	amd64_tramp_inline
> 
> /*
>- * void amd64_tramp(uint64_t stack, void *copy_finish, uint64_t kernend,
>- *		    uint64_t modulep, uint64_t pagetable, uint64_t entry)
>+ * void amd64_tramp_inline(uint64_t stack %rdi, uint64_t kernend %rsi,
>+ *    uint64_t modulep %rdx, uint64_t pagetable %rcx, uint64_t entry %r8,
>+ *    uint64_t copy_dst %r9, uint64_t copy_src 8(%rsp),
>+ *    uint64_t copy_src_end 16(%rsp))
>  */
>-amd64_tramp:
>+amd64_tramp_inline:
> 	cli			/* Make sure we don't get interrupted. */
>-	movq	%rdi,%rsp	/* Switch to our temporary stack. */
> 
>-	movq	%rdx,%r12	/* Stash the kernel values for later. */
>-	movq	%rcx,%r13
>-	movq	%r8,%r14
>-	movq	%r9,%r15
>+	/*
>+	 * Copy the kernel from the staging area to the expected location
>+	 * in memory. The following code is equivalent to the efi_copy_finish
>+	 * function that amd64_tramp used to call. Inlining this code avoids
>+	 * a scenario when the system froze because efi_copy_finish
>+	 * overwrote its own code that just happened to be located somewhere
>+	 * in the destination range.
>+	 *
>+	 * while (copy_src < copy_src_end) *copy_dst++ = *copy_src++;
>+	 */
>+	movq	8(%rsp), %rax	/* rax = copy_src */
>+	movq	16(%rsp), %r10	/* r10 = copy_src_end */
>+	cmpq	%r10, %rax
>+	jnb	copy_done
>+	subq	%rax, %r9	/* r9 = copy_dst - copy_src */
>+loop:
>+	movq	(%rax), %r11
>+	movq	%r11, (%rax,%r9)
>+	addq	$8, %rax
>+	cmpq	%rax, %r10
>+	ja	loop
>+copy_done:
> 
>-	callq	*%rsi		/* Call copy_finish so we're all ready to go. */
>+	movq	%rdi,%rsp	/* Switch to our temporary stack. */
> 
>-	pushq	%r12		/* Push kernend. */
>-	salq	$32,%r13	/* Shift modulep and push it. */
>-	pushq	%r13
>-	pushq	%r15		/* Push the entry address. */
>-	movq	%r14,%cr3	/* Switch page tables. */
>+	pushq	%rsi		/* Push kernend. */
>+	salq	$32,%rdx	/* Shift modulep and push it. */
>+	pushq	%rdx
>+	pushq	%r8		/* Push the entry address. */
>+	movq	%rcx,%cr3	/* Switch page tables. */
> 	ret			/* "Return" to kernel entry. */
> 
> 	ALIGN_TEXT
>-amd64_tramp_end:
>+amd64_tramp_inline_end:
> 
> 	.data
>-	.globl	amd64_tramp_size
>-amd64_tramp_size:
>-	.long	amd64_tramp_end-amd64_tramp
>+	.globl	amd64_tramp_inline_size
>+amd64_tramp_inline_size:
>+	.long	amd64_tramp_inline_end-amd64_tramp_inline
>diff --git a/stand/efi/loader/arch/amd64/elf64_freebsd.c b/stand/efi/loader/arch/amd64/elf64_freebsd.c
>index 896041e066c..6fcd561cb2f 100644
>--- a/stand/efi/loader/arch/amd64/elf64_freebsd.c
>+++ b/stand/efi/loader/arch/amd64/elf64_freebsd.c
>@@ -78,11 +78,12 @@ static pml4_entry_t *PT4;
> static pdp_entry_t *PT3;
> static pd_entry_t *PT2;
> 
>-static void (*trampoline)(uint64_t stack, void *copy_finish, uint64_t kernend,
>-    uint64_t modulep, pml4_entry_t *pagetable, uint64_t entry);
>+static void (*trampoline)(uint64_t stack, uint64_t kernend,
>+    uint64_t modulep, pml4_entry_t *pagetable, uint64_t entry,
>+    uint64_t copy_dst, uint64_t copy_src, uint64_t copy_src_end);
> 
>-extern uintptr_t amd64_tramp;
>-extern uint32_t amd64_tramp_size;
>+extern uintptr_t amd64_tramp_inline;
>+extern uint32_t amd64_tramp_inline_size;
> 
> /*
>  * There is an ELF kernel and one or more ELF modules loaded.
>@@ -95,6 +96,7 @@ elf64_exec(struct preloaded_file *fp)
> 	struct file_metadata	*md;
> 	Elf_Ehdr 		*ehdr;
> 	vm_offset_t		modulep, kernend, trampcode, trampstack;
>+	uint64_t		copy_dst, copy_src, copy_src_end;
> 	int			err, i;
> 	ACPI_TABLE_RSDP		*rsdp;
> 	char			buf[24];
>@@ -153,7 +155,7 @@ elf64_exec(struct preloaded_file *fp)
> 	    (EFI_PHYSICAL_ADDRESS *)&trampcode);
> 	bzero((void *)trampcode, EFI_PAGE_SIZE);
> 	trampstack = trampcode + EFI_PAGE_SIZE - 8;
>-	bcopy((void *)&amd64_tramp, (void *)trampcode, amd64_tramp_size);
>+	bcopy((void *)&amd64_tramp_inline, (void *)trampcode, amd64_tramp_inline_size);
> 	trampoline = (void *)trampcode;
> 
> 	PT4 = (pml4_entry_t *)0x0000000040000000;
>@@ -194,8 +196,10 @@ elf64_exec(struct preloaded_file *fp)
> 
> 	dev_cleanup();
> 
>-	trampoline(trampstack, efi_copy_finish, kernend, modulep, PT4,
>-	    ehdr->e_entry);
>+	efi_copy_get_locations(&copy_dst, &copy_src, &copy_src_end);
>+
>+	trampoline(trampstack, kernend, modulep, PT4,
>+	    ehdr->e_entry, copy_dst, copy_src, copy_src_end);
> 
> 	panic("exec returned");
> }
>diff --git a/stand/efi/loader/copy.c b/stand/efi/loader/copy.c
>index 5a174dbf51e..307ad3e0ac7 100644
>--- a/stand/efi/loader/copy.c
>+++ b/stand/efi/loader/copy.c
>@@ -361,3 +361,11 @@ efi_copy_finish(void)
> 	while (src < last)
> 		*dst++ = *src++;
> }
>+
>+void
>+efi_copy_get_locations(uint64_t *dst, uint64_t *src, uint64_t *src_end)
>+{
>+	*src = (uint64_t)staging;
>+	*dst = (uint64_t)(staging - stage_offset);
>+	*src_end = (uint64_t)staging_end;
>+}
>diff --git a/stand/efi/loader/loader_efi.h b/stand/efi/loader/loader_efi.h
>index 4d077514e42..5f0c7c6825a 100644
>--- a/stand/efi/loader/loader_efi.h
>+++ b/stand/efi/loader/loader_efi.h
>@@ -44,5 +44,6 @@ ssize_t	efi_readin(readin_handle_t fd, vm_offset_t dest, const size_t len);
> void * efi_translate(vm_offset_t ptr);
> 
> void	efi_copy_finish(void);
>+void	efi_copy_get_locations(uint64_t *dst, uint64_t *src, uint64_t *src_end);
> 
> #endif	/* _LOADER_EFI_COPY_H_ */
>-- 
>2.33.1
>

Actions: View | Diff

Attachments on bug 209821: 225333 | 225480 | 230798 | 230812 | 230823