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

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

Hi Sudeep,

[...]

quoted

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

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