[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 5/5] xen/arm: ffa: Enable VM to VM without firmware


  • To: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Mon, 24 Mar 2025 13:06:56 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=arm.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=KiIaM187TRCxqeo1kPMjwQeuuwvWBXXGXPGVEBLaDsE=; b=N4ANWwWdknCGfjeTl22AbbVzwo+1pO0603duaOeNi51nG65eYLFPBA/wgHckJmcTG4KV8osCt/+OjVMrvsRVJc76q+1a/z2aaIaNjVUsAVd00eD9EIrIoNUgh3Yq7yXRUZNbhayjOb2bGumj0lU28cZacAkKqIpVlT+XBtQzv4p+DIGqxaenEGq0WXuyQ1TCyQOeKwR4vMkX2dH/mT4aWq3aqxj3VHTnA02+K6S1OURu5Y34ayQfJpb2W3YTpjsJ/qfja2IpIK+tBst0wQl0BcBnf3tWwEIRz3rTOnWVS+j8M25DPKVhxDAfEdiSGtPHQMcBM0jXlS/WrB2tuW8YVA==
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=KiIaM187TRCxqeo1kPMjwQeuuwvWBXXGXPGVEBLaDsE=; b=SKJ0tS7CSKPs4z7Mqm+kp1lOfwQ4Y1jqdmCMTHFoM+CKwqNFYdu4m9bBJ7xz5VTCVV8F3/6FLgDmncrGR2n75Gd+HtvKryG+LlbuW8tNg7sJhu15fHQof2LcfuRN/4+SEhMNUC9eErGUUgQAUQIZUBjCpVCeK63e9NvVgqjqwYsW8Tv//Qj5gpiCM2DXv5APhYiUGix7i9cOCMMWR0Zn60ZTkoKcVn9sAbnQOs1ruvEmra8g/gOe0t2hW4mIP0jh6kkkXUAg6NfQpQEBo8MukTEK3JsrEYsNa0/ClRQv1sin+v7iPR6Lu2F992r4JEXo4ZMz/BUVns73PiXE1D2FIQ==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=pass; b=pQa9vRb8SnCoQZ3dm1/F5eMrNhohMDlDpKhqGP1+Q49cdikKtiakWxp2muPHeZK+RE7v77jPq1xgoxJkGjupcrI0AgBXbPkiYgx42SBJBLJFIq/VXOcN9zmRsRBvqGHgmkhbtPKnofObCcaUoUlgSXyyBsQComujmMkB8/EzgFTjadyvHGHF7Z5lt5PPDGYaqmrqg/kBHw2zdBjQ55Nbr3lUP86c7xWD24jOX/tp8DksSaotjWG4Eodp3i8bogEnHMSrGQXSLZTy42/zvAzEwcPTHiW3JfYYrlDnbWkB4r8vmFlduRg9ZYVBj9j6CdM+03UohaAJ/UQjkLagLJ7/Xg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=tBCRHLmPVECtm0sVMqH/SEa+RlrvuXS44G0fOftLm72aeXJs7Q0FJSgYc0arvm+JetuUfXphfOFSZyCY2YL2+ia+6YOj6/dn5P0ga8bJiHSwbxyGTamzcnfexkOiZvwYAToRricFnGyNUerf2HmufJSIiNXL2KsIwBKoSxP9J63eJBeI1EDTxT2rlHk1YxKQTLcroUmD1DTcS8x4J3ehe6yNEmEE1Sh0rbbn+Xrnv0LKjNQgQh0SFRX84FaM47nXbXZRUrirmInSptg4O6tuneWJLMAKWbWOoZWBiUVteVNw14UqYg8By9EqNRyO77ASOD06ZxkZDqplvUd0eQbVfg==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>
  • Delivery-date: Mon, 24 Mar 2025 13:07:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHbnJ1P36ESPN6dS0aOqSpHOpyt2LOCGzMAgAAmwwA=
  • Thread-topic: [PATCH v3 5/5] xen/arm: ffa: Enable VM to VM without firmware

Hi Jens,

> On 24 Mar 2025, at 11:48, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Hi Bertrand,
> 
> On Mon, Mar 24, 2025 at 10:15 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 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 Optee could be non functional (if FF-A is
> 
> Prefer OP-TEE

Right i fixed the first instance but not the second one.
Will fix in the next version.

> 
>> 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 no
>> firmware, rework the notification handling and modify the global flag to
>> only be used as check for firmware notification support instead.
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
>> ---
>> Changes in v3:
>> - fix typos in commit message
>> - add spaces around <<
>> - move notification id fix back into buffer full patch
>> - fix | position in if
>> Changes in v2:
>> - replace ifdef with IS_ENABLED when possible
>> ---
>> xen/arch/arm/tee/ffa.c       |  12 +++-
>> xen/arch/arm/tee/ffa_notif.c | 104 ++++++++++++++++-------------------
>> 2 files changed, 59 insertions(+), 57 deletions(-)
> 
> Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Thanks
Bertrand

> 
> Cheers,
> Jens
> 
>> 
>> 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));
>> }
>> --
>> 2.47.1



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.