From nobody Thu Sep 24 18:44:56 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8D8FB313E38 for ; Tue, 22 Sep 2026 00:54:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790038444; cv=none; b=l2TUZypCrumCwg0tZ0RyhmHBaQHh0wGzjtIrA2VAuW+TsiQiF+7nRARY7LBKoEmsrymou3dz98OL6lZNMB7roZ5Ha5MhJKgzgnT4OSrOxXgM9yVjoq/jws26G0P4OKqSaapq+9WhU0hnoJgGybsGh9VrzLLZ5VUAYE3RgbzFfC4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790038444; c=relaxed/simple; bh=sj6104KbgOlyuK8myB1cNzwOZ/rILyLpvYo64aFNZIg=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:To:Cc; b=jY5X51mxqHrKvb+MN1xiFdLUpa4w+m8qY291phIaxNe7fMpEB0+tLdakz0wi7S2Si9xPxfE6OG8jfdyiuf+8YvnJsI4wRqCwcTqEMHHfFCj19ubB0hQjCVN52M8mPEQuJ2NEbwTDP2bQ+a7dJYZrvrtlg1eKkGEtpkvQoas1KX8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=u5NOsjuX; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="u5NOsjuX" Received: by smtp.kernel.org (Postfix) with ESMTPS id 053B7C2BCB3; Tue, 22 Sep 2026 00:54:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1790038444; bh=sj6104KbgOlyuK8myB1cNzwOZ/rILyLpvYo64aFNZIg=; h=From:Date:Subject:To:Cc:Reply-To:From; b=u5NOsjuXEb5e9MHSh5jijdqtuyc/zjJhhFMDVXaCVHhE4aBJWrnRyp8CypG56UlKD SagxpTDeFSS4Hx0hvryND/IqqdcVErWREmfgGf0hBEemC90+Da0WJYWoDkW8nvCjOQ uakx/hmitl1m4iFnYakymef6RFVq9m6UsoHADbc79FcwNyVC45wWSF+6OhJ1biceFw 2EZJEYDuo0Im/PowPGfDp3bTJWhty/eIAZixv1Q7GNGjcwyuXAOOypWpgFzRXG/2zu fe+mu6lavElYf6+U8tOQ9W1MHoPKoCc+Ceh3aN1IWB/aRZXYYBbuMsVBpzAsf648gQ bi0/0blDZlTFg== Received: from aws-us-west-2-korg-lkml-1.web.codeaurora.org (localhost.localdomain [127.0.0.1]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC281C982F0; Tue, 22 Sep 2026 00:54:03 +0000 (UTC) From: Jaidev Shastri via B4 Relay Date: Mon, 21 Sep 2026 20:54:00 -0400 Subject: [PATCH] firmware: xilinx: zynqmp: serialise and publish the feature check cache Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20260921-mb-zynqmp-fw-v1-1-1b7f9773df46@vt.edu> X-B4-Tracking: v=1; b=H4sIAKfRsWoC/yXMzQ6CMBAE4Fche3ZNKYjRVzEe+rPImlBxK6AS3 t0Wj99kZhaIJEwRzsUCQhNHfoSEcleA60y4EbJPBq10o066xN7i9xOe/YDtjMrWlTroxhtvIE0 GoZbf293l+ncc7Z3cK3/khjWR0IoJrsvRxum4r1BcDev6A+kiiJ+QAAAA X-Change-ID: 20260921-mb-zynqmp-fw-0b430526dada To: Michal Simek Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jaidev Shastri X-Mailer: b4 0.16.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1790038443; l=10397; i=jaidevshastri@vt.edu; s=20260921; h=from:subject:message-id; bh=ld144bmnJCTHlODiihZvEWWLWKVWTqPEqRizGBX4ouo=; b=gDekVjwALg8OAzkZTix3rqQ6vJnBuSRB6bhgsQAHlbptwQJ4BFkw8/PCNVboPQa3CJyY4bfVr OKT+kZ8Uts7Adje9rM5grdhcXAp8ucVxkqK9q/MVowqPbaVGn6TpFOX X-Developer-Key: i=jaidevshastri@vt.edu; a=ed25519; pk=J7+xYJRlTPds+pv5hbqFFRqGCpDeJDzmZT1ggRwj7/0= X-Endpoint-Received: by B4 Relay for jaidevshastri@vt.edu/20260921 with auth_id=1044 X-Original-From: Jaidev Shastri Reply-To: jaidevshastri@vt.edu From: Jaidev Shastri Every firmware call reaches do_feature_check_call() through zynqmp_pm_invoke_fn() and zynqmp_pm_feature(). It caches the firmware's answers in pm_api_features_map and extends that hash table on a miss with hash_add(), taking no lock. Clock, pinctrl, reset, power domain and GPIO operations all reach it, from any CPU, concurrently. Two CPUs that miss on the same bucket both call hlist_add_head() on it and corrupt the list. hash_add() is also a plain store of the node pointer, so a CPU walking the bucket can see the node before pm_api_id and feature_status are visible and return an answer meant for a different API. The IOCTL and QUERY masks are copied into ioctl_features and query_features after the entry is published, so zynqmp_pm_is_function_supported() can find the entry and read an all zero mask, reporting every IOCTL as unsupported. Serialise insertions with a spinlock, recheck the bucket under it and publish with hash_add_rcu(). Look up under rcu_read_lock() with hash_for_each_possible_rcu(). Move the IOCTL and QUERY masks into the entry so that they are published with it, and free entries with kfree_rcu(). The lock is taken with interrupts disabled because firmware calls are made from atomic context, for example under the clock enable lock. Found with MBCheck, a static herd7-based memory consistency checker. Signed-off-by: Jaidev Shastri --- drivers/firmware/xilinx/zynqmp.c | 108 +++++++++++++++++++++++++++++------= ---- 1 file changed, 80 insertions(+), 28 deletions(-) diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zyn= qmp.c index fe650747a..3d395d5a4 100644 --- a/drivers/firmware/xilinx/zynqmp.c +++ b/drivers/firmware/xilinx/zynqmp.c @@ -25,6 +25,8 @@ #include #include #include +#include +#include =20 #include #include @@ -45,8 +47,8 @@ =20 static bool feature_check_enabled; static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDE= R); -static u32 ioctl_features[FEATURE_PAYLOAD_SIZE]; -static u32 query_features[FEATURE_PAYLOAD_SIZE]; +/* Serialises insertions into pm_api_features_map; lookups run under RCU. = */ +static DEFINE_SPINLOCK(pm_api_features_lock); =20 static u32 sip_svc_version; static struct platform_device *em_dev; @@ -65,12 +67,16 @@ struct zynqmp_devinfo { * struct pm_api_feature_data - PM API Feature data * @pm_api_id: PM API Id, used as key to index into hashmap * @feature_status: status of PM API feature: valid, invalid + * @features: supported IOCTL/QUERY IDs mask reported by the firmware * @hentry: hlist_node that hooks this entry into hashtable + * @rcu: used to free the entry after a grace period */ struct pm_api_feature_data { u32 pm_api_id; int feature_status; + u32 features[FEATURE_PAYLOAD_SIZE]; struct hlist_node hentry; + struct rcu_head rcu; }; =20 struct platform_fw_data { @@ -257,18 +263,46 @@ static int __do_feature_check_call(const u32 api_id, = u32 *ret_payload) return ret_payload[1]; } =20 -static int do_feature_check_call(const u32 api_id) +/* + * Look up the cached firmware answer for @api_id. Called under + * rcu_read_lock() or with pm_api_features_lock held. + */ +static struct pm_api_feature_data *pm_api_feature_lookup(const u32 api_id) +{ + struct pm_api_feature_data *feature_data; + + hash_for_each_possible_rcu(pm_api_features_map, feature_data, hentry, + api_id, lockdep_is_held(&pm_api_features_lock)) { + if (feature_data->pm_api_id =3D=3D api_id) + return feature_data; + } + + return NULL; +} + +/* + * Return the feature status of @api_id, asking the firmware on the first + * call. When @features is not NULL, the IOCTL/QUERY ID mask the firmware + * reported for @api_id is copied into it. + */ +static int do_feature_check_call(const u32 api_id, u32 *features) { int ret; u32 ret_payload[PAYLOAD_ARG_CNT]; - struct pm_api_feature_data *feature_data; + struct pm_api_feature_data *feature_data, *found; + unsigned long flags; =20 /* Check for existing entry in hash table for given api */ - hash_for_each_possible(pm_api_features_map, feature_data, hentry, - api_id) { - if (feature_data->pm_api_id =3D=3D api_id) - return feature_data->feature_status; + rcu_read_lock(); + found =3D pm_api_feature_lookup(api_id); + if (found) { + ret =3D found->feature_status; + if (features) + memcpy(features, found->features, sizeof(found->features)); + rcu_read_unlock(); + return ret; } + rcu_read_unlock(); =20 /* Add new entry if not present */ feature_data =3D kmalloc_obj(*feature_data, GFP_ATOMIC); @@ -277,16 +311,33 @@ static int do_feature_check_call(const u32 api_id) =20 feature_data->pm_api_id =3D api_id; ret =3D __do_feature_check_call(api_id, ret_payload); - feature_data->feature_status =3D ret; - hash_add(pm_api_features_map, &feature_data->hentry, api_id); + /* Supported IOCTL/QUERY IDs mask, meaningful for PM_IOCTL and PM_QUERY_D= ATA */ + memcpy(feature_data->features, &ret_payload[2], sizeof(feature_data->feat= ures)); + + /* + * Firmware calls are made from any CPU without a common lock, so a + * concurrent caller may have added an entry for the same API while the + * firmware was being asked. Insert under pm_api_features_lock and + * recheck. hash_add_rcu() publishes the entry with release semantics, + * which pairs with the rcu_dereference() in the lockless lookup above, + * so a reader never sees the entry before its fields. + */ + spin_lock_irqsave(&pm_api_features_lock, flags); + found =3D pm_api_feature_lookup(api_id); + if (found) { + ret =3D found->feature_status; + if (features) + memcpy(features, found->features, sizeof(found->features)); + spin_unlock_irqrestore(&pm_api_features_lock, flags); + kfree(feature_data); + return ret; + } + hash_add_rcu(pm_api_features_map, &feature_data->hentry, api_id); + spin_unlock_irqrestore(&pm_api_features_lock, flags); =20 - if (api_id =3D=3D PM_IOCTL) - /* Store supported IOCTL IDs mask */ - memcpy(ioctl_features, &ret_payload[2], FEATURE_PAYLOAD_SIZE * 4); - else if (api_id =3D=3D PM_QUERY_DATA) - /* Store supported QUERY IDs mask */ - memcpy(query_features, &ret_payload[2], FEATURE_PAYLOAD_SIZE * 4); + if (features) + memcpy(features, &ret_payload[2], sizeof(feature_data->features)); =20 return ret; } @@ -305,7 +356,7 @@ int zynqmp_pm_feature(const u32 api_id) if (!feature_check_enabled) return 0; =20 - ret =3D do_feature_check_call(api_id); + ret =3D do_feature_check_call(api_id, NULL); =20 return ret; } @@ -322,14 +373,14 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_feature); int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id) { int ret; - u32 *bit_mask; + u32 bit_mask[FEATURE_PAYLOAD_SIZE]; =20 /* Input arguments validation */ if (id >=3D 64 || (api_id !=3D PM_IOCTL && api_id !=3D PM_QUERY_DATA)) return -EINVAL; =20 /* Check feature check API version */ - ret =3D do_feature_check_call(PM_FEATURE_CHECK); + ret =3D do_feature_check_call(PM_FEATURE_CHECK, NULL); if (ret < 0) return ret; =20 @@ -339,12 +390,10 @@ int zynqmp_pm_is_function_supported(const u32 api_id,= const u32 id) * Call feature check for IOCTL/QUERY API to get IOCTL ID or * QUERY ID feature status. */ - ret =3D do_feature_check_call(api_id); + ret =3D do_feature_check_call(api_id, bit_mask); if (ret < 0) return ret; =20 - bit_mask =3D (api_id =3D=3D PM_IOCTL) ? ioctl_features : query_features; - if ((bit_mask[(id / 32)] & BIT((id % 32))) =3D=3D 0U) return -EOPNOTSUPP; } else { @@ -2184,7 +2233,7 @@ static int zynqmp_clear_pm_state(struct device *dev) if (pm_family_code =3D=3D PM_VERSAL_FAMILY_CODE || pm_family_code =3D=3D PM_VERSAL_NET_FAMILY_CODE) { /* Check if EL3 firmware supports TF_A_CLEAR_PM_STATE */ - ret =3D do_feature_check_call(TF_A_CLEAR_PM_STATE); + ret =3D do_feature_check_call(TF_A_CLEAR_PM_STATE, NULL); if (ret >=3D 0 && ((ret & FIRMWARE_VERSION_MASK) >=3D PM_API_VERSION_1))= { /* Clear PM specific data in EL3 firmware */ ret =3D zynqmp_pm_invoke_fn(TF_A_CLEAR_PM_STATE, NULL, 0); @@ -2197,7 +2246,7 @@ static int zynqmp_clear_pm_state(struct device *dev) } =20 /* Check if the firmware supports the PM_DEV_ALL_PERIPH node ID */ - ret =3D do_feature_check_call(PM_RELEASE_NODE); + ret =3D do_feature_check_call(PM_RELEASE_NODE, NULL); if (ret >=3D 0 && ((ret & FIRMWARE_VERSION_MASK) >=3D PM_API_VERSION_3))= { /* Attempt to release all peripheral devices via firmware */ ret =3D zynqmp_pm_release_node(PM_DEV_ALL_PERIPH); @@ -2210,7 +2259,7 @@ static int zynqmp_clear_pm_state(struct device *dev) } =20 /* Check if the firmware supports the PM_ALL_NOTIFIERS node ID */ - ret =3D do_feature_check_call(PM_REGISTER_NOTIFIER); + ret =3D do_feature_check_call(PM_REGISTER_NOTIFIER, NULL); if (ret >=3D 0 && ((ret & FIRMWARE_VERSION_MASK) >=3D PM_API_VERSION_3))= { /* Attempt to unregister all notifier callbacks via firmware */ ret =3D zynqmp_pm_register_notifier(PM_ALL_NOTIFIERS, 0, 0, 0); @@ -2248,7 +2297,7 @@ static int zynqmp_firmware_probe(struct platform_devi= ce *pdev) if (ret) return ret; =20 - ret =3D do_feature_check_call(PM_FEATURE_CHECK); + ret =3D do_feature_check_call(PM_FEATURE_CHECK, NULL); if (ret >=3D 0 && ((ret & FIRMWARE_VERSION_MASK) >=3D PM_API_VERSION_1)) feature_check_enabled =3D true; =20 @@ -2325,15 +2374,18 @@ static void zynqmp_firmware_remove(struct platform_= device *pdev) { struct pm_api_feature_data *feature_data; struct hlist_node *tmp; + unsigned long flags; int i; =20 mfd_remove_devices(&pdev->dev); zynqmp_pm_api_debugfs_exit(); =20 + spin_lock_irqsave(&pm_api_features_lock, flags); hash_for_each_safe(pm_api_features_map, i, tmp, feature_data, hentry) { - hash_del(&feature_data->hentry); - kfree(feature_data); + hash_del_rcu(&feature_data->hentry); + kfree_rcu(feature_data, rcu); } + spin_unlock_irqrestore(&pm_api_features_lock, flags); =20 platform_device_unregister(em_dev); } --- base-commit: 93f51579e7df248780214094418f205253383cc5 change-id: 20260921-mb-zynqmp-fw-0b430526dada Best regards, -- =20 Jaidev Shastri