Hi Jonathan,

On Tue, 17 Mar 2026 at 17:40, Jonathan Cameron
[off-list ref] wrote:

On Mon, 16 Mar 2026 17:35:24 +0000
Fuad Tabba [off-list ref] wrote:

quoted

Migrate manual hyp_spin_lock() and hyp_spin_unlock() calls managing
host_buffers.lock and version_lock to use the guard(hyp_spinlock)
macro.

This eliminates manual unlock calls on return paths and simplifies
error handling by replacing goto labels with direct returns.

Change-Id: I52e31c0bed3d2772c800a535af8abdabd81a178b
Signed-off-by: Fuad Tabba <redacted>

See below and read the cleanup.h guidance notes on usage at the top.
Specifically:

 * Lastly, given that the benefit of cleanup helpers is removal of
 * "goto", and that the "goto" statement can jump between scopes, the
 * expectation is that usage of "goto" and cleanup helpers is never
 * mixed in the same function. I.e. for a given routine, convert all
 * resources that need a "goto" cleanup to scope-based cleanup, or
 * convert none of them.

Doing otherwise might mean an encounter with a grumpy Linus :)

I should have read the cleanup.h guidance more closely, thanks for
pointing this out.

quoted

---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 86 +++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 48 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 94161ea1cd60..0c772501c3ba 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c

@@ -239,6 +239,8 @@ static void do_ffa_rxtx_map(struct arm_smccc_1_2_regs *res,
      int ret = 0;
      void *rx_virt, *tx_virt;

+     guard(hyp_spinlock)(&host_buffers.lock);
+
      if (npages != (KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE) / FFA_PAGE_SIZE) {
              ret = FFA_RET_INVALID_PARAMETERS;
              goto out;

@@ -249,10 +251,9 @@ static void do_ffa_rxtx_map(struct arm_smccc_1_2_regs *res,
              goto out;
      }

-     hyp_spin_lock(&host_buffers.lock);
      if (host_buffers.tx) {
              ret = FFA_RET_DENIED;
-             goto out_unlock;
+             goto out;

Whilst it isn't a bug as such because you increased the scope to avoid it,
there is some pretty strong guidance in cleanup.h about not using guard() or
any of the other magic if a function that also contains gotos.  That came from
some views Linus expressed pretty clearly about the dangers that brings (the
problem is a goto that crosses a guard() being defined and the risk of
refactors that happen to end up with that + general view that cleanup.h
stuff is about removing gotos, so if you still have them, why bother!

You can usually end up with the best of all worlds via some refactors
to pull out helper functions, but that's a lot of churn.

If I respin this series (depending on whether the maintainers think
it's worth the hassle), I'll remove all changes that just add churn.

Thanks for having a look at this and the other ones.

Cheers,
/fuad

Jonathan

quoted

      }

      /*

@@ -261,7 +262,7 @@ static void do_ffa_rxtx_map(struct arm_smccc_1_2_regs *res,
       */
      ret = ffa_map_hyp_buffers(npages);
      if (ret)
-             goto out_unlock;
+             goto out;

      ret = __pkvm_host_share_hyp(hyp_phys_to_pfn(tx));
      if (ret) {

@@ -292,8 +293,6 @@ static void do_ffa_rxtx_map(struct arm_smccc_1_2_regs *res,
      host_buffers.tx = tx_virt;
      host_buffers.rx = rx_virt;

-out_unlock:
-     hyp_spin_unlock(&host_buffers.lock);
 out:
      ffa_to_smccc_res(res, ret);
      return;

@@ -306,7 +305,7 @@ static void do_ffa_rxtx_map(struct arm_smccc_1_2_regs *res,
      __pkvm_host_unshare_hyp(hyp_phys_to_pfn(tx));
 err_unmap:
      ffa_unmap_hyp_buffers();
-     goto out_unlock;
+     goto out;
 }