[PATCH 11/19] nitro_enclaves: Keep the CPU pool when the new CPU list does not parse

[Alexander Graf <graf@amazon.com>]

An admin who mistypes a CPU list into ne_cpus loses the pool that was
there, because ne_set_kernel_param() tears the pool down before
ne_setup_cpu_pool() parses the new value. The write fails, but the
pool's CPUs come back online and ne_cpus reads empty.

Parse the list before the teardown and pass the mask down. The other
checks stay after the teardown, because they want every CPU of the list
online, and a list that extends the current pool names offline CPUs.

Fixes: ff8a4d3e3a99 ("nitro_enclaves: Add logic for setting an enclave vCPU")
Signed-off-by: Alexander Graf <graf@amazon.com>
---
 drivers/virt/nitro_enclaves/ne_misc_dev.c | 58 ++++++++++++-----------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 1c2a4994257d1..de3bf93a4ef0f 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -143,7 +143,7 @@ struct ne_phys_contig_mem_regions {
  *			 or not using CPU 0, which should remain available for
  *			 the primary / parent VM. Offline the CPUs from the
  *			 pool after the checks passed.
- * @ne_cpu_list:	The CPU list used for setting NE CPU pool.
+ * @cpu_pool:	The CPU mask used for setting NE CPU pool.
  *
  * Context: Process context. This function is called with the ne_pci_dev enclave
  *	    mutex held, if there is a NE PCI device.
@@ -151,35 +151,24 @@ struct ne_phys_contig_mem_regions {
  * * 0 on success.
  * * Negative return value on failure.
  */
-static int ne_setup_cpu_pool(const char *ne_cpu_list)
+static int ne_setup_cpu_pool(const struct cpumask *cpu_pool)
 {
 	int core_id = -1;
 	unsigned int cpu = 0;
-	cpumask_var_t cpu_pool;
 	unsigned int cpu_sibling = 0;
 	unsigned int i = 0;
 	int numa_node = -1;
 	int rc = -EINVAL;
 
-	if (!zalloc_cpumask_var(&cpu_pool, GFP_KERNEL))
-		return -ENOMEM;
-
 	mutex_lock(&ne_cpu_pool.mutex);
 
-	rc = cpulist_parse(ne_cpu_list, cpu_pool);
-	if (rc < 0) {
-		pr_err("%s: Error in cpulist parse [rc=%d]\n", ne_misc_dev.name, rc);
-
-		goto free_pool_cpumask;
-	}
-
 	cpu = cpumask_any(cpu_pool);
 	if (cpu >= nr_cpu_ids) {
 		pr_err("%s: No CPUs available in CPU pool\n", ne_misc_dev.name);
 
 		rc = -EINVAL;
 
-		goto free_pool_cpumask;
+		goto reset_cpu_pool;
 	}
 
 	/*
@@ -193,7 +182,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 			rc = -EINVAL;
 
-			goto free_pool_cpumask;
+			goto reset_cpu_pool;
 		}
 
 	/*
@@ -208,7 +197,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 				rc = -EINVAL;
 
-				goto free_pool_cpumask;
+				goto reset_cpu_pool;
 			}
 		} else {
 			if (numa_node != cpu_to_node(cpu)) {
@@ -217,7 +206,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 				rc = -EINVAL;
 
-				goto free_pool_cpumask;
+				goto reset_cpu_pool;
 			}
 		}
 
@@ -230,7 +219,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 		rc = -EINVAL;
 
-		goto free_pool_cpumask;
+		goto reset_cpu_pool;
 	}
 
 	for_each_cpu(cpu_sibling, topology_sibling_cpumask(0)) {
@@ -240,7 +229,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 			rc = -EINVAL;
 
-			goto free_pool_cpumask;
+			goto reset_cpu_pool;
 		}
 	}
 
@@ -257,7 +246,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 
 				rc = -EINVAL;
 
-				goto free_pool_cpumask;
+				goto reset_cpu_pool;
 			}
 		}
 	}
@@ -274,7 +263,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 	if (!ne_cpu_pool.avail_threads_per_core) {
 		rc = -ENOMEM;
 
-		goto free_pool_cpumask;
+		goto reset_cpu_pool;
 	}
 
 	for (i = 0; i < ne_cpu_pool.nr_parent_vm_cores; i++)
@@ -325,8 +314,6 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 		}
 	}
 
-	free_cpumask_var(cpu_pool);
-
 	ne_cpu_pool.numa_node = numa_node;
 
 	mutex_unlock(&ne_cpu_pool.mutex);
@@ -343,8 +330,7 @@ static int ne_setup_cpu_pool(const char *ne_cpu_list)
 	for (i = 0; i < ne_cpu_pool.nr_parent_vm_cores; i++)
 		free_cpumask_var(ne_cpu_pool.avail_threads_per_core[i]);
 	kfree(ne_cpu_pool.avail_threads_per_core);
-free_pool_cpumask:
-	free_cpumask_var(cpu_pool);
+reset_cpu_pool:
 	ne_cpu_pool.nr_parent_vm_cores = 0;
 	ne_cpu_pool.nr_threads_per_core = 0;
 	ne_cpu_pool.numa_node = -1;
@@ -409,15 +395,19 @@ static void ne_teardown_cpu_pool(void)
 static int ne_set_kernel_param(const char *val, const struct kernel_param *kp)
 {
 	char error_val[] = "";
+	cpumask_var_t cpu_pool;
 	struct ne_pci_dev *ne_pci_dev = NULL;
 	int rc = -EINVAL;
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
+	if (!zalloc_cpumask_var(&cpu_pool, GFP_KERNEL))
+		return -ENOMEM;
+
 	rc = down_read_killable(&ne_devs.lock);
 	if (rc)
-		return rc;
+		goto free_cpumasks;
 
 	ne_pci_dev = ne_devs.ne_pci_dev;
 
@@ -442,9 +432,20 @@ static int ne_set_kernel_param(const char *val, const struct kernel_param *kp)
 		goto unlock;
 	}
 
+	/*
+	 * Parse the list before the teardown below drops the pool the driver
+	 * already has, so that a value that does not parse leaves it in place.
+	 */
+	rc = cpulist_parse(val, cpu_pool);
+	if (rc < 0) {
+		pr_err("%s: Error in cpulist parse [rc=%d]\n", ne_misc_dev.name, rc);
+
+		goto unlock;
+	}
+
 	ne_teardown_cpu_pool();
 
-	rc = ne_setup_cpu_pool(val);
+	rc = ne_setup_cpu_pool(cpu_pool);
 	if (rc < 0) {
 		pr_err("%s: Error in setup CPU pool [rc=%d]\n", ne_misc_dev.name, rc);
 
@@ -469,6 +470,9 @@ static int ne_set_kernel_param(const char *val, const struct kernel_param *kp)
 unlock_devs:
 	up_read(&ne_devs.lock);
 
+free_cpumasks:
+	free_cpumask_var(cpu_pool);
+
 	return rc;
 }
 
-- 
2.47.1