|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC PATCH 4/4] xen/arm: ffa: Enable VM to VM without firmware
Hi Bertrand,
On Wed, Oct 16, 2024 at 11:22 AM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> When VM to VM support is activated and there is no suitable FF-A support
> in the firmware, enable FF-A support for VMs to allow using it for VM to
> VM communications.
> If there is Optee running in the secure world and using the non FF-A
> communication system, having CONFIG_FFA_VM_TO_VM could be non functional
> (if optee is probed first) or Optee could be non functional (if FF-A is
> probed first) so it is not recommended to activate the configuration
> option for such systems.
>
> To make buffer full notification work between VMs when there is not
> firmware, rework the notification handling and modify the global flag to
> only be used as check for firmware notification support instead.
>
> Modify part_info_get to return the list of VMs when there is no firmware
> support.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
> xen/arch/arm/tee/ffa.c | 11 +++
> xen/arch/arm/tee/ffa_notif.c | 118 ++++++++++++++++----------------
> xen/arch/arm/tee/ffa_partinfo.c | 2 +
> 3 files changed, 73 insertions(+), 58 deletions(-)
I think it is desirable or at least a good goal to be able to have all
TEE configurations enabled at compile time.
For optee_probe() to succeed, a DT node with compatible
"linaro,optee-tz" must be present, and a trusted OS responding with
the UUID used by OP-TEE. False positives can be ruled out unless the
system is grossly misconfigured and shouldn't be used.
If we could make the probe order deterministic with OP-TEE before
FF-A, we should be OK. If there is an odd system with OP-TEE SMC ABI
in the secure world that wants to use FF-A VM to VM, removing the
"linaro,optee-tz" compatible node from DT is enough to disable
optee_probe() without recompiling Xen.
Cheers,
Jens
>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 21d41b452dc9..6d427864f3da 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -324,8 +324,11 @@ static int ffa_domain_init(struct domain *d)
> struct ffa_ctx *ctx;
> int ret;
>
> +#ifndef CONFIG_FFA_VM_TO_VM
> if ( !ffa_fw_version )
> return -ENODEV;
> +#endif
> +
> /*
> * We are using the domain_id + 1 as the FF-A ID for VMs as FF-A ID 0 is
> * reserved for the hypervisor and we only support secure endpoints using
> @@ -549,7 +552,15 @@ err_no_fw:
> bitmap_zero(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE);
> printk(XENLOG_WARNING "ARM FF-A No firmware support\n");
>
> +#ifdef CONFIG_FFA_VM_TO_VM
> + INIT_LIST_HEAD(&ffa_teardown_head);
> + init_timer(&ffa_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
> +
> + printk(XENLOG_INFO "ARM FF-A only available between VMs\n");
> + return true;
> +#else
> return false;
> +#endif
> }
>
> static const struct tee_mediator_ops ffa_ops =
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index 052b3e364a70..f2c87d1320de 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -16,7 +16,7 @@
>
> #include "ffa_private.h"
>
> -static bool __ro_after_init notif_enabled;
> +static bool __ro_after_init fw_notif_enabled;
> static unsigned int __ro_after_init notif_sri_irq;
>
> int ffa_handle_notification_bind(struct cpu_user_regs *regs)
> @@ -27,21 +27,17 @@ int ffa_handle_notification_bind(struct cpu_user_regs
> *regs)
> uint32_t bitmap_lo = get_user_reg(regs, 3);
> uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> - if ( !notif_enabled )
> - return FFA_RET_NOT_SUPPORTED;
> -
> if ( (src_dst & 0xFFFFU) != ffa_get_vm_id(d) )
> return FFA_RET_INVALID_PARAMETERS;
>
> if ( flags ) /* Only global notifications are supported */
> return FFA_RET_DENIED;
>
> - /*
> - * We only support notifications from SP so no need to check the sender
> - * endpoint ID, the SPMC will take care of that for us.
> - */
> - return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, bitmap_hi,
> - bitmap_lo);
> + if ( FFA_ID_IS_SECURE(src_dst>>16) && fw_notif_enabled )
> + return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags,
> + bitmap_hi, bitmap_lo);
> +
> + return FFA_RET_NOT_SUPPORTED;
> }
>
> int ffa_handle_notification_unbind(struct cpu_user_regs *regs)
> @@ -51,32 +47,36 @@ int ffa_handle_notification_unbind(struct cpu_user_regs
> *regs)
> uint32_t bitmap_lo = get_user_reg(regs, 3);
> uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> - if ( !notif_enabled )
> - return FFA_RET_NOT_SUPPORTED;
> -
> if ( (src_dst & 0xFFFFU) != ffa_get_vm_id(d) )
> return FFA_RET_INVALID_PARAMETERS;
>
> - /*
> - * We only support notifications from SP so no need to check the
> - * destination endpoint ID, the SPMC will take care of that for us.
> - */
> - return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_hi,
> - bitmap_lo);
> + if ( FFA_ID_IS_SECURE(src_dst>>16) && fw_notif_enabled )
> + return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0,
> bitmap_hi,
> + bitmap_lo);
> +
> + return FFA_RET_NOT_SUPPORTED;
> }
>
> void ffa_handle_notification_info_get(struct cpu_user_regs *regs)
> {
> struct domain *d = current->domain;
> struct ffa_ctx *ctx = d->arch.tee;
> + bool notif_pending = false;
>
> - if ( !notif_enabled )
> +#ifndef CONFIG_FFA_VM_TO_VM
> + if ( !fw_notif_enabled )
> {
> ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> return;
> }
> +#endif
>
> - if ( test_and_clear_bool(ctx->notif.secure_pending) )
> + notif_pending = ctx->notif.secure_pending;
> +#ifdef CONFIG_FFA_VM_TO_VM
> + notif_pending |= ctx->notif.buff_full_pending;
> +#endif
> +
> + if ( notif_pending )
> {
> /* A pending global notification for the guest */
> ffa_set_regs(regs, FFA_SUCCESS_64, 0,
> @@ -103,11 +103,13 @@ void ffa_handle_notification_get(struct cpu_user_regs
> *regs)
> uint32_t w6 = 0;
> uint32_t w7 = 0;
>
> - if ( !notif_enabled )
> +#ifndef CONFIG_FFA_VM_TO_VM
> + if ( !fw_notif_enabled )
> {
> ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> return;
> }
> +#endif
>
> if ( (recv & 0xFFFFU) != ffa_get_vm_id(d) )
> {
> @@ -115,7 +117,8 @@ void ffa_handle_notification_get(struct cpu_user_regs
> *regs)
> return;
> }
>
> - if ( flags & ( FFA_NOTIF_FLAG_BITMAP_SP | FFA_NOTIF_FLAG_BITMAP_SPM ) )
> + if ( fw_notif_enabled && (flags & ( FFA_NOTIF_FLAG_BITMAP_SP
> + | FFA_NOTIF_FLAG_BITMAP_SPM )) )
> {
> struct arm_smccc_1_2_regs arg = {
> .a0 = FFA_NOTIFICATION_GET,
> @@ -170,15 +173,14 @@ int ffa_handle_notification_set(struct cpu_user_regs
> *regs)
> uint32_t bitmap_lo = get_user_reg(regs, 3);
> uint32_t bitmap_hi = get_user_reg(regs, 4);
>
> - if ( !notif_enabled )
> - return FFA_RET_NOT_SUPPORTED;
> -
> if ( (src_dst >> 16) != ffa_get_vm_id(d) )
> return FFA_RET_INVALID_PARAMETERS;
>
> - /* Let the SPMC check the destination of the notification */
> - return ffa_simple_call(FFA_NOTIFICATION_SET, src_dst, flags, bitmap_lo,
> - bitmap_hi);
> + if ( FFA_ID_IS_SECURE(src_dst>>16) && fw_notif_enabled )
> + return ffa_simple_call(FFA_NOTIFICATION_SET, src_dst, flags,
> bitmap_lo,
> + bitmap_hi);
> +
> + return FFA_RET_NOT_SUPPORTED;
> }
>
> #ifdef CONFIG_FFA_VM_TO_VM
> @@ -190,7 +192,7 @@ void ffa_raise_rx_buffer_full(struct domain *d)
> return;
>
> if ( !test_and_set_bool(ctx->notif.buff_full_pending) )
> - vgic_inject_irq(d, d->vcpu[0], notif_sri_irq, true);
> + vgic_inject_irq(d, d->vcpu[0], GUEST_FFA_NOTIF_PEND_INTR_ID, true);
> }
> #endif
>
> @@ -363,7 +365,7 @@ void ffa_notif_init_interrupt(void)
> {
> int ret;
>
> - if ( notif_enabled && notif_sri_irq < NR_GIC_SGI )
> + if ( fw_notif_enabled && notif_sri_irq < NR_GIC_SGI )
> {
> /*
> * An error here is unlikely since the primary CPU has already
> @@ -394,47 +396,47 @@ void ffa_notif_init(void)
> int ret;
>
> /* Only enable fw notification if all ABIs we need are supported */
> - if ( !(ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_CREATE) &&
> - ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_DESTROY) &&
> - ffa_fw_supports_fid(FFA_NOTIFICATION_GET) &&
> - ffa_fw_supports_fid(FFA_NOTIFICATION_INFO_GET_64)) )
> - return;
> -
> - arm_smccc_1_2_smc(&arg, &resp);
> - if ( resp.a0 != FFA_SUCCESS_32 )
> - return;
> -
> - irq = resp.a2;
> - notif_sri_irq = irq;
> - if ( irq >= NR_GIC_SGI )
> - irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
> - ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
> - if ( ret )
> + if ( ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_CREATE) &&
> + ffa_fw_supports_fid(FFA_NOTIFICATION_BITMAP_DESTROY) &&
> + ffa_fw_supports_fid(FFA_NOTIFICATION_GET) &&
> + ffa_fw_supports_fid(FFA_NOTIFICATION_INFO_GET_64) )
> {
> - printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
> - irq, ret);
> - return;
> - }
> + arm_smccc_1_2_smc(&arg, &resp);
> + if ( resp.a0 != FFA_SUCCESS_32 )
> + return;
>
> - notif_enabled = true;
> + irq = resp.a2;
> + notif_sri_irq = irq;
> + if ( irq >= NR_GIC_SGI )
> + irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
> + ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);
> + if ( ret )
> + {
> + printk(XENLOG_ERR "ffa: request_irq irq %u failed: error %d\n",
> + irq, ret);
> + return;
> + }
> + fw_notif_enabled = true;
> + }
> }
>
> int ffa_notif_domain_init(struct domain *d)
> {
> int32_t res;
>
> - if ( !notif_enabled )
> - return 0;
> + if ( fw_notif_enabled )
> + {
>
> - res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
> - if ( res )
> - return -ENOMEM;
> + res = ffa_notification_bitmap_create(ffa_get_vm_id(d), d->max_vcpus);
> + if ( res )
> + return -ENOMEM;
> + }
>
> return 0;
> }
>
> void ffa_notif_domain_destroy(struct domain *d)
> {
> - if ( notif_enabled )
> + if ( fw_notif_enabled )
> ffa_notification_bitmap_destroy(ffa_get_vm_id(d));
> }
> diff --git a/xen/arch/arm/tee/ffa_partinfo.c b/xen/arch/arm/tee/ffa_partinfo.c
> index d699a267cc76..2e09440fe6c2 100644
> --- a/xen/arch/arm/tee/ffa_partinfo.c
> +++ b/xen/arch/arm/tee/ffa_partinfo.c
> @@ -128,12 +128,14 @@ void ffa_handle_partition_info_get(struct cpu_user_regs
> *regs)
> goto out;
> }
>
> +#ifndef CONFIG_FFA_VM_TO_VM
> if ( !ffa_fw_supports_fid(FFA_PARTITION_INFO_GET) )
> {
> /* Just give an empty partition list to the caller */
> ret = FFA_RET_OK;
> goto out;
> }
> +#endif
>
> ret = ffa_rx_acquire(d);
> if ( ret != FFA_RET_OK )
> --
> 2.47.0
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |