Re: [PATCH v8 04/10] arm_ffa: efi: unmap RX/TX buffers

Simon Glass
2022-11-23T02:12:34Z

Hi Abdellatif,

On Tue, 22 Nov 2022 at 06:18, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> unmap RX/TX buffers at ExitBootServices()
>
> Unmapping the RX/TX buffers created by u-boot is needed before EFI
> runtime.

U-Boot

>
> At EFI runtime the linux kernel takes care of allocating its own RX/TX
> buffers and registering them with the secure world.
>
> Secure world should be using the RX/TX buffers created by the kernel.
> So, RX/TX buffers created by u-boot must be unmapped.

U-Boot

>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
>
> ---
>
> Changelog:
> ===============
>
> v8: pass NULL device pointer to the FF-A bus operation
> v7: replace debug() by log_err()
>
> lib/efi_loader/efi_boottime.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 253f9f75ef..8949aca250 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -3,6 +3,9 @@
> * EFI application boot time services
> *
> * Copyright (c) 2016 Alexander Graf
> + *
> + * (C) Copyright 2022 ARM Limited
> + * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> */
>
> #include <common.h>
> @@ -23,6 +26,10 @@
> #include <asm/setjmp.h>
> #include <linux/libfdt_env.h>
>
> +#if CONFIG_IS_ENABLED(ARM_FFA_TRANSPORT)
> +#include <arm_ffa.h>
> +#endif
> +

Can we include it always?

> DECLARE_GLOBAL_DATA_PTR;
>
> /* Task priority level */
> @@ -2178,6 +2185,12 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
> dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
> }
>
> +#if CONFIG_IS_ENABLED(ARM_FFA_TRANSPORT)

Can this use if() ?

I am not sure what this is being done in EFI code, though. We can use
device_remove(...DM_REMOVE_ACTIVE_ALL) which is designed to handle
this.

> + /* unmap FF-A RX/TX buffers */
> + if (ffa_bus_ops_get()->rxtx_unmap(NULL))
> + log_err("Can't unmap FF-A RX/TX buffers\n");
> +#endif
> +
> /* Patch out unsupported runtime function */
> efi_runtime_detach();
>
> --
> 2.17.1
>

Regards,
SImon