| Message ID | 0f5cf498c3c511ed192e8b7b7d0f4987e00e5c0d.1742824138.git.bertrand.marquis@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | FF-A VM to VM support | expand |
Hi Bertrand, On 24/03/2025 13:53, Bertrand Marquis 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. tee/ and the callbacks associated are meant to be used for mediatiors. My current interpretation ist this is only meant to interpose between a guest and physical resources. Here you are extending the meaning to "virtual TEE". I am sort of ok with that but ... > If there is OP-TEE 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 OP-TEE could be non functional (if FF-A is > probed first) so it is not recommended to activate the configuration > option for such systems. ... this part is concerning me. You should be able to build with CONFIG_FFA_VM_TO_VM and still boot when OP-TEE is present on the system. This is not too critical now as this is tech preview but this is definitely a blocker for making FFA supported. Can this be mentioned at the top of the ffa.c file (which already contains existing blocker)? Also, given this would expose a fully virtual TEE, we should be able to have a system where you have some VMs talking FFA and some using the physical OPTEE (or another TEE). Whether we want to support it is a different question but this design would prevent it. Is this intended? Cheers,
On 26/03/2025 23:41, Julien Grall wrote: > Hi Bertrand, > > On 24/03/2025 13:53, Bertrand Marquis 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. > > tee/ and the callbacks associated are meant to be used for mediatiors. > My current interpretation ist this is only meant to interpose between a > guest and physical resources. Here you are extending the meaning to > "virtual TEE". I am sort of ok with that but ... > >> If there is OP-TEE 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 OP-TEE could be non functional (if FF-A is >> probed first) so it is not recommended to activate the configuration >> option for such systems. > > ... this part is concerning me. You should be able to build with > CONFIG_FFA_VM_TO_VM and still boot when OP-TEE is present on the system. > This is not too critical now as this is tech preview but this is > definitely a blocker for making FFA supported. Can this be mentioned at > the top of the ffa.c file (which already contains existing blocker)? Just to clarify, I meant without having the OP-TEE folks to force the TEE on the command line because this is a bit of a hack... Cheers,
Hi Julien, > On 27 Mar 2025, at 00:41, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 24/03/2025 13:53, Bertrand Marquis 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. > > tee/ and the callbacks associated are meant to be used for mediatiors. My current interpretation ist this is only meant to interpose between a guest and physical resources. Here you are extending the meaning to "virtual TEE". I am sort of ok with that but ... I see what you mean but FF-A will not only be used to communicate with TEE (even if the primary use case right now is this one, including have it in a VM instead of the secure world). > >> If there is OP-TEE 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 OP-TEE could be non functional (if FF-A is >> probed first) so it is not recommended to activate the configuration >> option for such systems. > > ... this part is concerning me. You should be able to build with CONFIG_FFA_VM_TO_VM and still boot when OP-TEE is present on the system. This is not too critical now as this is tech preview but this is definitely a blocker for making FFA supported. Can this be mentioned at the top of the ffa.c file (which already contains existing blocker)? OP-TEE supports FF-A and in fact should be switched to using it by default as it allows it to run in parallel of other TEEs in the secure world or have FF-A compliant SPs running on top of OP-TEE. More and more you will see FF-A popping up as a recommended (or required) part of the firmware features. So the only reason to use OP-TEE without FF-A is if you have an old OP-TEE in which case your firmware will not support FF-A and using it between VMs is probably not required. > > Also, given this would expose a fully virtual TEE, we should be able to have a system where you have some VMs talking FFA and some using the physical OPTEE (or another TEE). Whether we want to support it is a different question but this design would prevent it. Is this intended? Right now i would say this is definitely not something we need as part of the tech preview as anybody using this feature in Xen would use an OP-TEE with FF-A support. But from Xen point of view, we should (if we can) support running on old systems with an existing OP-TEE but still use FF-A between VMs. This has some consequences on how the tee mediator and FF-A support is designed and I will definitely give it some thoughts (primary idea would be to decouple FF-A with secure as mediator to FF-A between VMs somehow). For the review side of things, am I right to understand from your comments that this ok for now as tech-preview ? Cheers Bertrand > > Cheers, > > -- > Julien Grall >
Hi Bertrand, On 27/03/2025 08:37, Bertrand Marquis wrote: >> On 27 Mar 2025, at 00:41, Julien Grall <julien@xen.org> wrote: >> >> Hi Bertrand, >> >> On 24/03/2025 13:53, Bertrand Marquis 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. >> >> tee/ and the callbacks associated are meant to be used for mediatiors. My current interpretation ist this is only meant to interpose between a guest and physical resources. Here you are extending the meaning to "virtual TEE". I am sort of ok with that but ... > > I see what you mean but FF-A will not only be used to communicate with TEE (even if the primary use case right now is this one, including have it in a VM instead of the secure world). > >> >>> If there is OP-TEE 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 OP-TEE could be non functional (if FF-A is >>> probed first) so it is not recommended to activate the configuration >>> option for such systems. >> >> ... this part is concerning me. You should be able to build with CONFIG_FFA_VM_TO_VM and still boot when OP-TEE is present on the system. This is not too critical now as this is tech preview but this is definitely a blocker for making FFA supported. Can this be mentioned at the top of the ffa.c file (which already contains existing blocker)? > > OP-TEE supports FF-A and in fact should be switched to using it by default as it allows it to run in parallel of other TEEs in the secure world or have FF-A compliant SPs running on top of OP-TEE. > More and more you will see FF-A popping up as a recommended (or required) part of the firmware features. > > So the only reason to use OP-TEE without FF-A is if you have an old OP-TEE in which case your firmware will not support FF-A and using it between VMs is probably not required. Thanks for the clarification. I know we only support OP-TEE in Xen today, but do you know what will happen for the other TEEs? Will they adopt FF-A? > >> >> Also, given this would expose a fully virtual TEE, we should be able to have a system where you have some VMs talking FFA and some using the physical OPTEE (or another TEE). Whether we want to support it is a different question but this design would prevent it. Is this intended? > > Right now i would say this is definitely not something we need as part of the tech preview as anybody using this feature in Xen would use an OP-TEE with FF-A support. > But from Xen point of view, we should (if we can) support running on old systems with an existing OP-TEE but still use FF-A between VMs. > This has some consequences on how the tee mediator and FF-A support is designed and I will definitely give it some thoughts (primary idea would be to decouple FF-A with secure as mediator to FF-A between VMs somehow). I am not sure we need to decouple anything. Today, we can already specify the type of the TEE used by a given VM (see tee_type). But we are enforcing the TEE type match the one of the current mediator. So what if we allow multiple mediator to run and when the domain is initialized we select the correct medatior/ops for the VM? For simplification, we could even hardocode FF-A as the second mediator. > > For the review side of things, am I right to understand from your comments that this ok for now as tech-preview ? Yes I am happy with the approach for now. Cheers,
Hi Julien, > On 30 Mar 2025, at 23:38, Julien Grall <julien@xen.org> wrote: > > Hi Bertrand, > > On 27/03/2025 08:37, Bertrand Marquis wrote: >>> On 27 Mar 2025, at 00:41, Julien Grall <julien@xen.org> wrote: >>> >>> Hi Bertrand, >>> >>> On 24/03/2025 13:53, Bertrand Marquis 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. >>> >>> tee/ and the callbacks associated are meant to be used for mediatiors. My current interpretation ist this is only meant to interpose between a guest and physical resources. Here you are extending the meaning to "virtual TEE". I am sort of ok with that but ... >> I see what you mean but FF-A will not only be used to communicate with TEE (even if the primary use case right now is this one, including have it in a VM instead of the secure world). > > >> >>>> If there is OP-TEE 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 OP-TEE could be non functional (if FF-A is >>>> probed first) so it is not recommended to activate the configuration >>>> option for such systems. >>> >>> ... this part is concerning me. You should be able to build with CONFIG_FFA_VM_TO_VM and still boot when OP-TEE is present on the system. This is not too critical now as this is tech preview but this is definitely a blocker for making FFA supported. Can this be mentioned at the top of the ffa.c file (which already contains existing blocker)? >> OP-TEE supports FF-A and in fact should be switched to using it by default as it allows it to run in parallel of other TEEs in the secure world or have FF-A compliant SPs running on top of OP-TEE. >> More and more you will see FF-A popping up as a recommended (or required) part of the firmware features. >> So the only reason to use OP-TEE without FF-A is if you have an old OP-TEE in which case your firmware will not support FF-A and using it between VMs is probably not required. > > Thanks for the clarification. I know we only support OP-TEE in Xen today, but do you know what will happen for the other TEEs? Will they adopt FF-A? On Arm the idea is to make them adopt FF-A yes and it will be part of System Ready recommendations in the future. > >>> >>> Also, given this would expose a fully virtual TEE, we should be able to have a system where you have some VMs talking FFA and some using the physical OPTEE (or another TEE). Whether we want to support it is a different question but this design would prevent it. Is this intended? >> Right now i would say this is definitely not something we need as part of the tech preview as anybody using this feature in Xen would use an OP-TEE with FF-A support. >> But from Xen point of view, we should (if we can) support running on old systems with an existing OP-TEE but still use FF-A between VMs. >> This has some consequences on how the tee mediator and FF-A support is designed and I will definitely give it some thoughts (primary idea would be to decouple FF-A with secure as mediator to FF-A between VMs somehow). > > I am not sure we need to decouple anything. Today, we can already specify the type of the TEE used by a given VM (see tee_type). But we are enforcing the TEE type match the one of the current mediator. Yes for VMs this has to be specified explicitly, this was the idea behind the command line parameter for Xen to. > > So what if we allow multiple mediator to run and when the domain is initialized we select the correct medatior/ops for the VM? Right now a VM gets the mediator selected by configuration if it is available. I do not think we should make it automatic as there might be good reasons to not allow to access a TEE from some VMs. > > For simplification, we could even hardocode FF-A as the second mediator. That could be a long term solution yes but we definitely need to solve the access rights first. As long as VMs can use FF-A to communicate with any other VMs or TEEs, the current approach is the most secure one. > >> For the review side of things, am I right to understand from your comments that this ok for now as tech-preview ? > > Yes I am happy with the approach for now. Great thanks. Cheers Bertrand > > Cheers, > > -- > Julien Grall >
diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c index e41ab5f8ada6..0627625efe60 100644 --- a/xen/arch/arm/tee/ffa.c +++ b/xen/arch/arm/tee/ffa.c @@ -324,8 +324,9 @@ static int ffa_domain_init(struct domain *d) struct ffa_ctx *ctx; int ret; - if ( !ffa_fw_version ) + if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !ffa_fw_version ) return -ENODEV; + /* * 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 @@ -561,6 +562,15 @@ err_no_fw: bitmap_zero(ffa_fw_abi_supported, FFA_ABI_BITMAP_SIZE); printk(XENLOG_WARNING "ARM FF-A No firmware support\n"); + if ( IS_ENABLED(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; + } + return false; } diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c index f6df2f15bb00..86bef6b3b2ab 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_lo, - bitmap_hi); + if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled ) + return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags, + bitmap_lo, bitmap_hi); + + return FFA_RET_NOT_SUPPORTED; } int ffa_handle_notification_unbind(struct cpu_user_regs *regs) @@ -51,18 +47,14 @@ 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_lo, - bitmap_hi); + if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled ) + return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, bitmap_lo, + bitmap_hi); + + return FFA_RET_NOT_SUPPORTED; } void ffa_handle_notification_info_get(struct cpu_user_regs *regs) @@ -71,7 +63,7 @@ void ffa_handle_notification_info_get(struct cpu_user_regs *regs) struct ffa_ctx *ctx = d->arch.tee; bool notif_pending; - if ( !notif_enabled ) + if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled ) { ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED); return; @@ -108,7 +100,7 @@ void ffa_handle_notification_get(struct cpu_user_regs *regs) uint32_t w6 = 0; uint32_t w7 = 0; - if ( !notif_enabled ) + if ( !IS_ENABLED(CONFIG_FFA_VM_TO_VM) && !fw_notif_enabled ) { ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED); return; @@ -120,7 +112,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, @@ -177,15 +170,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 @@ -371,7 +363,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 @@ -402,41 +394,41 @@ 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; } @@ -447,6 +439,6 @@ void ffa_notif_domain_destroy(struct domain *d) * Call bitmap_destroy even if bitmap create failed as the SPMC will * return a DENIED error that we will ignore. */ - if ( notif_enabled ) + if ( fw_notif_enabled ) ffa_notification_bitmap_destroy(ffa_get_vm_id(d)); }