Re: [PATCH 2/2] KVM: arm64: support optional calls of FF-A v1.2

[Yeoreum Yun <yeoreum.yun@arm.com>]

Hi Ben,

> > To use TPM drvier which uses CRB over FF-A with FFA_DIRECT_REQ2,
> > support the FF-A v1.2's optinal calls by querying whether
> > SPMC supports those.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> >  arch/arm64/kvm/hyp/nvhe/ffa.c | 19 ++++++++++++++++++-
> >  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > index 0ae87ff61758..9ded1c6369b9 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/ffa.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
> > @@ -646,6 +646,22 @@ static void do_ffa_mem_reclaim(struct arm_smccc_1_2_regs *res,
> >  		ffa_to_smccc_res(res, ret);
> >  }
> >
> > +static bool ffa_1_2_optional_calls_supported(u64 func_id)
> > +{
> > +	struct arm_smccc_1_2_regs res;
> > +
> > +	if (!smp_load_acquire(&has_version_negotiated) ||
> > +		(FFA_MINOR_VERSION(FFA_VERSION_1_2) < 2))
> > +		return false;
> > +
> > +	arm_smccc_1_2_smc(&(struct arm_smccc_1_2_regs) {
> > +		.a0 = FFA_FEATURES,
> > +		.a1 = func_id,
> > +	}, &res);
> > +
> > +	return res.a0 == FFA_SUCCESS;
> > +}
> > +
> >  /*
> >   * Is a given FFA function supported, either by forwarding on directly
> >   * or by handling at EL2?
> > @@ -678,12 +694,13 @@ static bool ffa_call_supported(u64 func_id)
> >  	case FFA_NOTIFICATION_SET:
> >  	case FFA_NOTIFICATION_GET:
> >  	case FFA_NOTIFICATION_INFO_GET:
> > +		return false;
> >  	/* Optional interfaces added in FF-A 1.2 */
> >  	case FFA_MSG_SEND_DIRECT_REQ2:		/* Optional per 7.5.1 */
> >  	case FFA_MSG_SEND_DIRECT_RESP2:		/* Optional per 7.5.1 */
> >  	case FFA_CONSOLE_LOG:			/* Optional per 13.1: not in Table 13.1 */
>
> Looking at table 13.54 in the FF-A 1.2 spec FFA_CONSOLE_LOG is only supported in secure FF-A
> instances and not from the normal world.

Thanks. in that case, we can return false for FFA_CONSOLE_LOG
unconditionally.

>
> >  	case FFA_PARTITION_INFO_GET_REGS:	/* Optional for virtual instances per 13.1 */
> > -		return false;
> > +		return ffa_1_2_optional_calls_supported(func_id);
> >  	}
>
> I don't think that an smc call here is the right thing to do. This changes this from a light
> weight deny list to an extra smc call for each ffa_msg_send_direct_req2 from the driver.
>
> Instead, I would expect this patch just to remove FFA_MSG_SEND_DIRECT_REQ2 from the deny list
> and rely on the TPM driver to use FFA_FEATURES to check whether it's supported.
>
> So, just this change:
>
> @@ -679,7 +679,6 @@ static bool ffa_call_supported(u64 func_id)
>         case FFA_NOTIFICATION_GET:
>         case FFA_NOTIFICATION_INFO_GET:
>         /* Optional interfaces added in FF-A 1.2 */
> -       case FFA_MSG_SEND_DIRECT_REQ2:          /* Optional per 7.5.1 */
>         case FFA_MSG_SEND_DIRECT_RESP2:         /* Optional per 7.5.1 */
>         case FFA_CONSOLE_LOG:                   /* Optional per 13.1: not in Table 13.1 */
>         case FFA_PARTITION_INFO_GET_REGS:       /* Optional for virtual instances per 13.1 */
>
> Am I missing something?

Nope. I think you don't think you miss anything and
I also think about it.

But, I'm not sure about "support" means in the pkvm about FF-A.
Anyway unless the SPMC doesn't support the specific FF-A ABI,
I don't know that's meaningful to return "TRUE" in here.
IOW, suppose pkvm returns supports of  FFA_MSG_SEND_DIRECT_REQ2
but user receive when it calls FFA_MSG_SEND_DIRECT_REQ2 with NOT SUPPORTED.

I'm not sure this inconsistency is allowed or not so as a defensive
perspective.

If that allows, I don't have a any special comment for this.

Thanks!

--
Sincerely,
Yeoreum Yun