A smalls set of fixes arounds FFA initialization routines.
Signed-off-by: Cristian Marussi <[email protected]>
---
Cristian Marussi (6):
firmware: arm_ffa: Add missing rwlock_init()
firmware: arm_ffa: Add missing host rwlock_init()
firmware: arm_ffa: Check xa_load() return value
firmware: arm_ffa: Simplify ffa_partitions_cleanup()
firmware: arm_ffa: Use xa_insert() and check for result
firmware: arm_ffa: Handle partitions setup failures
drivers/firmware/arm_ffa/driver.c | 85 ++++++++++++++++++++++++++-------------
1 file changed, 57 insertions(+), 28 deletions(-)
---
base-commit: dd6de5552d8d59db58952cc6ae34a15267d0b437
change-id: 20240108-ffa_fixes_6-8-ade3fcdd2595
Best regards,
--
Cristian Marussi <[email protected]>
Add missing rwlock initialization.
Fixes: 0184450b8b1e ("firmware: arm_ffa: Add schedule receiver callback mechanism")
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 6146b2927d5c..ed1d6a24934e 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1226,6 +1226,7 @@ static void ffa_setup_partitions(void)
ffa_device_unregister(ffa_dev);
continue;
}
+ rwlock_init(&info->rw_lock);
xa_store(&drv_info->partition_info, tpbuf->id, info, GFP_KERNEL);
}
drv_info->partition_count = count;
--
2.34.1
Add missing rwlock initialization for host partition setup.
Fixes: 1b6bf41b7a65 ("firmware: arm_ffa: Add notification handling mechanism")
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index ed1d6a24934e..8df92c9521f4 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1237,6 +1237,7 @@ static void ffa_setup_partitions(void)
info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info)
return;
+ rwlock_init(&info->rw_lock);
xa_store(&drv_info->partition_info, drv_info->vm_id, info, GFP_KERNEL);
drv_info->partition_count++;
}
--
2.34.1
Add a check to verify the result of xa_load() during partition lookups.
Fixes: 0184450b8b1e ("firmware: arm_ffa: Add schedule receiver callback mechanism")
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 8df92c9521f4..0ea1dd6e55c4 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -733,6 +733,11 @@ static void __do_sched_recv_cb(u16 part_id, u16 vcpu, bool is_per_vcpu)
void *cb_data;
partition = xa_load(&drv_info->partition_info, part_id);
+ if (!partition) {
+ pr_err("%s: Invalid partition ID 0x%x\n", __func__, part_id);
+ return;
+ }
+
read_lock(&partition->rw_lock);
callback = partition->callback;
cb_data = partition->cb_data;
@@ -915,6 +920,11 @@ static int ffa_sched_recv_cb_update(u16 part_id, ffa_sched_recv_cb callback,
return -EOPNOTSUPP;
partition = xa_load(&drv_info->partition_info, part_id);
+ if (!partition) {
+ pr_err("%s: Invalid partition ID 0x%x\n", __func__, part_id);
+ return -EINVAL;
+ }
+
write_lock(&partition->rw_lock);
cb_valid = !!partition->callback;
--
2.34.1
On cleanup iterate the XArrays with xa_for_each() and remove the existent
entries with xa_erase(), finally destroy the XArray itself.
Remove partition_count field from drv_info since no more used anywhwere.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 0ea1dd6e55c4..2426021dbb58 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -107,7 +107,6 @@ struct ffa_drv_info {
struct work_struct notif_pcpu_work;
struct work_struct irq_work;
struct xarray partition_info;
- unsigned int partition_count;
DECLARE_HASHTABLE(notifier_hash, ilog2(FFA_MAX_NOTIFICATIONS));
struct mutex notify_lock; /* lock to protect notifier hashtable */
};
@@ -1239,7 +1238,6 @@ static void ffa_setup_partitions(void)
rwlock_init(&info->rw_lock);
xa_store(&drv_info->partition_info, tpbuf->id, info, GFP_KERNEL);
}
- drv_info->partition_count = count;
kfree(pbuf);
@@ -1249,29 +1247,18 @@ static void ffa_setup_partitions(void)
return;
rwlock_init(&info->rw_lock);
xa_store(&drv_info->partition_info, drv_info->vm_id, info, GFP_KERNEL);
- drv_info->partition_count++;
}
static void ffa_partitions_cleanup(void)
{
- struct ffa_dev_part_info **info;
- int idx, count = drv_info->partition_count;
-
- if (!count)
- return;
-
- info = kcalloc(count, sizeof(*info), GFP_KERNEL);
- if (!info)
- return;
-
- xa_extract(&drv_info->partition_info, (void **)info, 0, VM_ID_MASK,
- count, XA_PRESENT);
+ struct ffa_dev_part_info *info;
+ unsigned long idx;
- for (idx = 0; idx < count; idx++)
- kfree(info[idx]);
- kfree(info);
+ xa_for_each(&drv_info->partition_info, idx, info) {
+ xa_erase(&drv_info->partition_info, idx);
+ kfree(info);
+ }
- drv_info->partition_count = 0;
xa_destroy(&drv_info->partition_info);
}
@@ -1547,7 +1534,6 @@ static void __exit ffa_exit(void)
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);
- xa_destroy(&drv_info->partition_info);
kfree(drv_info);
arm_ffa_bus_exit();
}
--
2.34.1
While adding new partitions descriptors to the XArray the outcome of the
stores should be checked and, in particular, it has also to be ensured
that an existing entry with the same index was not already present, since
partitions IDs are expected to be unique.
Use xa_insert() instead of xa_store() since it returns -EBUSY when the
index is already in use and log an error when that happens.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 2426021dbb58..c613b57747cf 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -1197,7 +1197,7 @@ void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid)
static void ffa_setup_partitions(void)
{
- int count, idx;
+ int count, idx, ret;
uuid_t uuid;
struct ffa_device *ffa_dev;
struct ffa_dev_part_info *info;
@@ -1236,7 +1236,14 @@ static void ffa_setup_partitions(void)
continue;
}
rwlock_init(&info->rw_lock);
- xa_store(&drv_info->partition_info, tpbuf->id, info, GFP_KERNEL);
+ ret = xa_insert(&drv_info->partition_info, tpbuf->id,
+ info, GFP_KERNEL);
+ if (ret) {
+ pr_err("%s: failed to save partition ID 0x%x - ret:%d\n",
+ __func__, tpbuf->id, ret);
+ ffa_device_unregister(ffa_dev);
+ kfree(info);
+ }
}
kfree(pbuf);
@@ -1246,7 +1253,13 @@ static void ffa_setup_partitions(void)
if (!info)
return;
rwlock_init(&info->rw_lock);
- xa_store(&drv_info->partition_info, drv_info->vm_id, info, GFP_KERNEL);
+ ret = xa_insert(&drv_info->partition_info, drv_info->vm_id,
+ info, GFP_KERNEL);
+ if (ret) {
+ pr_err("%s: failed to save Host partition ID 0x%x - ret:%d. Abort.\n",
+ __func__, drv_info->vm_id, ret);
+ kfree(info);
+ }
}
static void ffa_partitions_cleanup(void)
--
2.34.1
Make ffa_setup_partitions() fail, cleanup and return an error when the Host
partition setup fails: in such a case ffa_init() itself will fail.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_ffa/driver.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index c613b57747cf..f2556a8e9401 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -112,6 +112,7 @@ struct ffa_drv_info {
};
static struct ffa_drv_info *drv_info;
+static void ffa_partitions_cleanup(void);
/*
* The driver must be able to support all the versions from the earliest
@@ -1195,7 +1196,7 @@ void ffa_device_match_uuid(struct ffa_device *ffa_dev, const uuid_t *uuid)
kfree(pbuf);
}
-static void ffa_setup_partitions(void)
+static int ffa_setup_partitions(void)
{
int count, idx, ret;
uuid_t uuid;
@@ -1206,7 +1207,7 @@ static void ffa_setup_partitions(void)
count = ffa_partition_probe(&uuid_null, &pbuf);
if (count <= 0) {
pr_info("%s: No partitions found, error %d\n", __func__, count);
- return;
+ return -EINVAL;
}
xa_init(&drv_info->partition_info);
@@ -1250,8 +1251,14 @@ static void ffa_setup_partitions(void)
/* Allocate for the host */
info = kzalloc(sizeof(*info), GFP_KERNEL);
- if (!info)
- return;
+ if (!info) {
+ pr_err("%s: failed to alloc Host partition ID 0x%x. Abort.\n",
+ __func__, drv_info->vm_id);
+ /* Already registered devices are freed on bus_exit */
+ ffa_partitions_cleanup();
+ return -ENOMEM;
+ }
+
rwlock_init(&info->rw_lock);
ret = xa_insert(&drv_info->partition_info, drv_info->vm_id,
info, GFP_KERNEL);
@@ -1259,7 +1266,11 @@ static void ffa_setup_partitions(void)
pr_err("%s: failed to save Host partition ID 0x%x - ret:%d. Abort.\n",
__func__, drv_info->vm_id, ret);
kfree(info);
+ /* Already registered devices are freed on bus_exit */
+ ffa_partitions_cleanup();
}
+
+ return ret;
}
static void ffa_partitions_cleanup(void)
@@ -1520,7 +1531,11 @@ static int __init ffa_init(void)
ffa_notifications_setup();
- ffa_setup_partitions();
+ ret = ffa_setup_partitions();
+ if (ret) {
+ pr_err("failed to setup partitions\n");
+ goto cleanup_notifs;
+ }
ret = ffa_sched_recv_cb_update(drv_info->vm_id, ffa_self_notif_handle,
drv_info, true);
@@ -1528,6 +1543,9 @@ static int __init ffa_init(void)
pr_info("Failed to register driver sched callback %d\n", ret);
return 0;
+
+cleanup_notifs:
+ ffa_notifications_cleanup();
free_pages:
if (drv_info->tx_buffer)
free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
--
2.34.1
On Mon, 08 Jan 2024 12:34:10 +0000, Cristian Marussi wrote:
> A smalls set of fixes arounds FFA initialization routines.
>
Applied to sudeep.holla/linux (for-next/ffa/fixes), thanks!
[1/6] firmware: arm_ffa: Add missing rwlock_init()
https://git.kernel.org/sudeep.holla/c/59b2e242b131
[2/6] firmware: arm_ffa: Add missing host rwlock_init()
https://git.kernel.org/sudeep.holla/c/5ff30ade16cd
[3/6] firmware: arm_ffa: Check xa_load() return value
https://git.kernel.org/sudeep.holla/c/c00d9738fd5f
[4/6] firmware: arm_ffa: Simplify ffa_partitions_cleanup()
https://git.kernel.org/sudeep.holla/c/ad9d9a107a43
[5/6] firmware: arm_ffa: Use xa_insert() and check for result
https://git.kernel.org/sudeep.holla/c/ace760d9c049
[6/6] firmware: arm_ffa: Handle partitions setup failures
https://git.kernel.org/sudeep.holla/c/0c565d16b800
--
Regards,
Sudeep