| Message ID | 57c59cae4141dd9601d7b4e9260030a16809b764.1729069025.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | FF-A VM to VM support | expand |
Hi Bertrand, On Wed, Oct 16, 2024 at 11:22 AM Bertrand Marquis <bertrand.marquis@arm.com> 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@arm.com> > --- > 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 >
Hi Jens, > On 1 Nov 2024, at 11:44, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > Hi Bertrand, > > On Wed, Oct 16, 2024 at 11:22 AM Bertrand Marquis > <bertrand.marquis@arm.com> 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@arm.com> >> --- >> 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. I do agree with the deterministic argument but I am not sure having the order forced is the right solution. Maybe we could use a command line argument so that one could select explicitly the tee: tee=ffa / tee=optee If we could prevent to modify the device tree that will probably make things easier. What do you think ? Cheers Bertrand > > 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
Hi Bertrand, On Mon, Nov 4, 2024 at 8:27 AM Bertrand Marquis <Bertrand.Marquis@arm.com> wrote: > > Hi Jens, > > > On 1 Nov 2024, at 11:44, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > > > Hi Bertrand, > > > > On Wed, Oct 16, 2024 at 11:22 AM Bertrand Marquis > > <bertrand.marquis@arm.com> 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@arm.com> > >> --- > >> 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. > > I do agree with the deterministic argument but I am not sure having > the order forced is the right solution. CONFIG_FFA_VM_TO_VM ensures that FF-A probing always succeeds and claims the TEE configuration. Logically, this should be last after all other probing has been tried since it disables further probing. > > Maybe we could use a command line argument so that one could > select explicitly the tee: > tee=ffa / tee=optee > > If we could prevent to modify the device tree that will probably make > things easier. > > What do you think ? That works for me. Cheers, Jens > > Cheers > Bertrand > > > > > 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 > >
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 )
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@arm.com> --- 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(-)