[RFC PATCH 2/3] firmware: arm_ffa: initialise ff-a after finalising pKVM initialisation

Yeoreum Yun <[email protected]>
Newsgroups org.kernel.vger.linux-integrity,dev.linux.lists.kvmarm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.keyrings,org.kernel.vger.linux-kernel,org.kernel.vger.linux-security-module
Message-ID <[email protected]>
When pKVM is enabled, the FF-A driver must be initialised after pKVM.
Otherwise, pKVM cannot negotiate the FF-A version or obtain the RX/TX
buffer information, leading to failures in FF-A calls.

Currently, pKVM initialisation completes at device_initcall_sync,
while ffa_init() runs at the device_initcall level.

So far, linker deployes kvm_arm_init() before ffa_init(), and SMCs can
still be trapped even before finalise_pkvm() is invoked.
As a result, this issue has not been observed.

However, relying on above stuff is fragile.
Therefore, when pKVM is enabled, the FF-A infrastructure should be
initialised only after pKVM initialisation has been fully finalised.

To achieve this, introduce an ffa_root_dev ("arm-ffa") and
a corresponding driver to defer initialisation of the FF-A infrastructure
until pKVM initialisation is complete, and to defer probing of all FF-A devices until then
when pKVM is enabled.

This change slightly alters the sysfs layout.

Previously, all FF-A devices were located directly under /sys/devices/,
for example:

  ls /sys/devices/
  ...
  arm-ffa-1
  arm-ffa-2
  arm-ffa-3
  ...

After this patch, all FF-A devices are placed under the FF-A root device
/sys/devices/arm-ffa/:

  ls /sys/devices/arm-ffa/
  ...
  arm-ffa-1
  arm-ffa-2
  arm-ffa-3
  ...

Signed-off-by: Yeoreum Yun <[email protected]>
---
 drivers/firmware/arm_ffa/bus.c    | 125 ++++++++++++++++++++++++++++--
 drivers/firmware/arm_ffa/common.h |  13 +++-
 drivers/firmware/arm_ffa/driver.c |  21 +++--
 drivers/firmware/arm_ffa/smccc.c  |   2 +-
 4 files changed, 146 insertions(+), 15 deletions(-)

diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index 9576862d89c4..ddb352806d59 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -13,11 +13,14 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include <asm/virt.h>
+
 #include "common.h"
 
 #define FFA_UEVENT_MODALIAS_FMT	"arm_ffa:%04x:%pUb"
 
 static DEFINE_IDA(ffa_bus_id);
+static struct ffa_device *ffa_root_dev;
 
 static int ffa_device_match(struct device *dev, const struct device_driver *drv)
 {
@@ -27,7 +30,13 @@ static int ffa_device_match(struct device *dev, const struct device_driver *drv)
 	id_table = to_ffa_driver(drv)->id_table;
 	ffa_dev = to_ffa_dev(dev);
 
-	while (!uuid_is_null(&id_table->uuid)) {
+	if (is_ffa_root_device(ffa_dev)) {
+		if (!id_table)
+			return 1;
+		return 0;
+	}
+
+	while (id_table && !uuid_is_null(&id_table->uuid)) {
 		/*
 		 * FF-A v1.0 doesn't provide discovery of UUIDs, just the
 		 * partition IDs, so match it unconditionally here and handle
@@ -50,7 +59,7 @@ static int ffa_device_probe(struct device *dev)
 	struct ffa_device *ffa_dev = to_ffa_dev(dev);
 
 	/* UUID can be still NULL with FF-A v1.0, so just skip probing them */
-	if (uuid_is_null(&ffa_dev->uuid))
+	if (!is_ffa_root_device(ffa_dev) && uuid_is_null(&ffa_dev->uuid))
 		return -ENODEV;
 
 	return ffa_drv->probe(ffa_dev);
@@ -68,6 +77,9 @@ static int ffa_device_uevent(const struct device *dev, struct kobj_uevent_env *e
 {
 	const struct ffa_device *ffa_dev = to_ffa_dev(dev);
 
+	if (is_ffa_root_device(ffa_dev))
+		return 0;
+
 	return add_uevent_var(env, "MODALIAS=" FFA_UEVENT_MODALIAS_FMT,
 			      ffa_dev->vm_id, &ffa_dev->uuid);
 }
@@ -114,7 +126,6 @@ const struct bus_type ffa_bus_type = {
 	.probe		= ffa_device_probe,
 	.remove		= ffa_device_remove,
 	.uevent		= ffa_device_uevent,
-	.dev_groups	= ffa_device_attributes_groups,
 };
 EXPORT_SYMBOL_GPL(ffa_bus_type);
 
@@ -149,13 +160,18 @@ static void ffa_release_device(struct device *dev)
 {
 	struct ffa_device *ffa_dev = to_ffa_dev(dev);
 
-	ida_free(&ffa_bus_id, ffa_dev->id);
+	if (!is_ffa_root_device(ffa_dev))
+		ida_free(&ffa_bus_id, ffa_dev->id);
 	kfree(ffa_dev);
 }
 
 static int __ffa_devices_unregister(struct device *dev, void *data)
 {
-	device_unregister(dev);
+	struct ffa_device *ffa_dev = to_ffa_dev(dev);
+
+	/* ffa_root_dev will be removed in arm_ffa_bus_exit(). */
+	if (!is_ffa_root_device(ffa_dev))
+		device_unregister(dev);
 
 	return 0;
 }
@@ -195,6 +211,10 @@ ffa_device_register(const struct ffa_partition_info *part_info,
 	int id, ret;
 	struct device *dev;
 	struct ffa_device *ffa_dev;
+	struct device_link *link;
+
+	if (!ffa_root_dev)
+		return NULL;
 
 	if (!part_info)
 		return NULL;
@@ -213,6 +233,8 @@ ffa_device_register(const struct ffa_partition_info *part_info,
 	dev->bus = &ffa_bus_type;
 	dev->release = ffa_release_device;
 	dev->dma_mask = &dev->coherent_dma_mask;
+	dev->parent = &ffa_root_dev->dev;
+	dev->groups = ffa_device_attributes_groups;
 	dev_set_name(&ffa_dev->dev, "arm-ffa-%d", id);
 
 	ffa_dev->id = id;
@@ -221,7 +243,18 @@ ffa_device_register(const struct ffa_partition_info *part_info,
 	ffa_dev->ops = ops;
 	uuid_copy(&ffa_dev->uuid, &part_info->uuid);
 
-	ret = device_register(&ffa_dev->dev);
+	device_initialize(dev);
+
+	link = device_link_add(dev, &ffa_root_dev->dev,
+			       DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOPROBE_CONSUMER);
+	if (!link) {
+		dev_err(dev, "unable to link device %s\n", dev_name(dev));
+		device_link_del(link);
+		put_device(dev);
+		return NULL;
+	}
+
+	ret = device_add(dev);
 	if (ret) {
 		dev_err(dev, "unable to register device %s err=%d\n",
 			dev_name(dev), ret);
@@ -242,15 +275,93 @@ void ffa_device_unregister(struct ffa_device *ffa_dev)
 }
 EXPORT_SYMBOL_GPL(ffa_device_unregister);
 
+static int __init ffa_root_device_register(void)
+{
+	int ret;
+	struct device *dev;
+	struct ffa_device *ffa_dev;
+
+	ffa_dev = kzalloc_obj(*ffa_dev);
+	if (!ffa_dev) {
+		return -ENOMEM;
+	}
+
+	dev = &ffa_dev->dev;
+	dev->bus = &ffa_bus_type;
+	dev->release = ffa_release_device;
+	dev->dma_mask = &dev->coherent_dma_mask;
+	dev_set_name(&ffa_dev->dev, "%s", "arm-ffa");
+
+	ffa_dev->id = FFA_ROOT_DEV_ID;
+
+	ret = device_register(&ffa_dev->dev);
+	if (ret) {
+		dev_err(dev, "unable to register root device. err=%d\n", ret);
+		put_device(dev);
+		return ret;
+	}
+
+	ffa_root_dev = ffa_dev;
+
+	return 0;
+}
+
+static void ffa_root_device_unregister(void)
+{
+	ffa_device_unregister(ffa_root_dev);
+	ffa_root_dev = NULL;
+}
+
+static int ffa_root_device_probe(struct ffa_device *ffa_dev)
+{
+	if (!is_ffa_root_device(ffa_dev))
+		return -EINVAL;
+
+	if (is_protected_kvm_enabled() && !is_pkvm_initialized())
+		return -EPROBE_DEFER;
+
+	return ffa_core_init();
+}
+
+static struct ffa_driver ffa_root_dev_driver = {
+	.name = "arm-ffa",
+	.probe = ffa_root_device_probe,
+	.id_table = NULL,
+};
+
+int ffa_root_device_driver_register(void)
+{
+	return ffa_driver_register(&ffa_root_dev_driver, THIS_MODULE, KBUILD_MODNAME);
+}
+
+static void ffa_root_device_driver_unregister(void)
+{
+	ffa_driver_unregister(&ffa_root_dev_driver);
+}
+
 static int __init arm_ffa_bus_init(void)
 {
-	return bus_register(&ffa_bus_type);
+	int ret;
+
+	ret = bus_register(&ffa_bus_type);
+	if (ret)
+		return ret;
+
+	ret = ffa_root_device_register();
+	if (ret) {
+		bus_unregister(&ffa_bus_type);
+		return ret;
+	}
+
+	return 0;
 }
 subsys_initcall(arm_ffa_bus_init);
 
 static void __exit arm_ffa_bus_exit(void)
 {
 	ffa_devices_unregister();
+	ffa_root_device_unregister();
+	ffa_root_device_driver_unregister();
 	bus_unregister(&ffa_bus_type);
 	ida_destroy(&ffa_bus_id);
 }
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index 9c6425a81d0d..32ffe41c3cfc 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -10,17 +10,26 @@
 #include <linux/arm-smccc.h>
 #include <linux/err.h>
 
+#define FFA_ROOT_DEV_ID	(0)
+
 typedef struct arm_smccc_1_2_regs ffa_value_t;
 
 typedef void (ffa_fn)(ffa_value_t, ffa_value_t *);
 
 bool ffa_device_is_valid(struct ffa_device *ffa_dev);
 void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid);
+int ffa_root_device_driver_register(void);
+int ffa_core_init(void);
+
+static __always_inline bool is_ffa_root_device(const struct ffa_device *ffa_dev)
+{
+	return ffa_dev->id == FFA_ROOT_DEV_ID;
+}
 
 #ifdef CONFIG_ARM_FFA_SMCCC
-int __init ffa_transport_init(ffa_fn **invoke_ffa_fn);
+int ffa_transport_init(ffa_fn **invoke_ffa_fn);
 #else
-static inline int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+static inline int ffa_transport_init(ffa_fn **invoke_ffa_fn)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index eb2782848283..c41e7ed4eac8 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1610,6 +1610,9 @@ ffa_bus_notifier(struct notifier_block *nb, unsigned long action, void *data)
 		struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver);
 		const struct ffa_device_id *id_table = ffa_drv->id_table;
 
+		if (is_ffa_root_device(fdev))
+			return NOTIFY_DONE;
+
 		/*
 		 * FF-A v1.1 provides UUID for each partition as part of the
 		 * discovery API, the discovered UUID must be populated in the
@@ -2029,7 +2032,7 @@ static void ffa_notifications_setup(void)
 	ffa_notifications_cleanup();
 }
 
-static int __init ffa_init(void)
+int ffa_core_init(void)
 {
 	int ret;
 	u32 buf_sz;
@@ -2106,16 +2109,24 @@ static int __init ffa_init(void)
 	kfree(drv_info);
 	return ret;
 }
-rootfs_initcall(ffa_init);
+
+static int __init ffa_init(void)
+{
+	return ffa_root_device_driver_register();
+}
+device_initcall(ffa_init);
 
 static void __exit ffa_exit(void)
 {
 	ffa_notifications_cleanup();
 	ffa_partitions_cleanup();
 	ffa_rxtx_unmap();
-	free_pages_exact(drv_info->tx_buffer, drv_info->rxtx_bufsz);
-	free_pages_exact(drv_info->rx_buffer, drv_info->rxtx_bufsz);
-	kfree(drv_info);
+
+	if (drv_info) {
+		free_pages_exact(drv_info->tx_buffer, drv_info->rxtx_bufsz);
+		free_pages_exact(drv_info->rx_buffer, drv_info->rxtx_bufsz);
+		kfree(drv_info);
+	}
 }
 module_exit(ffa_exit);
 
diff --git a/drivers/firmware/arm_ffa/smccc.c b/drivers/firmware/arm_ffa/smccc.c
index 4d85bfff0a4e..e6125dd9f58f 100644
--- a/drivers/firmware/arm_ffa/smccc.c
+++ b/drivers/firmware/arm_ffa/smccc.c
@@ -17,7 +17,7 @@ static void __arm_ffa_fn_hvc(ffa_value_t args, ffa_value_t *res)
 	arm_smccc_1_2_hvc(&args, res);
 }
 
-int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+int ffa_transport_init(ffa_fn **invoke_ffa_fn)
 {
 	enum arm_smccc_conduit conduit;
 
-- 
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.