Re: [PATCH v2 7/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions

[Sudeep Holla <sudeep.holla@arm.com>]

On Sat, Nov 28, 2020 at 02:36:55PM +0100, Jens Wiklander wrote:
> Hi Sudeep,
>

[...]

> > @@ -113,12 +117,19 @@ struct ffa_driver {
> >
> >  #define to_ffa_driver(d) container_of(d, struct ffa_driver, driver)
> >
> > +struct ffa_dev_ops {
> > +	int (*open)(struct ffa_device *dev);
> > +	int (*close)(struct ffa_device *dev);
> > +	long (*ioctl)(struct ffa_device *dev, unsigned int ioctl, void *arg);
> > +};
>
> I assume that all interaction with a SP is done via ffa_ops->ioctl().

Yes that was the idea.

> For example the ffa_msg_send_direct_req() function is then called via:
> struct ffa_send_recv_sync arg = { .endpoint_id = xxx, .data = yyy };
> rc = ffa_ops->ioctl(ffa_dev, FFA_SEND_RECEIVE_SYNC, &arg);
>

Correct.

> That isn't too hard to use, but is a bit inconvenient and less safe
> compared to a plain:
> rc = ffa_ops->send_recieve_sync(ffa_dev, xxx, yyy);
>

Agreed.

> I don't see any big win in then next patch with ffa_ioctl() either. That
> function must still do some actions specific for each ioctl id. So I'm a
> bit curious about the design choice.
>

Initial idea was to keep both in-kernel and user-space interface inline.
Also the assumption was that expect few old/legacy usecases, the userspace
is the way forward. While that is still ideal, but things have changed
since the main user of userspace interface(pKVM) is no longer using
FFA. I will change the interface as you mention above. I was also more
inclined towards that after dropping userspace. Good timing though, I was
about to post revised version dropping userspace. I will modify the
interface something on lines of your suggestion.

--
Regards,
Sudeep