[xen staging] xen/arm: ffa: Fix version negotiation

[patchbot@xxxxxxx]

commit da07df55580368ad696f39d58a8c100a37f6cd13
Author:     Bertrand Marquis <bertrand.marquis@xxxxxxx>
AuthorDate: Wed Nov 27 17:07:35 2024 +0100
Commit:     Julien Grall <julien@xxxxxxx>
CommitDate: Mon Dec 16 21:00:25 2024 +0100

    xen/arm: ffa: Fix version negotiation
    
    Fix FFA version negotiation with the firmware to follow the
    specification guidance more closely (see FF-A Specification Version 1.1
    in chapter 13.2.1).
    When the firmware returns OK we can have several cases:
    - the version requested is accepted but the firmware supports a greater
      one in the same major.
    - the firmware supports a greater major version. It could still return
      OK even if the version requested is not accepted. Reject it.
    - the firmware supports a lower version. It will return OK and give that
      version. Check if we support it and use it or reject it if we do not.
    
    Adapt the code to:
    - reject any version lower than the one we support or not with the same
      major version
    - use the version returned if in our supported range (currently 1.1
      only)
    - use 1.1 if the version returned is greater.
    
    Also adapt the handling of version requests from VM:
    - use our version if same major but greater minor is requested
    - use requested version if same major but lower minor is requested
    - do not use if incompatible major is requested
    - always return our version without error to the requester
    
    [1] https://developer.arm.com/documentation/den0077/e/
    
    Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
    Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
---
 xen/arch/arm/tee/ffa.c         | 53 +++++++++++++++++++++++++++++++-----------
 xen/arch/arm/tee/ffa_private.h |  3 +++
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index ade6aaa6fd..c5dcb4fe24 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -148,13 +148,20 @@ static void handle_version(struct cpu_user_regs *regs)
     struct ffa_ctx *ctx = d->arch.tee;
     uint32_t vers = get_user_reg(regs, 1);
 
-    if ( vers < FFA_VERSION_1_1 )
-        vers = FFA_VERSION_1_0;
-    else
-        vers = FFA_VERSION_1_1;
-
-    ctx->guest_vers = vers;
-    ffa_set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
+    /*
+     * Guest will use the version it requested if it is our major and minor
+     * lower or equals to ours. If the minor is greater, our version will be
+     * used.
+     * In any case return our version to the caller.
+     */
+    if ( FFA_VERSION_MAJOR(vers) == FFA_MY_VERSION_MAJOR )
+    {
+        if ( FFA_VERSION_MINOR(vers) > FFA_MY_VERSION_MINOR )
+            ctx->guest_vers = FFA_MY_VERSION;
+        else
+            ctx->guest_vers = vers;
+    }
+    ffa_set_regs(regs, FFA_MY_VERSION, 0, 0, 0, 0, 0, 0, 0);
 }
 
 static void handle_msg_send_direct_req(struct cpu_user_regs *regs, uint32_t 
fid)
@@ -537,17 +544,39 @@ static bool ffa_probe(void)
         goto err_no_fw;
     }
 
-    if ( vers < FFA_MIN_SPMC_VERSION || vers > FFA_MY_VERSION )
+    /* Some sanity check in case we update the version we support */
+    BUILD_BUG_ON(FFA_MIN_SPMC_VERSION > FFA_MY_VERSION);
+    BUILD_BUG_ON(FFA_VERSION_MAJOR(FFA_MIN_SPMC_VERSION) !=
+                                   FFA_MY_VERSION_MAJOR);
+
+    major_vers = FFA_VERSION_MAJOR(vers);
+    minor_vers = FFA_VERSION_MINOR(vers);
+
+    if ( major_vers != FFA_MY_VERSION_MAJOR ||
+         minor_vers < FFA_VERSION_MINOR(FFA_MIN_SPMC_VERSION) )
     {
-        printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
+        printk(XENLOG_ERR "ffa: Incompatible firmware version %u.%u\n",
+               major_vers, minor_vers);
         goto err_no_fw;
     }
 
-    major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;
-    minor_vers = vers & FFA_VERSION_MINOR_MASK;
     printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
            major_vers, minor_vers);
 
+    /*
+     * If the call succeed and the version returned is higher or equal to
+     * the one Xen requested, the version requested by Xen will be the one
+     * used. If the version returned is lower but compatible with Xen, Xen
+     * will use that version instead.
+     * A version with a different major or lower than the minimum version
+     * we support is rejected before.
+     * See https://developer.arm.com/documentation/den0077/e/ chapter 13.2.1
+     */
+    if ( minor_vers > FFA_MY_VERSION_MINOR )
+        ffa_fw_version = FFA_MY_VERSION;
+    else
+        ffa_fw_version = vers;
+
     /*
      * At the moment domains must support the same features used by Xen.
      * TODO: Rework the code to allow domain to use a subset of the
@@ -563,8 +592,6 @@ static bool ffa_probe(void)
         }
     }
 
-    ffa_fw_version = vers;
-
     if ( !ffa_rxtx_init() )
     {
         printk(XENLOG_ERR "ffa: Error during RXTX buffer init\n");
diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 7c6b06f686..045d9c4a0b 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -35,6 +35,9 @@
 #define MAKE_FFA_VERSION(major, minor)  \
         ((((major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) | \
          ((minor) & FFA_VERSION_MINOR_MASK))
+#define FFA_VERSION_MAJOR(vers) (((vers) >> FFA_VERSION_MAJOR_SHIFT) & \
+                                 FFA_VERSION_MAJOR_MASK)
+#define FFA_VERSION_MINOR(vers) ((vers) & FFA_VERSION_MINOR_MASK)
 
 #define FFA_VERSION_1_0         MAKE_FFA_VERSION(1, 0)
 #define FFA_VERSION_1_1         MAKE_FFA_VERSION(1, 1)
--
generated by git-patchbot for /home/xen/git/xen.git#staging