[PATCH] firmware: xilinx: zynqmp: serialise and publish the feature check cache

@@ -25,6 +25,8 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/hashtable.h>
+#include <linux/rcupdate.h>
+#include <linux/spinlock.h>
 
 #include <linux/firmware/xlnx-zynqmp.h>
 #include <linux/firmware/xlnx-event-manager.h>

@@ -45,8 +47,8 @@
 
 static bool feature_check_enabled;
 static DEFINE_HASHTABLE(pm_api_features_map, PM_API_FEATURE_CHECK_MAX_ORDER);
-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);
 
 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;
 };
 
 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];
 }
 
-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 == 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;
 
 	/* 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 == api_id)
-			return feature_data->feature_status;
+	rcu_read_lock();
+	found = pm_api_feature_lookup(api_id);
+	if (found) {
+		ret = found->feature_status;
+		if (features)
+			memcpy(features, found->features, sizeof(found->features));
+		rcu_read_unlock();
+		return ret;
 	}
+	rcu_read_unlock();
 
 	/* Add new entry if not present */
 	feature_data = kmalloc_obj(*feature_data, GFP_ATOMIC);

@@ -277,16 +311,33 @@ static int do_feature_check_call(const u32 api_id)
 
 	feature_data->pm_api_id = api_id;
 	ret = __do_feature_check_call(api_id, ret_payload);
-
 	feature_data->feature_status = ret;
-	hash_add(pm_api_features_map, &feature_data->hentry, api_id);
+	/* Supported IOCTL/QUERY IDs mask, meaningful for PM_IOCTL and PM_QUERY_DATA */
+	memcpy(feature_data->features, &ret_payload[2], sizeof(feature_data->features));
+
+	/*
+	 * 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 = pm_api_feature_lookup(api_id);
+	if (found) {
+		ret = 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);
 
-	if (api_id == PM_IOCTL)
-		/* Store supported IOCTL IDs mask */
-		memcpy(ioctl_features, &ret_payload[2], FEATURE_PAYLOAD_SIZE * 4);
-	else if (api_id == 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));
 
 	return ret;
 }

@@ -305,7 +356,7 @@ int zynqmp_pm_feature(const u32 api_id)
 	if (!feature_check_enabled)
 		return 0;
 
-	ret = do_feature_check_call(api_id);
+	ret = do_feature_check_call(api_id, NULL);
 
 	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];
 
 	/* Input arguments validation */
 	if (id >= 64 || (api_id != PM_IOCTL && api_id != PM_QUERY_DATA))
 		return -EINVAL;
 
 	/* Check feature check API version */
-	ret = do_feature_check_call(PM_FEATURE_CHECK);
+	ret = do_feature_check_call(PM_FEATURE_CHECK, NULL);
 	if (ret < 0)
 		return ret;
 

@@ -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 = do_feature_check_call(api_id);
+		ret = do_feature_check_call(api_id, bit_mask);
 		if (ret < 0)
 			return ret;
 
-		bit_mask = (api_id == PM_IOCTL) ? ioctl_features : query_features;
-
 		if ((bit_mask[(id / 32)] & BIT((id % 32))) == 0U)
 			return -EOPNOTSUPP;
 	} else {

@@ -2184,7 +2233,7 @@ static int zynqmp_clear_pm_state(struct device *dev)
 	if (pm_family_code == PM_VERSAL_FAMILY_CODE ||
 	    pm_family_code == PM_VERSAL_NET_FAMILY_CODE) {
 		/* Check if EL3 firmware supports TF_A_CLEAR_PM_STATE */
-		ret = do_feature_check_call(TF_A_CLEAR_PM_STATE);
+		ret = do_feature_check_call(TF_A_CLEAR_PM_STATE, NULL);
 		if (ret >= 0 && ((ret & FIRMWARE_VERSION_MASK) >= PM_API_VERSION_1)) {
 			/* Clear PM specific data in EL3 firmware */
 			ret = zynqmp_pm_invoke_fn(TF_A_CLEAR_PM_STATE, NULL, 0);

@@ -2197,7 +2246,7 @@ static int zynqmp_clear_pm_state(struct device *dev)
 		}
 
 		/* Check if the firmware supports the PM_DEV_ALL_PERIPH node ID */
-		ret = do_feature_check_call(PM_RELEASE_NODE);
+		ret = do_feature_check_call(PM_RELEASE_NODE, NULL);
 		if (ret >= 0 && ((ret & FIRMWARE_VERSION_MASK) >= PM_API_VERSION_3)) {
 			/* Attempt to release all peripheral devices via firmware */
 			ret = zynqmp_pm_release_node(PM_DEV_ALL_PERIPH);

@@ -2210,7 +2259,7 @@ static int zynqmp_clear_pm_state(struct device *dev)
 		}
 
 		/* Check if the firmware supports the PM_ALL_NOTIFIERS node ID */
-		ret = do_feature_check_call(PM_REGISTER_NOTIFIER);
+		ret = do_feature_check_call(PM_REGISTER_NOTIFIER, NULL);
 		if (ret >= 0 && ((ret & FIRMWARE_VERSION_MASK) >= PM_API_VERSION_3)) {
 			/* Attempt to unregister all notifier callbacks via firmware */
 			ret = zynqmp_pm_register_notifier(PM_ALL_NOTIFIERS, 0, 0, 0);

@@ -2248,7 +2297,7 @@ static int zynqmp_firmware_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = do_feature_check_call(PM_FEATURE_CHECK);
+	ret = do_feature_check_call(PM_FEATURE_CHECK, NULL);
 	if (ret >= 0 && ((ret & FIRMWARE_VERSION_MASK) >= PM_API_VERSION_1))
 		feature_check_enabled = true;
 

@@ -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;
 
 	mfd_remove_devices(&pdev->dev);
 	zynqmp_pm_api_debugfs_exit();
 
+	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);
 
 	platform_device_unregister(em_dev);
 }