From 244f5d597e1ea519c2085fbd9819458688775e42 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Fri, 24 Sep 2021 10:28:58 +0100 Subject: [PATCH 1/5] firmware: arm_ffa: Add missing remove callback to ffa_bus_type Currently the arm_ffa firmware driver can be built as module and hence all the users of FFA driver. If any driver on the ffa bus is removed or unregistered, the remove callback on all the device bound to the driver being removed should be callback. For that to happen, we must register a remove callback on the ffa_bus which is currently missing. This results in the probe getting called again without the previous remove callback on a device which may result in kernel crash. Fix the issue by registering the remove callback on the FFA bus. Link: https://lore.kernel.org/r/20210924092859.3057562-1-sudeep.holla@arm.com Fixes: e781858488b9 ("firmware: arm_ffa: Add initial FFA bus support for device enumeration") Reported-by: Jens Wiklander Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/bus.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c index 00fe595a5bc8..f01348e6cf1c 100644 --- a/drivers/firmware/arm_ffa/bus.c +++ b/drivers/firmware/arm_ffa/bus.c @@ -49,6 +49,13 @@ static int ffa_device_probe(struct device *dev) return ffa_drv->probe(ffa_dev); } +static void ffa_device_remove(struct device *dev) +{ + struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver); + + ffa_drv->remove(to_ffa_dev(dev)); +} + static int ffa_device_uevent(struct device *dev, struct kobj_uevent_env *env) { struct ffa_device *ffa_dev = to_ffa_dev(dev); @@ -86,6 +93,7 @@ struct bus_type ffa_bus_type = { .name = "arm_ffa", .match = ffa_device_match, .probe = ffa_device_probe, + .remove = ffa_device_remove, .uevent = ffa_device_uevent, .dev_groups = ffa_device_attributes_groups, }; From eb7b52e6db7c21400b9b2d539f9343fb6e94bd94 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Fri, 24 Sep 2021 10:28:59 +0100 Subject: [PATCH 2/5] firmware: arm_ffa: Fix __ffa_devices_unregister When arm_ffa firmware driver module is unloaded or removed we call __ffa_devices_unregister on all the devices on the ffa bus. It must unregister all the devices instead it is currently just releasing the devices without unregistering. That is pure wrong as when we try to load the module back again, it will result in the kernel crash something like below. -->8 CPU: 2 PID: 232 Comm: modprobe Not tainted 5.15.0-rc2+ #169 Hardware name: FVP Base RevC (DT) Call trace: dump_backtrace+0x0/0x1cc show_stack+0x18/0x64 dump_stack_lvl+0x64/0x7c dump_stack+0x18/0x38 sysfs_create_dir_ns+0xe4/0x140 kobject_add_internal+0x170/0x358 kobject_add+0x94/0x100 device_add+0x178/0x5f0 device_register+0x20/0x30 ffa_device_register+0x80/0xcc [ffa_module] ffa_setup_partitions+0x7c/0x108 [ffa_module] init_module+0x290/0x2dc [ffa_module] do_one_initcall+0xbc/0x230 do_init_module+0x58/0x304 load_module+0x15e0/0x1f68 __arm64_sys_finit_module+0xb8/0xf4 invoke_syscall+0x44/0x140 el0_svc_common+0xb4/0xf0 do_el0_svc+0x24/0x80 el0_svc+0x20/0x50 el0t_64_sync_handler+0x84/0xe4 el0t_64_sync+0x1a0/0x1a4 kobject_add_internal failed for arm-ffa-8001 with -EEXIST, don't try to register things with the same name in the same directory. ---- Fix the issue by calling device_unregister in __ffa_devices_unregister which will also take care of calling device_release(which is mapped to ffa_release_device) Link: https://lore.kernel.org/r/20210924092859.3057562-2-sudeep.holla@arm.com Fixes: e781858488b9 ("firmware: arm_ffa: Add initial FFA bus support for device enumeration") Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c index f01348e6cf1c..641a91819088 100644 --- a/drivers/firmware/arm_ffa/bus.c +++ b/drivers/firmware/arm_ffa/bus.c @@ -135,7 +135,7 @@ static void ffa_release_device(struct device *dev) static int __ffa_devices_unregister(struct device *dev, void *data) { - ffa_release_device(dev); + device_unregister(dev); return 0; } From 8e3f9da608f14cfebac2659d8dd8737b79d01308 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 13 Oct 2021 10:11:27 +0100 Subject: [PATCH 3/5] firmware: arm_ffa: Handle compatibility with different firmware versions The driver currently just support v1.0 of Arm FFA specification. It also expects the firmware implementation to match the same and bail out if it doesn't match. This is causing issue when running with higher version of firmware implementation(e.g. v1.1 which will released soon). In order to support compatibility with different firmware versions, let us add additional checks and find the compatible version the driver can work with. Link: https://lore.kernel.org/r/20211013091127.990992-1-sudeep.holla@arm.com Reviewed-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 37 ++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index c9fb56afbcb4..6e0c883ab708 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -167,6 +167,28 @@ struct ffa_drv_info { static struct ffa_drv_info *drv_info; +/* + * The driver must be able to support all the versions from the earliest + * supported FFA_MIN_VERSION to the latest supported FFA_DRIVER_VERSION. + * The specification states that if firmware supports a FFA implementation + * that is incompatible with and at a greater version number than specified + * by the caller(FFA_DRIVER_VERSION passed as parameter to FFA_VERSION), + * it must return the NOT_SUPPORTED error code. + */ +static u32 ffa_compatible_version_find(u32 version) +{ + u32 compat_version; + u16 major = MAJOR_VERSION(version), minor = MINOR_VERSION(version); + u16 drv_major = MAJOR_VERSION(FFA_DRIVER_VERSION); + u16 drv_minor = MINOR_VERSION(FFA_DRIVER_VERSION); + + if ((major < drv_major) || (major == drv_major && minor <= drv_minor)) + return version; + + pr_info("Firmware version higher than driver version, downgrading\n"); + return FFA_DRIVER_VERSION; +} + static int ffa_version_check(u32 *version) { ffa_value_t ver; @@ -180,15 +202,20 @@ static int ffa_version_check(u32 *version) return -EOPNOTSUPP; } - if (ver.a0 < FFA_MIN_VERSION || ver.a0 > FFA_DRIVER_VERSION) { - pr_err("Incompatible version %d.%d found\n", - MAJOR_VERSION(ver.a0), MINOR_VERSION(ver.a0)); + if (ver.a0 < FFA_MIN_VERSION) { + pr_err("Incompatible v%d.%d! Earliest supported v%d.%d\n", + MAJOR_VERSION(ver.a0), MINOR_VERSION(ver.a0), + MAJOR_VERSION(FFA_MIN_VERSION), + MINOR_VERSION(FFA_MIN_VERSION)); return -EINVAL; } - *version = ver.a0; - pr_info("Version %d.%d found\n", MAJOR_VERSION(ver.a0), + pr_info("Driver version %d.%d\n", MAJOR_VERSION(FFA_DRIVER_VERSION), + MINOR_VERSION(FFA_DRIVER_VERSION)); + pr_info("Firmware version %d.%d found\n", MAJOR_VERSION(ver.a0), MINOR_VERSION(ver.a0)); + *version = ffa_compatible_version_find(ver.a0); + return 0; } From 82a8daaecfd9382e9450a05f86be8a274cf69a27 Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Fri, 15 Oct 2021 17:57:42 +0100 Subject: [PATCH 4/5] firmware: arm_ffa: Add support for MEM_LEND As part of the FF-A spec, an endpoint is allowed to transfer access of, or lend, a memory region to one or more borrowers. Extend the existing memory sharing implementation to support FF-A MEM_LEND functionality and expose this to other kernel drivers. Note that upon a successful MEM_LEND request the caller must ensure that the memory region specified is not accessed until a successful MEM_RECALIM call has been made. On systems with a hypervisor present this will been enforced, however on systems without a hypervisor the responsibility falls to the calling kernel driver to prevent access. Link: https://lore.kernel.org/r/20211015165742.2513065-1-marc.bonnici@arm.com Reviewed-by: Jens Wiklander Signed-off-by: Marc Bonnici Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 17 +++++++++++++++++ include/linux/arm_ffa.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 6e0c883ab708..12f4c87c4555 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -613,6 +613,22 @@ ffa_memory_share(struct ffa_device *dev, struct ffa_mem_ops_args *args) return ffa_memory_ops(FFA_FN_NATIVE(MEM_SHARE), args); } +static int +ffa_memory_lend(struct ffa_device *dev, struct ffa_mem_ops_args *args) +{ + /* Note that upon a successful MEM_LEND request the caller + * must ensure that the memory region specified is not accessed + * until a successful MEM_RECALIM call has been made. + * On systems with a hypervisor present this will been enforced, + * however on systems without a hypervisor the responsibility + * falls to the calling kernel driver to prevent access. + */ + if (dev->mode_32bit) + return ffa_memory_ops(FFA_MEM_LEND, args); + + return ffa_memory_ops(FFA_FN_NATIVE(MEM_LEND), args); +} + static const struct ffa_dev_ops ffa_ops = { .api_version_get = ffa_api_version_get, .partition_info_get = ffa_partition_info_get, @@ -620,6 +636,7 @@ static const struct ffa_dev_ops ffa_ops = { .sync_send_receive = ffa_sync_send_receive, .memory_reclaim = ffa_memory_reclaim, .memory_share = ffa_memory_share, + .memory_lend = ffa_memory_lend, }; const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev) diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h index 505c679b6a9b..85651e41ded8 100644 --- a/include/linux/arm_ffa.h +++ b/include/linux/arm_ffa.h @@ -262,6 +262,8 @@ struct ffa_dev_ops { int (*memory_reclaim)(u64 g_handle, u32 flags); int (*memory_share)(struct ffa_device *dev, struct ffa_mem_ops_args *args); + int (*memory_lend)(struct ffa_device *dev, + struct ffa_mem_ops_args *args); }; #endif /* _LINUX_ARM_FFA_H */ From 01537a078b86917c7bb69aa4b756b42b980c158b Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Tue, 26 Oct 2021 10:33:50 +0200 Subject: [PATCH 5/5] firmware: arm_ffa: Remove unused 'compat_version' variable The newly added ffa_compatible_version_find() function causes a build warning because of a variable that is never used: drivers/firmware/arm_ffa/driver.c:180:6: error: unused variable 'compat_version' [-Werror,-Wunused-variable] u32 compat_version; Link: https://lore.kernel.org/r/20211026083400.3444946-1-arnd@kernel.org Fixes: 8e3f9da608f1 ("firmware: arm_ffa: Handle compatibility with different firmware versions") Signed-off-by: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 12f4c87c4555..14f900047ac0 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -177,7 +177,6 @@ static struct ffa_drv_info *drv_info; */ static u32 ffa_compatible_version_find(u32 version) { - u32 compat_version; u16 major = MAJOR_VERSION(version), minor = MINOR_VERSION(version); u16 drv_major = MAJOR_VERSION(FFA_DRIVER_VERSION); u16 drv_minor = MINOR_VERSION(FFA_DRIVER_VERSION);