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

[Will Deacon]

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:
> > 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 <qwandor at google.com>
> > Signed-off-by: Andrew Walbran <qwandor at google.com>
> > Signed-off-by: Will Deacon <will at kernel.org>
> > ---
> >  arch/arm64/kvm/hyp/nvhe/ffa.c | 184 ++++++++++++++++++++++++++++++++++
> >  include/linux/arm_ffa.h       |   8 ++
> >  2 files changed, 192 insertions(+)

[...]

> > +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_'.

> > +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