| Message ID | 20201103174350.991593-1-sudeep.holla@arm.com |
|---|---|
| Headers | show |
| Series | firmware: Add initial support for Arm FF-A | expand |
On Tue, 3 Nov 2020 at 17:44, Sudeep Holla <sudeep.holla@arm.com> wrote: > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17 > to be used as result registers in SMC64/HVC64. Arm Firmware Framework > for Armv8-A specification makes use of x0-x7 as parameter and result > registers. > > Current SMCCC interface in the kernel just use x0-x7 as parameter and > x0-x3 as result registers. Let us add new interface to support x0-x7 > as parameter and result registers. This can be extended to include > x8-x17 when there are users for the same. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > arch/arm64/kernel/asm-offsets.c | 4 +++ > arch/arm64/kernel/smccc-call.S | 22 +++++++++++++++ > include/linux/arm-smccc.h | 50 +++++++++++++++++++++++++++++++++ > 3 files changed, 76 insertions(+) > > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 7d32fc959b1a..32bcc25337ce 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -122,6 +122,10 @@ int main(void) > DEFINE(ARM_SMCCC_RES_X2_OFFS, offsetof(struct arm_smccc_res, a2)); > DEFINE(ARM_SMCCC_QUIRK_ID_OFFS, offsetof(struct arm_smccc_quirk, id)); > DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS, offsetof(struct arm_smccc_quirk, state)); > + DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS, offsetof(struct arm_smccc_v1_2_res, a0)); > + DEFINE(ARM_SMCCC_V1_2_RES_X2_OFFS, offsetof(struct arm_smccc_v1_2_res, a2)); > + DEFINE(ARM_SMCCC_V1_2_RES_X4_OFFS, offsetof(struct arm_smccc_v1_2_res, a4)); > + DEFINE(ARM_SMCCC_V1_2_RES_X6_OFFS, offsetof(struct arm_smccc_v1_2_res, a6)); > BLANK(); > DEFINE(HIBERN_PBE_ORIG, offsetof(struct pbe, orig_address)); > DEFINE(HIBERN_PBE_ADDR, offsetof(struct pbe, address)); > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S > index d62447964ed9..0ea15c1742f3 100644 > --- a/arch/arm64/kernel/smccc-call.S > +++ b/arch/arm64/kernel/smccc-call.S > @@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc) > SMCCC hvc > SYM_FUNC_END(__arm_smccc_hvc) > EXPORT_SYMBOL(__arm_smccc_hvc) > + > + .macro SMCCC_v1_2 instr > + .cfi_startproc > + \instr #0 > + ldr x8, [sp] > + stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS] > + stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS] > + stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS] > + stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS] > + ret > + .cfi_endproc > +.endm > + > +SYM_FUNC_START(arm_smccc_v1_2_hvc) > + SMCCC_v1_2 hvc > +SYM_FUNC_END(arm_smccc_v1_2_hvc) > +EXPORT_SYMBOL(arm_smccc_v1_2_hvc) > + > +SYM_FUNC_START(arm_smccc_v1_2_smc) > + SMCCC_v1_2 smc > +SYM_FUNC_END(arm_smccc_v1_2_smc) > +EXPORT_SYMBOL(arm_smccc_v1_2_smc) > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h > index f860645f6512..aed27214c88f 100644 > --- a/include/linux/arm-smccc.h > +++ b/include/linux/arm-smccc.h > @@ -155,6 +155,56 @@ struct arm_smccc_res { > unsigned long a3; > }; > > +#ifdef CONFIG_ARM64 > +/* TODO Need to implement for ARM too */ > +/** > + * struct arm_smccc_v1_2_res - Result from SMC/HVC call > + * @a0-a3 result values from registers 0 to 3 Should this be a0-a7? > + */ > +struct arm_smccc_v1_2_res { > + unsigned long a0; > + unsigned long a1; > + unsigned long a2; > + unsigned long a3; > + unsigned long a4; > + unsigned long a5; > + unsigned long a6; > + unsigned long a7; > +}; > + > +/** > + * arm_smccc_v1_2_hvc() - make HVC calls > + * @a0-a7: arguments passed in registers 0 to 7 > + * @res: result values from registers 0 to 7 > + * > + * This function is used to make SMC calls following SMC Calling Convention > + * v1.2 or above. The content of the supplied param are copied to registers > + * 0 to 7 prior to the SMC instruction. The return values are updated with > + * the content from register 0 to 7 on return from the SMC instruction. s/SMC/HVC/ in several places above. > + */ > +asmlinkage > +void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2, > + unsigned long a3, unsigned long a4, unsigned long a5, > + unsigned long a6, unsigned long a7, > + struct arm_smccc_v1_2_res *res); > + > +/** > + * arm_smccc_v1_2_smc() - make SMC calls > + * @a0-a7: arguments passed in registers 0 to 7 > + * @res: result values from registers 0 to 7 > + * > + * This function is used to make SMC calls following SMC Calling Convention > + * v1.2 or above. The content of the supplied param are copied to registers > + * 0 to 7 prior to the SMC instruction. The return values are updated with > + * the content from register 0 to 7 on return from the SMC instruction. > + */ > +asmlinkage > +void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2, > + unsigned long a3, unsigned long a4, unsigned long a5, > + unsigned long a6, unsigned long a7, > + struct arm_smccc_v1_2_res *res); > +#endif > + > /** > * struct arm_smccc_quirk - Contains quirk information > * @id: quirk identification > -- > 2.25.1 >
Hi Sudeep, On Tue, Nov 03, 2020 at 05:43:41PM +0000, Sudeep Holla wrote: > Hi all, > > Let me start stating this is just initial implementation to check on > the idea of providing more in-kernel and userspace support. Lot of things > are still work in progress, I am posting just to get the early feedback > before building lot of things on this idea. Consider this more as RFC > though not tagged explicity(just to avoid it being ignored :)) > > Arm Firmware Framework for Armv8-A specification[1] describes a software > architecture that provides mechanism to utilise the virtualization > extension to isolate software images and describes interfaces that > standardize communication between the various software images. This > includes communication between images in the Secure and Normal world. > > The main idea here is to create FFA device to establish any communication > with a partition(secure or normal world VM). > > If it is a partition managed by hypervisor, then we will register chardev > associated with each of those partition FFA device. > > /dev/arm_ffa: > > e3a48fa5-dc54-4a8b-898b-bdc4dfeeb7b8 > 49f65057-d002-4ae2-b4ee-d31c7940a13d > > For in-kernel usage(mostly communication with secure partitions), only > in-kernel APIs are accessible(no userspace). There may be a need to > provide userspace access instead of in-kernel, it is not yet support > in this series as we need way to identify those and I am not sure if > that belong to DT. With unfiltered VM to VM commnication from user space there's no easy way for two VMs to exchange privileged information that excludes user space. Perhaps access to the FFA device is considered privileged and enough for all purposes. If I've understood it correctly is VM to SP communication only allowed via kernel mode in the VM. The communication with OP-TEE depends on this with the recent commit c5b4312bea5d ("tee: optee: Add support for session login client UUID generation"). Cheers, Jens
Hi Sudeep, On Tue, Nov 03, 2020 at 05:43:48PM +0000, Sudeep Holla wrote: > Parse the FFA nodes from the device-tree and register all the partitions > whose services will be used in the kernel. > > In order to also enable in-kernel users of FFA interface, 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/bus.c | 2 + > drivers/firmware/arm_ffa/driver.c | 281 ++++++++++++++++++++++++++++++ > include/linux/arm_ffa.h | 15 ++ > include/uapi/linux/arm_ffa.h | 56 ++++++ > 4 files changed, 354 insertions(+) > create mode 100644 include/uapi/linux/arm_ffa.h > > diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c > index 2dbcf356bd8b..de4f8b54271a 100644 > --- a/drivers/firmware/arm_ffa/bus.c > +++ b/drivers/firmware/arm_ffa/bus.c > @@ -90,6 +90,7 @@ static void ffa_release_device(struct device *dev) > { > struct ffa_device *ffa_dev = to_ffa_dev(dev); > > + mutex_destroy(&ffa_dev->mutex); > kfree(ffa_dev); > } > > @@ -127,6 +128,7 @@ int ffa_device_register(struct ffa_device *ffa_dev) > > dev = &ffa_dev->dev; > cdev = &ffa_dev->cdev; > + mutex_init(&ffa_dev->mutex); > > dev->bus = &ffa_bus_type; > dev->type = &ffa_dev_type; > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c > index 1c4a5e5095b5..2e5fa56fffb7 100644 > --- a/drivers/firmware/arm_ffa/driver.c > +++ b/drivers/firmware/arm_ffa/driver.c > @@ -24,9 +24,16 @@ > > #include <linux/arm_ffa.h> > #include <linux/bitfield.h> > +#include <linux/device.h> > +#include <linux/fs.h> > #include <linux/io.h> > +#include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/of.h> > #include <linux/slab.h> > +#include <linux/uaccess.h> > +#include <linux/uuid.h> > +#include <uapi/linux/arm_ffa.h> > > #include "common.h" > > @@ -48,6 +55,13 @@ > /* Keeping RX TX buffer size as 64K for now */ > #define RXTX_BUFFER_SIZE SZ_64K > > +#define list_to_ffa_dev(n) container_of(n, struct ffa_device, node) > + > +/* List of all FFA devices active in system */ > +static LIST_HEAD(ffa_devs_list); > +/* Protection for the entire list */ > +static DEFINE_MUTEX(ffa_devs_list_mutex); > + > static ffa_fn *invoke_ffa_fn; > > static const int ffa_linux_errmap[] = { > @@ -104,6 +118,20 @@ static int ffa_version_check(u32 *version) > return 0; > } > > +static int ffa_rx_release(void) > +{ > + ffa_res_t ret; > + > + ret = invoke_ffa_fn(FFA_RX_RELEASE, 0, 0, 0, 0, 0, 0, 0); > + > + if (ret.a0 == FFA_ERROR) > + return ffa_to_linux_errno((int)ret.a2); > + > + /* check for ret.a0 == FFA_RX_RELEASE ? */ > + > + return 0; > +} > + > static int ffa_rxtx_map(phys_addr_t tx_buf, phys_addr_t rx_buf, u32 pg_cnt) > { > ffa_res_t ret; > @@ -128,6 +156,50 @@ static int ffa_rxtx_unmap(u16 vm_id) > return 0; > } > > +static int ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3, > + struct ffa_partition_info **buffer) > +{ > + int count; > + ffa_res_t partition_info; > + > + mutex_lock(&drv_info->rx_lock); > + partition_info = invoke_ffa_fn(FFA_PARTITION_INFO_GET, uuid0, uuid1, > + uuid2, uuid3, 0, 0, 0); > + > + if (partition_info.a0 == FFA_ERROR) > + return ffa_to_linux_errno((int)partition_info.a2); > + > + count = partition_info.a2; > + > + if (buffer) > + memcpy(*buffer, drv_info->rx_buffer, sizeof(*buffer) * count); > + > + ffa_rx_release(); > + > + mutex_unlock(&drv_info->rx_lock); > + > + return count; > +} > + > +static int ffa_partition_probe(const char *uuid_str, > + struct ffa_partition_info *buffer) > +{ > + int count; > + uuid_t uuid; > + u32 uuid0_4[4] = { 0 }; > + > + if (uuid_parse(uuid_str, &uuid)) { > + pr_err("invalid uuid (%s)\n", uuid_str); > + return -ENODEV; > + } > + > + export_uuid((u8 *)uuid0_4, &uuid); > + count = ffa_partition_info_get(uuid0_4[0], uuid0_4[1], uuid0_4[2], > + uuid0_4[3], &buffer); > + > + return count; > +} > + > #define VM_ID_MASK GENMASK(15, 0) > static int ffa_id_get(u16 *vm_id) > { > @@ -143,6 +215,205 @@ static int ffa_id_get(u16 *vm_id) > return 0; > } > > +static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, > + struct ffa_send_direct_data *data) > +{ > + u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id); > + ffa_res_t ret; > + > + ret = invoke_ffa_fn(FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ), src_dst_ids, 0, > + data->data0, data->data1, data->data2, > + data->data3, data->data4); > + > + while (ret.a0 == FFA_INTERRUPT) > + ret = invoke_ffa_fn(FFA_RUN, ret.a1, 0, 0, 0, 0, 0, 0); > + if (ret.a0 == FFA_ERROR) > + return ffa_to_linux_errno((int)ret.a2); > + > + if (ret.a0 == FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP)) { > + data->data0 = ret.a3; > + data->data1 = ret.a4; > + data->data2 = ret.a5; > + data->data3 = ret.a6; > + data->data4 = ret.a7; > + } > + > + return 0; > +} > + > +static void ffa_device_get(struct ffa_device *ffa_dev) > +{ > + mutex_lock(&ffa_dev->mutex); > + ffa_dev->num_users++; > + mutex_unlock(&ffa_dev->mutex); > +} > + > +static void ffa_device_put(struct ffa_device *ffa_dev) > +{ > + mutex_lock(&ffa_dev->mutex); > + ffa_dev->num_users--; > + mutex_unlock(&ffa_dev->mutex); > +} > + > +static const struct ffa_dev_ops ffa_ops; > + > +const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev) > +{ > + struct list_head *p; > + const 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_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: > + return drv_info->version; > + case FFA_GET_PARTITION_ID: > + return ffa_dev->vm_id; > + 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; > + } > + default: > + r = -EINVAL; > + } > + > + return r; > +} > + > +static const struct ffa_dev_ops ffa_ops = { > + .open = ffa_dev_open, > + .close = ffa_dev_close, > + .ioctl = ffa_dev_ioctl, > +}; > + > +static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid, > + const struct file_operations *fops) > +{ > + int ret; > + struct ffa_device *ffa_dev; > + > + ffa_dev = kzalloc(sizeof(*ffa_dev), GFP_KERNEL); > + if (!ffa_dev) > + return -ENOMEM; > + > + ffa_dev->vm_id = vm_id; > + if (uuid) > + uuid_copy(&ffa_dev->uuid, uuid); > + > + dev_set_name(&ffa_dev->dev, name); > + dev_set_drvdata(&ffa_dev->dev, drv_info); > + cdev_init(&ffa_dev->cdev, fops); > + > + ret = ffa_device_register(ffa_dev); > + if (ret) > + return ret; > + > + mutex_lock(&ffa_devs_list_mutex); > + list_add_tail(&ffa_dev->node, &ffa_devs_list); > + mutex_unlock(&ffa_devs_list_mutex); > + > + return 0; > +} > + > +int ffa_setup_partitions(const char *compatible, > + const struct file_operations *fops) > +{ > + int ret; > + struct ffa_partition_info pbuf; > + struct device_node *child, *parent; > + const char *p_uuid, *pfx = "Ignoring FFA partition"; > + uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0); > + > + parent = of_find_compatible_node(NULL, NULL, compatible); > + if (!parent) > + return 0; > + > + for_each_child_of_node(parent, child) { > + if (!of_device_is_compatible(child, "arm,ffa-1.0-partition")) { > + of_node_put(child); > + continue; > + } > + > + if (of_property_read_string(child, "uuid", &p_uuid)) { > + pr_err("%s: failed to parse \"uuid\" property\n", pfx); > + of_node_put(child); > + continue; > + } > + > + of_node_put(child); > + > + if (uuid_parse(p_uuid, &uuid)) { > + pr_err("%s: invalid \"uuid\" property (%s)\n", > + pfx, p_uuid); > + continue; > + } > + > + ret = ffa_partition_probe(p_uuid, &pbuf); > + if (ret != 1) { > + pr_err("%s: %s partition info probe failed\n", > + pfx, p_uuid); > + return -EINVAL; > + } > + > + ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid, fops); > + if (ret) { > + pr_err("%s: failed to register %s\n", pfx, p_uuid); > + continue; > + } > + } > + > + of_node_put(parent); > + return 0; > +} > + > static int __init ffa_init(void) > { > int ret; > @@ -192,6 +463,9 @@ static int __init ffa_init(void) > mutex_init(&drv_info->rx_lock); > mutex_init(&drv_info->tx_lock); > > + /* Set up all the partitions which have in-kernel drivers */ > + ffa_setup_partitions("arm,ffa-1.0", NULL); > + > return 0; > free_pages: > if (drv_info->tx_buffer) > @@ -205,6 +479,13 @@ module_init(ffa_init); > > static void __exit ffa_exit(void) > { > + struct list_head *p; > + > + mutex_lock(&ffa_devs_list_mutex); > + list_for_each(p, &ffa_devs_list) > + ffa_device_unregister(list_to_ffa_dev(p)); > + mutex_unlock(&ffa_devs_list_mutex); > + > ffa_rxtx_unmap(drv_info->vm_id); > free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE); > free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE); > diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h > index 6185d0d12f15..719ef02fe42d 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> > > #define FFA_SMC(calling_convention, func_num) \ > ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, (calling_convention), \ > @@ -94,6 +95,9 @@ struct ffa_device { > uuid_t uuid; > struct device dev; > struct cdev cdev; > + size_t num_users; > + struct mutex mutex; /* protects num_users */ > + struct list_head node; > }; > > #define to_ffa_dev(d) container_of(d, struct ffa_device, dev) > @@ -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(). 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); 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); 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. Cheers, Jens > + > #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); > +const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev); > > #else > static inline int ffa_device_register(struct ffa_device *ffa_dev) > @@ -137,6 +148,10 @@ ffa_driver_register(struct ffa_driver *driver, struct module *owner, > > static inline void ffa_driver_unregister(struct ffa_driver *driver) {} > > +const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev) > +{ > + return NULL; > +} > #endif /* CONFIG_ARM_FFA_TRANSPORT */ > > #define ffa_register(driver) \ > diff --git a/include/uapi/linux/arm_ffa.h b/include/uapi/linux/arm_ffa.h > new file mode 100644 > index 000000000000..88ddddb4742f > --- /dev/null > +++ b/include/uapi/linux/arm_ffa.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* > + * Copyright (C) 2020 Arm Ltd. > + */ > + > +#ifndef _UAPI_LINUX_ARM_FFA_H > +#define _UAPI_LINUX_ARM_FFA_H > + > +#include <linux/types.h> > +#include <linux/ioctl.h> > + > +#define FFA_BASE 0xFF > + > +struct ffa_partition_info { > + __u16 id; > + __u16 exec_ctxt; > +/* partition supports receipt of direct requests */ > +#define FFA_PARTITION_DIRECT_RECV BIT(0) > +/* partition can send direct requests. */ > +#define FFA_PARTITION_DIRECT_SEND BIT(1) > +/* partition can send and receive indirect messages. */ > +#define FFA_PARTITION_INDIRECT_MSG BIT(2) > + __u32 properties; > +}; > + > +struct ffa_part_info { > + char uuid_str[36]; > + struct ffa_partition_info info; > +}; > + > +struct ffa_send_direct_data { > + unsigned long data0; > + unsigned long data1; > + unsigned long data2; > + unsigned long data3; > + unsigned long data4; > +}; > + > +struct ffa_send_recv_sync { > + __u16 endpoint_id; > + struct ffa_send_direct_data data; > +}; > + > +struct ffa_send_recv_async { > + __u16 endpoint_id; > + int length; > + char buffer[]; > +}; > + > +#define FFA_GET_API_VERSION _IO(FFA_BASE, 0x00) > +#define FFA_GET_PARTITION_ID _IO(FFA_BASE, 0x01) > +#define FFA_GET_PARTITION_INFO _IOWR(FFA_BASE, 0x02, struct ffa_part_info) > +#define FFA_SEND_RECEIVE_SYNC _IOWR(FFA_BASE, 0x03, struct ffa_send_recv_sync) > +#define FFA_SEND_RECEIVE_ASYNC _IOW(FFA_BASE, 0x04, struct ffa_send_recv_async) > + > +#endif /*_UAPI_LINUX_ARM_FFA_H*/ > -- > 2.25.1 >
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
On Sat, Nov 28, 2020 at 01:25:02PM +0100, Jens Wiklander wrote: > Hi Sudeep, > > On Tue, Nov 03, 2020 at 05:43:41PM +0000, Sudeep Holla wrote: > > Hi all, > > > > Let me start stating this is just initial implementation to check on > > the idea of providing more in-kernel and userspace support. Lot of things > > are still work in progress, I am posting just to get the early feedback > > before building lot of things on this idea. Consider this more as RFC > > though not tagged explicity(just to avoid it being ignored :)) > > > > Arm Firmware Framework for Armv8-A specification[1] describes a software > > architecture that provides mechanism to utilise the virtualization > > extension to isolate software images and describes interfaces that > > standardize communication between the various software images. This > > includes communication between images in the Secure and Normal world. > > > > The main idea here is to create FFA device to establish any communication > > with a partition(secure or normal world VM). > > > > If it is a partition managed by hypervisor, then we will register chardev > > associated with each of those partition FFA device. > > > > /dev/arm_ffa: > > > > e3a48fa5-dc54-4a8b-898b-bdc4dfeeb7b8 > > 49f65057-d002-4ae2-b4ee-d31c7940a13d > > > > For in-kernel usage(mostly communication with secure partitions), only > > in-kernel APIs are accessible(no userspace). There may be a need to > > provide userspace access instead of in-kernel, it is not yet support > > in this series as we need way to identify those and I am not sure if > > that belong to DT. > > With unfiltered VM to VM commnication from user space there's no easy > way for two VMs to exchange privileged information that excludes user > space. Though this usercase is dropped now, it was targeted for VMM and may be it was not an issue there. > Perhaps access to the FFA device is considered privileged and > enough for all purposes. > I don't know TBH. > If I've understood it correctly is VM to SP communication only allowed > via kernel mode in the VM. Correct. > The communication with OP-TEE depends on this with the recent commit > c5b4312bea5d ("tee: optee: Add support for session login client UUID > generation"). > OK, thanks for the info.
Hi all, Let me start stating this is just initial implementation to check on the idea of providing more in-kernel and userspace support. Lot of things are still work in progress, I am posting just to get the early feedback before building lot of things on this idea. Consider this more as RFC though not tagged explicity(just to avoid it being ignored :)) Arm Firmware Framework for Armv8-A specification[1] describes a software architecture that provides mechanism to utilise the virtualization extension to isolate software images and describes interfaces that standardize communication between the various software images. This includes communication between images in the Secure and Normal world. The main idea here is to create FFA device to establish any communication with a partition(secure or normal world VM). If it is a partition managed by hypervisor, then we will register chardev associated with each of those partition FFA device. /dev/arm_ffa: e3a48fa5-dc54-4a8b-898b-bdc4dfeeb7b8 49f65057-d002-4ae2-b4ee-d31c7940a13d For in-kernel usage(mostly communication with secure partitions), only in-kernel APIs are accessible(no userspace). There may be a need to provide userspace access instead of in-kernel, it is not yet support in this series as we need way to identify those and I am not sure if that belong to DT. -- Regards, Sudeep [1] https://developer.arm.com/documentation/den0077/latest v1->v2: - Moved userspace code to a separate unit, will move to separate module. Still working on minimizing initcall dependencies and exported functions to reuse some of the code. - Fixed couple of minor issues pointed out - Dropped ASYNC send message as I haven't been able to test Sudeep Holla (8): dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions arm64: smccc: Add support for SMCCCv1.2 input/output registers firmware: arm_ffa: Add initial FFA bus support for device enumeration firmware: arm_ffa: Add initial Arm FFA driver support firmware: arm_ffa: Add support for SMCCC as transport to FFA driver firmware: arm_ffa: Setup in-kernel users of FFA partitions firmware: arm_ffa: Setup and register all the KVM managed partitions firmware: arm_ffa: Add support for MEM_* interfaces Will Deacon (1): dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding .../devicetree/bindings/arm/arm,ffa-hyp.yaml | 102 +++ .../devicetree/bindings/arm/arm,ffa.yaml | 58 ++ .../reserved-memory/arm,ffa-memory.yaml | 71 ++ arch/arm64/kernel/asm-offsets.c | 4 + arch/arm64/kernel/smccc-call.S | 22 + drivers/firmware/Kconfig | 1 + drivers/firmware/Makefile | 1 + drivers/firmware/arm_ffa/Kconfig | 21 + drivers/firmware/arm_ffa/Makefile | 6 + drivers/firmware/arm_ffa/bus.c | 199 +++++ drivers/firmware/arm_ffa/common.h | 35 + drivers/firmware/arm_ffa/driver.c | 737 ++++++++++++++++++ drivers/firmware/arm_ffa/hyp_partitions.c | 132 ++++ drivers/firmware/arm_ffa/smccc.c | 54 ++ include/linux/arm-smccc.h | 50 ++ include/linux/arm_ffa.h | 310 ++++++++ include/uapi/linux/arm_ffa.h | 67 ++ 17 files changed, 1870 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa-hyp.yaml create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml create mode 100644 drivers/firmware/arm_ffa/Kconfig create mode 100644 drivers/firmware/arm_ffa/Makefile create mode 100644 drivers/firmware/arm_ffa/bus.c create mode 100644 drivers/firmware/arm_ffa/common.h create mode 100644 drivers/firmware/arm_ffa/driver.c create mode 100644 drivers/firmware/arm_ffa/hyp_partitions.c create mode 100644 drivers/firmware/arm_ffa/smccc.c create mode 100644 include/linux/arm_ffa.h create mode 100644 include/uapi/linux/arm_ffa.h -- 2.25.1