Re: [PATCH v2 04/10] KVM: arm64: Handle FFA_RXTX_MAP and FFA_RXTX_UNMAP calls from the host

Hey Oliver,

On Wed, May 10, 2023 at 08:50:39PM +0000, Oliver Upton wrote:

On Wed, Apr 19, 2023 at 01:20:45PM +0100, Will Deacon wrote:

quoted

Handle FFA_RXTX_MAP and FFA_RXTX_UNMAP calls from the host by sharing
the host's mailbox memory with the hypervisor and establishing a
separate pair of mailboxes between the hypervisor and the SPMD at EL3.

Co-developed-by: Andrew Walbran <redacted>
Signed-off-by: Andrew Walbran <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
 arch/arm64/kvm/hyp/nvhe/ffa.c | 184 ++++++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h       |   8 ++
 2 files changed, 192 insertions(+)

[...]

quoted

+static int spmd_map_ffa_buffers(u64 ffa_page_count)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_smc(FFA_FN64_RXTX_MAP,
+			  hyp_virt_to_phys(hyp_buffers.tx),
+			  hyp_virt_to_phys(hyp_buffers.rx),
+			  ffa_page_count,
+			  0, 0, 0, 0,
+			  &res);
+
+	return res.a0 == FFA_SUCCESS ? FFA_RET_SUCCESS : res.a2;
+}
+
+static int spmd_unmap_ffa_buffers(void)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_1_1_smc(FFA_RXTX_UNMAP,
+			  HOST_FFA_ID,
+			  0, 0, 0, 0, 0, 0,
+			  &res);
+
+	return res.a0 == FFA_SUCCESS ? FFA_RET_SUCCESS : res.a2;
+}

</snip>

I understand SPMD is significant in the context of the whole FF-A spec,
but I don't think the degree of precision is necessary to get the point
across for someone coming at this from a more KVM-centric background.

So, for the sake of readability, could these helpers be named
ffa_map_hyp_buffers()/ffa_unmap_hyp_buffers() (or similar)? Sticking
'hyp' in there somewhere helps the reader understand exactly what is
getting mapped, since there are both hyp and host buffers.

Apologies for bikeshedding, but I want to make sure folks w/o knowledge
of FF-A can at least grasp some of the mechanics here.

It's no skin off my back, we can name them whatever you like!

The only thing I ask is that we're consistent, so if it's ok with you,
I'll rename all the 'spmd_' functions to 'ffa_'.

quoted

+static void do_ffa_rxtx_map(struct arm_smccc_res *res,
+			    struct kvm_cpu_context *ctxt)

This could do with some clarifying comments as well, since there is
rather significant interaction between host MAP calls and how we handle
host/hyp buffers.

Is there something in particular you think needs to be clarified? The
function looks pretty straightforward to me, but then I'm used to it by
now. I'd just like to avoid writing a comment that basically describes
how the code works because that doesn't help anybody.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel