devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Achin Gupta <Achin.Gupta@arm.com>
To: Sudeep Holla <Sudeep.Holla@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"kernel-team@android.com" <kernel-team@android.com>,
	Will Deacon <will@kernel.org>,
	"tsoni@quicinc.com" <tsoni@quicinc.com>,
	Pratik Patel <pratikp@quicinc.com>, nd <nd@arm.com>
Subject: Re: [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions
Date: Mon, 7 Sep 2020 11:32:16 +0000	[thread overview]
Message-ID: <04735224-9609-4E48-A7F6-8E811AF05223@arm.com> (raw)
In-Reply-To: <20200829170923.29949-10-sudeep.holla@arm.com>

Hi Sudeep,

CIL...

> On 29 Aug 2020, at 18:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> In order to also enable in-kernel users of FFA interface along with
> the access to user-space applications, let us add simple set of operations
> for such devices.
> 
> The in-kernel users are registered without the character device interface.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_ffa/driver.c | 119 ++++++++++++++++++++++++++----
> include/linux/arm_ffa.h           |  12 +++
> 2 files changed, 117 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 96113e594db6..811558ef2a1d 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -372,6 +372,97 @@ static void ffa_device_put(struct ffa_device *ffa_dev)
> 	mutex_unlock(&ffa_dev->mutex);
> }
> 
> +static int ffa_dev_open(struct ffa_device *ffa_dev)
> +{
> +	ffa_device_get(ffa_dev);
> +
> +	return 0;
> +}
> +
> +static int ffa_dev_close(struct ffa_device *ffa_dev)
> +{
> +	ffa_device_put(ffa_dev);
> +
> +	return 0;
> +}
> +
> +static long
> +ffa_dev_ioctl(struct ffa_device *ffa_dev, unsigned int ioctl, void *arg)
> +{
> +	long r = -EINVAL;
> +
> +	switch (ioctl) {
> +	case FFA_GET_API_VERSION:
> +		r = drv_info->version;
> +		break;
> +	case FFA_GET_PARTITION_ID:
> +		r = ffa_dev->vm_id;
> +		break;
> +	case FFA_GET_PARTITION_INFO: {
> +		struct ffa_part_info *pinfo = arg;
> +
> +		if (ffa_partition_probe(pinfo->uuid_str, &pinfo->info) != 1)
> +			r = -E2BIG;
> +		break;
> +	}
> +	case FFA_SEND_RECEIVE_SYNC: {
> +		struct ffa_send_recv_sync *kdata = arg;
> +
> +		r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata->endpoint_id,
> +					    &kdata->data);
> +		break;
> +	}
> +	case FFA_SEND_RECEIVE_ASYNC: {

For indirect messaging, I am thinking who is responsible for creating and managing the “threads” for each vCPU of a FF-A VM that has an in-kernel user.

An approach is the one taken by the Hafnium driver [1]. It creates and manages a kthread for each vCPU of each VM. 

A client of the Hafnium driver only needs to worry about message exchange with various VMs. Thread management is Hafnium specific and rightly stays in the Hafnium module instead of being replicated in each client.

Is this the direct of travel for the FF-A driver as well?

Cheers,
Achin

[1] https://git.trustedfirmware.org/hafnium/driver/linux.git/tree/main.c#n342

> +		struct ffa_send_recv_async *kdata = arg;
> +
> +		if (kdata->length < 0 || kdata->length > RXTX_BUFFER_SIZE) {
> +			r = -EINVAL;
> +			break;
> +		}
> +
> +		r = ffa_msg_send(ffa_dev->vm_id, kdata->endpoint_id,
> +				 kdata->buffer, kdata->length);
> +		break;
> +	}
> +	default:
> +		r = -EINVAL;
> +	}
> +
> +	return r;
> +}
> +
> +struct ffa_dev_ops ffa_ops = {
> +	.open = ffa_dev_open,
> +	.close = ffa_dev_close,
> +	.ioctl = ffa_dev_ioctl,
> +};
> +
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
> +{
> +	struct list_head *p;
> +	struct ffa_dev_ops *ops = NULL;
> +
> +	if (uuid_is_null(&dev->uuid))
> +		return NULL;
> +
> +	mutex_lock(&ffa_devs_list_mutex);
> +
> +	list_for_each(p, &ffa_devs_list) {
> +		struct ffa_device *tmp_dev;
> +
> +		tmp_dev = list_to_ffa_dev(p);
> +
> +		if (uuid_equal(&tmp_dev->uuid, &dev->uuid)) {
> +			ops = &ffa_ops;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&ffa_devs_list_mutex);
> +
> +	return ops;
> +}
> +
> static int ffa_open(struct inode *inode, struct file *filp)
> {
> 	struct ffa_device *ffa_dev;
> @@ -380,7 +471,7 @@ static int ffa_open(struct inode *inode, struct file *filp)
> 	if (!ffa_dev)
> 		return -EINVAL;
> 
> -	ffa_device_get(ffa_dev);
> +	ffa_dev_open(ffa_dev);
> 
> 	filp->private_data = ffa_dev;
> 
> @@ -391,7 +482,7 @@ static int ffa_release(struct inode *inode, struct file *filp)
> {
> 	struct ffa_device *ffa_dev = filp->private_data;
> 
> -	ffa_device_put(ffa_dev);
> +	ffa_dev_close(ffa_dev);
> 
> 	filp->private_data = NULL;
> 
> @@ -406,14 +497,10 @@ static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> 
> 	switch (ioctl) {
> 	case FFA_GET_API_VERSION:
> -		if (arg)
> -			goto out;
> -		r = drv_info->version;
> -		break;
> 	case FFA_GET_PARTITION_ID:
> 		if (arg)
> 			goto out;
> -		r = ffa_dev->vm_id;
> +		r = ffa_dev_ioctl(ffa_dev, FFA_GET_PARTITION_ID, NULL);
> 		break;
> 	case FFA_GET_PARTITION_INFO: {
> 		struct ffa_partition_info pbuf;
> @@ -499,7 +586,8 @@ static const struct file_operations ffa_fops = {
> 	.llseek		= noop_llseek,
> };
> 
> -static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> +static int ffa_device_alloc_register(const char *name, u16 vm_id,
> +				     uuid_t *uuid, bool cdev_if)
> {
> 	int ret;
> 	struct ffa_device *ffa_dev;
> @@ -514,7 +602,7 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> 
> 	dev_set_name(&ffa_dev->dev, name);
> 	dev_set_drvdata(&ffa_dev->dev, drv_info);
> -	cdev_init(&ffa_dev->cdev, &ffa_fops);
> +	cdev_init(&ffa_dev->cdev, cdev_if ? &ffa_fops : NULL);
> 
> 	ret = ffa_device_register(ffa_dev);
> 	if (ret)
> @@ -527,13 +615,13 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> 	return 0;
> }
> 
> -static int ffa_setup_partitions(void)
> +static int ffa_setup_partitions(bool hyp)
> {
> 	int ret;
> 	struct ffa_partition_info pbuf;
> 	struct device_node *child, *parent;
> 	const char *p_uuid, *pfx = "Ignoring FFA partition";
> -	const char *compatible = "arm,ffa-1.0-hypervisor";
> +	const char *compatible = hyp ? "arm,ffa-1.0-hypervisor" : "arm,ffa-1.0";
> 	uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> 
> 	parent = of_find_compatible_node(NULL, NULL, compatible);
> @@ -567,7 +655,7 @@ static int ffa_setup_partitions(void)
> 			return -EINVAL;
> 		}
> 
> -		ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid);
> +		ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid, hyp);
> 		if (ret) {
> 			pr_err("%s: failed to register %s\n", pfx, p_uuid);
> 			continue;
> @@ -624,12 +712,15 @@ static int __init ffa_init(void)
> 	mutex_init(&drv_info->tx_lock);
> 
> 	/* This will be default device both in theguests and host */
> -	ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
> +	ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL, true);
> 	if (ret)
> 		return ret;
> 
> 	/* Set up all the partitions that KVM hypervisor maintains */
> -	ffa_setup_partitions();
> +	ffa_setup_partitions(true);
> +
> +	/* Set up all the partitions which have in-kernel drivers */
> +	ffa_setup_partitions(false);
> 
> 	return 0;
> free_pages:
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 6824e0633c77..1df7399d207d 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -11,6 +11,7 @@
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/uuid.h>
> +#include <uapi/linux/arm_ffa.h>
> 
> struct ffa_device {
> 	u32 vm_id;
> @@ -39,12 +40,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);
> +};
> +
> #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
> int ffa_device_register(struct ffa_device *ffa_dev);
> void ffa_device_unregister(struct ffa_device *ffa_dev);
> int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
> 			const char *mod_name);
> void ffa_driver_unregister(struct ffa_driver *driver);
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
> 
> #else
> static inline int ffa_device_register(struct ffa_device *ffa_dev)
> @@ -63,6 +71,10 @@ ffa_driver_register(struct ffa_driver *driver, struct module *owner,
> 
> static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
> 
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
> +{
> +	return NULL;
> +}
> #endif /* CONFIG_ARM_FFA_TRANSPORT */
> 
> #define ffa_register(driver) \
> -- 
> 2.17.1
> 
> 


  parent reply	other threads:[~2020-09-07 11:32 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2020-09-01 16:03   ` Jonathan Cameron
2020-09-07 11:40     ` Sudeep Holla
2020-09-02 22:14   ` Rob Herring
2020-11-03 15:59     ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions Sudeep Holla
2020-09-02 21:36   ` Rob Herring
2020-09-07 11:41     ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 3/9] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
2020-08-29 17:09 ` [PATCH 4/9] firmware: smccc: export both smccc functions Sudeep Holla
2020-08-29 17:09 ` [PATCH 5/9] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
2020-08-29 17:09 ` [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
2020-09-07  7:55   ` Fuad Tabba
2020-09-07  9:29     ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
2020-09-01 16:23   ` Jonathan Cameron
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
2020-09-01 16:34   ` Jonathan Cameron
2020-09-07 11:20   ` Achin Gupta
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
2020-09-01 16:38   ` Jonathan Cameron
2020-09-07 11:32   ` Achin Gupta [this message]
2020-09-07 12:04     ` Andrew Walbran

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=04735224-9609-4E48-A7F6-8E811AF05223@arm.com \
    --to=achin.gupta@arm.com \
    --cc=Sudeep.Holla@arm.com \
    --cc=devicetree@vger.kernel.org \
    --cc=kernel-team@android.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=nd@arm.com \
    --cc=pratikp@quicinc.com \
    --cc=tsoni@quicinc.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).