From 95520fc07743d3f58e8872acd72e928b09fbc143 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 24 Oct 2023 11:56:17 +0100 Subject: firmware: arm_ffa: Allow FF-A initialisation even when notification fails FF-A notifications are optional feature in the specification. Currently we allow to continue if the firmware reports no support for the notifications. However, we fail to continue and complete the FF-A driver initialisation if the notification setup fails for any reason. Let us allow the FF-A driver to complete the initialisation even if the notification fails to setup. We will just flag the error and continue to provide other features in the driver. Link: https://lore.kernel.org/r/20231024-ffa-notification-fixes-v1-1-d552c0ec260d@arm.com Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 07b72c679247..585632a444b4 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -1390,20 +1390,20 @@ static void ffa_notifications_cleanup(void) } } -static int ffa_notifications_setup(void) +static void ffa_notifications_setup(void) { int ret, irq; ret = ffa_features(FFA_NOTIFICATION_BITMAP_CREATE, 0, NULL, NULL); if (ret) { - pr_err("Notifications not supported, continuing with it ..\n"); - return 0; + pr_info("Notifications not supported, continuing with it ..\n"); + return; } ret = ffa_notification_bitmap_create(); if (ret) { - pr_err("notification_bitmap_create error %d\n", ret); - return ret; + pr_info("Notification bitmap create error %d\n", ret); + return; } drv_info->bitmap_created = true; @@ -1426,10 +1426,10 @@ static int ffa_notifications_setup(void) ret = ffa_sched_recv_cb_update(drv_info->vm_id, ffa_self_notif_handle, drv_info, true); if (!ret) - return ret; + return; cleanup: + pr_info("Notification setup failed %d, not enabled\n", ret); ffa_notifications_cleanup(); - return ret; } static int __init ffa_init(void) @@ -1487,13 +1487,9 @@ static int __init ffa_init(void) ffa_set_up_mem_ops_native_flag(); - ret = ffa_notifications_setup(); - if (ret) - goto partitions_cleanup; + ffa_notifications_setup(); return 0; -partitions_cleanup: - ffa_partitions_cleanup(); free_pages: if (drv_info->tx_buffer) free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE); -- cgit v1.2.3 From 6f47023f7a52f3482937e9271ba41570b8752067 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 24 Oct 2023 11:56:18 +0100 Subject: firmware: arm_ffa: Setup the partitions after the notification initialisation Currently the notifications are setup of the partitions are probed and FF-A devices are added on the FF-A bus. The FF-A driver probe can be called even before the FF-A notification setup happens which is wrong and may result in failure or misbehaviour in the FF-A partition device probe. In order to ensure the FF-A notifications are setup before the FF-A devices are probed, let us move the FF-A partition setup after the completion of FF-A notification setup. Link: https://lore.kernel.org/r/20231024-ffa-notification-fixes-v1-2-d552c0ec260d@arm.com Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 585632a444b4..69ad3add3175 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -1422,11 +1422,7 @@ static void ffa_notifications_setup(void) hash_init(drv_info->notifier_hash); mutex_init(&drv_info->notify_lock); - /* Register internal scheduling callback */ - ret = ffa_sched_recv_cb_update(drv_info->vm_id, ffa_self_notif_handle, - drv_info, true); - if (!ret) - return; + return; cleanup: pr_info("Notification setup failed %d, not enabled\n", ret); ffa_notifications_cleanup(); @@ -1483,12 +1479,17 @@ static int __init ffa_init(void) mutex_init(&drv_info->rx_lock); mutex_init(&drv_info->tx_lock); - ffa_setup_partitions(); - ffa_set_up_mem_ops_native_flag(); ffa_notifications_setup(); + ffa_setup_partitions(); + + ret = ffa_sched_recv_cb_update(drv_info->vm_id, ffa_self_notif_handle, + drv_info, true); + if (ret) + pr_info("Failed to register driver sched callback %d\n", ret); + return 0; free_pages: if (drv_info->tx_buffer) -- cgit v1.2.3 From f4bfcaee34bc9527274710884f8d14039f3ee506 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 24 Oct 2023 11:56:19 +0100 Subject: firmware: arm_ffa: Add checks for the notification enabled state We need to check if the FF-A notifications are enabled or not in all the notification operations that are accessible for the FF-A device from the FF-A driver. This helps to avoid making calls to the FF-A firmware even if the notification setup has failed. Link: https://lore.kernel.org/r/20231024-ffa-notification-fixes-v1-3-d552c0ec260d@arm.com Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 69ad3add3175..1724a0cbb2cf 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -99,6 +99,7 @@ struct ffa_drv_info { void *tx_buffer; bool mem_ops_native; bool bitmap_created; + bool notif_enabled; unsigned int sched_recv_irq; unsigned int cpuhp_state; struct ffa_pcpu_irq __percpu *irq_pcpu; @@ -889,6 +890,8 @@ static int ffa_memory_lend(struct ffa_mem_ops_args *args) #define FFA_SECURE_PARTITION_ID_FLAG BIT(15) +#define ffa_notifications_disabled() (!drv_info->notif_enabled) + enum notify_type { NON_SECURE_VM, SECURE_PARTITION, @@ -908,6 +911,9 @@ static int ffa_sched_recv_cb_update(u16 part_id, ffa_sched_recv_cb callback, struct ffa_dev_part_info *partition; bool cb_valid; + if (ffa_notifications_disabled()) + return -EOPNOTSUPP; + partition = xa_load(&drv_info->partition_info, part_id); write_lock(&partition->rw_lock); @@ -1001,6 +1007,9 @@ static int ffa_notify_relinquish(struct ffa_device *dev, int notify_id) int rc; enum notify_type type = ffa_notify_type_get(dev->vm_id); + if (ffa_notifications_disabled()) + return -EOPNOTSUPP; + if (notify_id >= FFA_MAX_NOTIFICATIONS) return -EINVAL; @@ -1027,6 +1036,9 @@ static int ffa_notify_request(struct ffa_device *dev, bool is_per_vcpu, u32 flags = 0; enum notify_type type = ffa_notify_type_get(dev->vm_id); + if (ffa_notifications_disabled()) + return -EOPNOTSUPP; + if (notify_id >= FFA_MAX_NOTIFICATIONS) return -EINVAL; @@ -1057,6 +1069,9 @@ static int ffa_notify_send(struct ffa_device *dev, int notify_id, { u32 flags = 0; + if (ffa_notifications_disabled()) + return -EOPNOTSUPP; + if (is_per_vcpu) flags |= (PER_VCPU_NOTIFICATION_FLAG | vcpu << 16); @@ -1388,6 +1403,7 @@ static void ffa_notifications_cleanup(void) ffa_notification_bitmap_destroy(); drv_info->bitmap_created = false; } + drv_info->notif_enabled = false; } static void ffa_notifications_setup(void) @@ -1422,6 +1438,7 @@ static void ffa_notifications_setup(void) hash_init(drv_info->notifier_hash); mutex_init(&drv_info->notify_lock); + drv_info->notif_enabled = true; return; cleanup: pr_info("Notification setup failed %d, not enabled\n", ret); -- cgit v1.2.3 From 6d67cbe67a86a87307df0c6fafa74394a6820ad6 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 24 Oct 2023 11:56:20 +0100 Subject: firmware: arm_ffa: Fix FFA notifications cleanup path We allow the FF-A to be initialised successfully even when notification fails. When the notification fails, ffa_notifications_cleanup() gets called on the failure path. However, the driver information about the notifications like the irq, workqueues and cpu hotplug state for enabling and disabling percpu IRQ are not cleared. This may result in unexpected behaviour during CPU hotplug because of percpu IRQ being enabled and disabled or during the driver removal when ffa_notifications_cleanup() gets executed again. Fix the FFA notifications cleanup path by clearing all the notification related driver information. Link: https://lore.kernel.org/r/20231024-ffa-notification-fixes-v1-4-d552c0ec260d@arm.com Tested-by: Jens Wiklander Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 1724a0cbb2cf..49ebdc2775dd 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -1326,8 +1326,10 @@ static int ffa_sched_recv_irq_map(void) static void ffa_sched_recv_irq_unmap(void) { - if (drv_info->sched_recv_irq) + if (drv_info->sched_recv_irq) { irq_dispose_mapping(drv_info->sched_recv_irq); + drv_info->sched_recv_irq = 0; + } } static int ffa_cpuhp_pcpu_irq_enable(unsigned int cpu) @@ -1344,17 +1346,23 @@ static int ffa_cpuhp_pcpu_irq_disable(unsigned int cpu) static void ffa_uninit_pcpu_irq(void) { - if (drv_info->cpuhp_state) + if (drv_info->cpuhp_state) { cpuhp_remove_state(drv_info->cpuhp_state); + drv_info->cpuhp_state = 0; + } - if (drv_info->notif_pcpu_wq) + if (drv_info->notif_pcpu_wq) { destroy_workqueue(drv_info->notif_pcpu_wq); + drv_info->notif_pcpu_wq = NULL; + } if (drv_info->sched_recv_irq) free_percpu_irq(drv_info->sched_recv_irq, drv_info->irq_pcpu); - if (drv_info->irq_pcpu) + if (drv_info->irq_pcpu) { free_percpu(drv_info->irq_pcpu); + drv_info->irq_pcpu = NULL; + } } static int ffa_init_pcpu_irq(unsigned int irq) -- cgit v1.2.3 From 05857a1eb723190923b091a857184ced17a83770 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 31 Oct 2023 14:13:35 +0000 Subject: firmware: arm_ffa: Fix the size of the allocation in ffa_partitions_cleanup() Arry of pointer to struct ffa_dev_part_info needs to be allocated to fetch the pointers stored in XArray. However, currently we allocate the entire structure instead of just pointers. Fix the allocation size. This will also eliminate the below Smatch istatic checker warning: | drivers/firmware/arm_ffa/driver.c:1251 ffa_partitions_cleanup() | warn: double check that we're allocating correct size: 8 vs 88 Reported-by: Dan Carpenter Closes: https://lore.kernel.org/all/0e8ddbca-d9da-4a3b-aae3-328993b62ba2@moroto.mountain Link: https://lore.kernel.org/r/20231031141335.3077026-1-sudeep.holla@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 49ebdc2775dd..034bce8a2ca1 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -1248,7 +1248,7 @@ static void ffa_partitions_cleanup(void) if (!count) return; - info = kcalloc(count, sizeof(**info), GFP_KERNEL); + info = kcalloc(count, sizeof(*info), GFP_KERNEL); if (!info) return; -- cgit v1.2.3 From f1ed48ef97e2d12dee21e42db4a6ebb895ed3a79 Mon Sep 17 00:00:00 2001 From: Lorenzo Pieralisi Date: Wed, 8 Nov 2023 12:15:49 +0100 Subject: firmware: arm_ffa: Fix ffa_notification_info_get() IDs handling To parse the retrieved ID lists appropriately in ffa_notification_info_get() the ids_processed variable should not be pre-incremented - we are dropping an identifier at the beginning of the list. Fix it by using the post-increment operator to increment the number of processed IDs. Fixes: 3522be48d82b ("firmware: arm_ffa: Implement the NOTIFICATION_INFO_GET interface") Signed-off-by: Lorenzo Pieralisi Cc: Sudeep Holla Link: https://lore.kernel.org/r/20231108111549.155974-1-lpieralisi@kernel.org Signed-off-by: Sudeep Holla --- drivers/firmware/arm_ffa/driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c index 034bce8a2ca1..6146b2927d5c 100644 --- a/drivers/firmware/arm_ffa/driver.c +++ b/drivers/firmware/arm_ffa/driver.c @@ -783,7 +783,7 @@ static void ffa_notification_info_get(void) if (ids_processed >= max_ids - 1) break; - part_id = packed_id_list[++ids_processed]; + part_id = packed_id_list[ids_processed++]; if (!ids_count[list]) { /* Global Notification */ __do_sched_recv_cb(part_id, 0, false); @@ -795,7 +795,7 @@ static void ffa_notification_info_get(void) if (ids_processed >= max_ids - 1) break; - vcpu_id = packed_id_list[++ids_processed]; + vcpu_id = packed_id_list[ids_processed++]; __do_sched_recv_cb(part_id, vcpu_id, true); } -- cgit v1.2.3 From 01b1e3ca0e5ce47bbae8217d47376ad01b331b07 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Fri, 3 Nov 2023 10:13:54 -0500 Subject: efi/unaccepted: Fix off-by-one when checking for overlapping ranges When a task needs to accept memory it will scan the accepting_list to see if any ranges already being processed by other tasks overlap with its range. Due to an off-by-one in the range comparisons, a task might falsely determine that an overlapping range is being accepted, leading to an unnecessary delay before it begins processing the range. Fix the off-by-one in the range comparison to prevent this and slightly improve performance. Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 Reviewed-by: Kirill A. Shutemov Signed-off-by: Michael Roth Acked-by: Vlastimil Babka Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/unaccepted_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c index 3f2f7bf6e335..5b439d04079c 100644 --- a/drivers/firmware/efi/unaccepted_memory.c +++ b/drivers/firmware/efi/unaccepted_memory.c @@ -101,7 +101,7 @@ retry: * overlap on physical address level. */ list_for_each_entry(entry, &accepting_list, list) { - if (entry->end < range.start) + if (entry->end <= range.start) continue; if (entry->start >= range.end) continue; -- cgit v1.2.3 From 8e3c98d9187e09274fc000a7d1a77b070a42d259 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 30 Nov 2023 20:43:42 +0000 Subject: firmware: arm_scmi: Fix frequency truncation by promoting multiplier type Fix the possible frequency truncation for all values equal to or greater 4GHz on 64bit machines by updating the multiplier 'mult_factor' to 'unsigned long' type. It is also possible that the multiplier itself can be greater than or equal to 2^32. So we need to also fix the equation computing the value of the multiplier. Fixes: a9e3fbfaa0ff ("firmware: arm_scmi: add initial support for performance protocol") Reported-by: Sibi Sankar Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ Cc: Cristian Marussi Link: https://lore.kernel.org/r/20231130204343.503076-1-sudeep.holla@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/perf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index c2435be0ae1b..f98791274495 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -152,7 +152,7 @@ struct perf_dom_info { u32 opp_count; u32 sustained_freq_khz; u32 sustained_perf_level; - u32 mult_factor; + unsigned long mult_factor; struct scmi_perf_domain_info info; struct scmi_opp opp[MAX_OPPS]; struct scmi_fc_info *fc_info; @@ -273,8 +273,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, dom_info->mult_factor = 1000; else dom_info->mult_factor = - (dom_info->sustained_freq_khz * 1000) / - dom_info->sustained_perf_level; + (dom_info->sustained_freq_khz * 1000UL) + / dom_info->sustained_perf_level; strscpy(dom_info->info.name, attr->name, SCMI_SHORT_NAME_MAX_SIZE); } -- cgit v1.2.3 From 77f5032e94f244ba08db51e17ca8f37bd7ff9acb Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 30 Nov 2023 20:43:43 +0000 Subject: firmware: arm_scmi: Fix possible frequency truncation when using level indexing mode The multiplier is already promoted to unsigned long, however the frequency calculations done when using level indexing mode doesn't use the multiplier computed. It instead hardcodes the multiplier value of 1000 at all the usage sites. Clean that up by assigning the multiplier value of 1000 when using the perf level indexing mode and update the frequency calculations to use the multiplier instead. It should fix the possible frequency truncation for all the values greater than or equal to 4GHz on 64-bit machines. Fixes: 31c7c1397a33 ("firmware: arm_scmi: Add v3.2 perf level indexing mode support") Reported-by: Sibi Sankar Closes: https://lore.kernel.org/all/20231129065748.19871-3-quic_sibis@quicinc.com/ Cc: Cristian Marussi Link: https://lore.kernel.org/r/20231130204343.503076-2-sudeep.holla@arm.com Reviewed-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/perf.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index f98791274495..e11555de99ab 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -268,7 +268,8 @@ scmi_perf_domain_attributes_get(const struct scmi_protocol_handle *ph, dom_info->sustained_perf_level = le32_to_cpu(attr->sustained_perf_level); if (!dom_info->sustained_freq_khz || - !dom_info->sustained_perf_level) + !dom_info->sustained_perf_level || + dom_info->level_indexing_mode) /* CPUFreq converts to kHz, hence default 1000 */ dom_info->mult_factor = 1000; else @@ -798,7 +799,7 @@ static int scmi_dvfs_device_opps_add(const struct scmi_protocol_handle *ph, if (!dom->level_indexing_mode) freq = dom->opp[idx].perf * dom->mult_factor; else - freq = dom->opp[idx].indicative_freq * 1000; + freq = dom->opp[idx].indicative_freq * dom->mult_factor; data.level = dom->opp[idx].perf; data.freq = freq; @@ -845,7 +846,8 @@ static int scmi_dvfs_freq_set(const struct scmi_protocol_handle *ph, u32 domain, } else { struct scmi_opp *opp; - opp = LOOKUP_BY_FREQ(dom->opps_by_freq, freq / 1000); + opp = LOOKUP_BY_FREQ(dom->opps_by_freq, + freq / dom->mult_factor); if (!opp) return -EIO; @@ -879,7 +881,7 @@ static int scmi_dvfs_freq_get(const struct scmi_protocol_handle *ph, u32 domain, if (!opp) return -EIO; - *freq = opp->indicative_freq * 1000; + *freq = opp->indicative_freq * dom->mult_factor; } return ret; @@ -902,7 +904,7 @@ static int scmi_dvfs_est_power_get(const struct scmi_protocol_handle *ph, if (!dom->level_indexing_mode) opp_freq = opp->perf * dom->mult_factor; else - opp_freq = opp->indicative_freq * 1000; + opp_freq = opp->indicative_freq * dom->mult_factor; if (opp_freq < *freq) continue; -- cgit v1.2.3