Re: [kvm-unit-tests PATCH v2 17/18] arm64: efi: Switch to our own stack

Nikos Nikoleris
2024-03-04T09:03:37Z

On 27/02/2024 19:21, Andrew Jones wrote:
> We don't want to map EFI_BOOT_SERVICES_DATA regions, so move the
> stack from its EFI_BOOT_SERVICES_DATA region to EFI_LOADER_CODE,
> which we always map. We'll still map the stack as R/W instead of
> R/X because we split EFI_LOADER_CODE regions on the _etext boundary
> and map addresses before _etext as R/X and the rest as R/W.
>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

Thanks,

Nikos

> ---
> arm/efi/crt0-efi-aarch64.S | 22 +++++++++++++++++-----
> lib/arm/setup.c | 4 ----
> lib/memregions.c | 6 ------
> 3 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/arm/efi/crt0-efi-aarch64.S b/arm/efi/crt0-efi-aarch64.S
> index 5fd3dc94dae8..71ce2794f059 100644
> --- a/arm/efi/crt0-efi-aarch64.S
> +++ b/arm/efi/crt0-efi-aarch64.S
> @@ -125,12 +125,18 @@ _start:
>
> ldp x0, x1, [sp, #16]
>
> - /* Align sp; this is necessary due to way we store cpu0's thread_info */
> + /*
> + * Switch to our own stack and align sp; this is necessary due
> + * to way we store cpu0's thread_info
> + */
> + adrp x2, stacktop
> + add x2, x2, :lo12:stacktop
> + and x2, x2, #THREAD_MASK
> + mov x3, sp
> + mov sp, x2
> + stp xzr, xzr, [sp, #-16]!
> mov x29, sp
> - mov x30, sp
> - and x30, x30, #THREAD_MASK
> - mov sp, x30
> - str x29, [sp, #-16]!
> + str x3, [sp, #-16]!
>
> bl efi_main
>
> @@ -140,3 +146,9 @@ _start:
>
> 0: ldp x29, x30, [sp], #32
> ret
> +
> + .section .data
> +
> +.balign 65536
> +.space 65536
> +stacktop:
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 08658b9a222b..d535cec88709 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -340,10 +340,6 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
> {
> efi_status_t status;
>
> - struct thread_info *ti = current_thread_info();
> -
> - memset(ti, 0, sizeof(*ti));
> -
> exceptions_init();
>
> memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> diff --git a/lib/memregions.c b/lib/memregions.c
> index 3c6f751eb4f2..53fc0c7cfc58 100644
> --- a/lib/memregions.c
> +++ b/lib/memregions.c
> @@ -114,12 +114,6 @@ void memregions_efi_init(struct efi_boot_memmap *mem_map,
> break;
> case EFI_LOADER_DATA:
> break;
> - case EFI_BOOT_SERVICES_DATA:
> - /*
> - * FIXME: This would ideally be MR_F_RESERVED, but the
> - * primary stack is in a region of this EFI type.
> - */
> - break;
> case EFI_PERSISTENT_MEMORY:
> r.flags = MR_F_PERSISTENT;
> break;
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.